Constify all accesses to qdev properties, except for the
ObjectPropertyAccessor itself. This makes it possible to place them in
read-only memory, and also lets Rust bindings switch from "static mut"
arrays to "static"; which is advantageous, because mutable statics are
highly discouraged.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/qdev-core.h | 4 ++--
include/hw/qdev-properties.h | 4 ++--
hw/core/qdev-properties.c | 26 +++++++++++++-------------
system/qdev-monitor.c | 2 +-
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index aa97c34a4be..f9fa291cc63 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -132,7 +132,7 @@ struct DeviceClass {
* ensures a compile-time error if someone attempts to assign
* dc->props directly.
*/
- Property *props_;
+ const Property *props_;
/**
* @user_creatable: Can user instantiate with -device / device_add?
@@ -935,7 +935,7 @@ char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, DeviceState *dev);
* you attempt to add an existing property defined by a parent class.
* To modify an inherited property you need to use????
*/
-void device_class_set_props(DeviceClass *dc, Property *props);
+void device_class_set_props(DeviceClass *dc, const Property *props);
/**
* device_class_set_parent_realize() - set up for chaining realize fns
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 09aa04ca1e2..26ebd230685 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -37,7 +37,7 @@ struct PropertyInfo {
int (*print)(Object *obj, Property *prop, char *dest, size_t len);
void (*set_default_value)(ObjectProperty *op, const Property *prop);
ObjectProperty *(*create)(ObjectClass *oc, const char *name,
- Property *prop);
+ const Property *prop);
ObjectPropertyAccessor *get;
ObjectPropertyAccessor *set;
ObjectPropertyRelease *release;
@@ -223,7 +223,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
* On error, store error in @errp. Static properties access data in a struct.
* The type of the QOM property is derived from prop->info.
*/
-void qdev_property_add_static(DeviceState *dev, Property *prop);
+void qdev_property_add_static(DeviceState *dev, const Property *prop);
/**
* qdev_alias_all_properties: Create aliases on source for all target properties
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 86a583574dd..315196bd85a 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -749,7 +749,7 @@ const PropertyInfo qdev_prop_array = {
/* --- public helpers --- */
-static Property *qdev_prop_walk(Property *props, const char *name)
+static const Property *qdev_prop_walk(const Property *props, const char *name)
{
if (!props) {
return NULL;
@@ -763,10 +763,10 @@ static Property *qdev_prop_walk(Property *props, const char *name)
return NULL;
}
-static Property *qdev_prop_find(DeviceState *dev, const char *name)
+static const Property *qdev_prop_find(DeviceState *dev, const char *name)
{
ObjectClass *class;
- Property *prop;
+ const Property *prop;
/* device properties */
class = object_get_class(OBJECT(dev));
@@ -840,7 +840,7 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value)
{
- Property *prop;
+ const Property *prop;
prop = qdev_prop_find(dev, name);
object_property_set_str(OBJECT(dev), name,
@@ -956,7 +956,7 @@ const PropertyInfo qdev_prop_size = {
/* --- object link property --- */
static ObjectProperty *create_link_property(ObjectClass *oc, const char *name,
- Property *prop)
+ const Property *prop)
{
return object_class_property_add_link(oc, name, prop->link_type,
prop->offset,
@@ -969,7 +969,7 @@ const PropertyInfo qdev_prop_link = {
.create = create_link_property,
};
-void qdev_property_add_static(DeviceState *dev, Property *prop)
+void qdev_property_add_static(DeviceState *dev, const Property *prop)
{
Object *obj = OBJECT(dev);
ObjectProperty *op;
@@ -980,7 +980,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
field_prop_getter(prop->info),
field_prop_setter(prop->info),
prop->info->release,
- prop);
+ (Property *)prop);
object_property_set_description(obj, prop->name,
prop->info->description);
@@ -994,7 +994,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
}
static void qdev_class_add_property(DeviceClass *klass, const char *name,
- Property *prop)
+ const Property *prop)
{
ObjectClass *oc = OBJECT_CLASS(klass);
ObjectProperty *op;
@@ -1007,7 +1007,7 @@ static void qdev_class_add_property(DeviceClass *klass, const char *name,
field_prop_getter(prop->info),
field_prop_setter(prop->info),
prop->info->release,
- prop);
+ (Property *)prop);
}
if (prop->set_default) {
prop->info->set_default_value(op, prop);
@@ -1046,7 +1046,7 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v,
* Do not use this in new code! QOM Properties added through this interface
* will be given names in the "legacy" namespace.
*/
-static void qdev_class_add_legacy_property(DeviceClass *dc, Property *prop)
+static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop)
{
g_autofree char *name = NULL;
@@ -1058,12 +1058,12 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, Property *prop)
name = g_strdup_printf("legacy-%s", prop->name);
object_class_property_add(OBJECT_CLASS(dc), name, "str",
prop->info->print ? qdev_get_legacy_property : prop->info->get,
- NULL, NULL, prop);
+ NULL, NULL, (Property *)prop);
}
-void device_class_set_props(DeviceClass *dc, Property *props)
+void device_class_set_props(DeviceClass *dc, const Property *props)
{
- Property *prop;
+ const Property *prop;
dc->props_ = props;
for (prop = props; prop && prop->name; prop++) {
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 44994ea0e16..c346ea6ae4b 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -751,7 +751,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
#define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
-static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
+static void qdev_print_props(Monitor *mon, DeviceState *dev, const Property *props,
int indent)
{
if (!props)
--
2.46.2Hi,
On 21/10/24 13:35, Paolo Bonzini wrote:
> Constify all accesses to qdev properties, except for the
> ObjectPropertyAccessor itself. This makes it possible to place them in
> read-only memory, and also lets Rust bindings switch from "static mut"
> arrays to "static"; which is advantageous, because mutable statics are
> highly discouraged.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/hw/qdev-core.h | 4 ++--
> include/hw/qdev-properties.h | 4 ++--
> hw/core/qdev-properties.c | 26 +++++++++++++-------------
> system/qdev-monitor.c | 2 +-
> 4 files changed, 18 insertions(+), 18 deletions(-)
> -void qdev_property_add_static(DeviceState *dev, Property *prop)
> +void qdev_property_add_static(DeviceState *dev, const Property *prop)
> {
> Object *obj = OBJECT(dev);
> ObjectProperty *op;
> @@ -980,7 +980,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
> field_prop_getter(prop->info),
> field_prop_setter(prop->info),
> prop->info->release,
> - prop);
> + (Property *)prop);
I like the overall patch idea, but I'm not keep on casting
const to non-const. Should we adapt the callee -- here
object_property_add() -- to also take a const argument?
>
> object_property_set_description(obj, prop->name,
> prop->info->description);
> @@ -994,7 +994,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
> }
>
> static void qdev_class_add_property(DeviceClass *klass, const char *name,
> - Property *prop)
> + const Property *prop)
> {
> ObjectClass *oc = OBJECT_CLASS(klass);
> ObjectProperty *op;
> @@ -1007,7 +1007,7 @@ static void qdev_class_add_property(DeviceClass *klass, const char *name,
> field_prop_getter(prop->info),
> field_prop_setter(prop->info),
> prop->info->release,
> - prop);
> + (Property *)prop);
> }
> if (prop->set_default) {
> prop->info->set_default_value(op, prop);
> @@ -1046,7 +1046,7 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v,
> * Do not use this in new code! QOM Properties added through this interface
> * will be given names in the "legacy" namespace.
> */
> -static void qdev_class_add_legacy_property(DeviceClass *dc, Property *prop)
> +static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop)
> {
> g_autofree char *name = NULL;
>
> @@ -1058,12 +1058,12 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, Property *prop)
> name = g_strdup_printf("legacy-%s", prop->name);
> object_class_property_add(OBJECT_CLASS(dc), name, "str",
> prop->info->print ? qdev_get_legacy_property : prop->info->get,
> - NULL, NULL, prop);
> + NULL, NULL, (Property *)prop);
> }
On Tue, Oct 22, 2024 at 6:31 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
> > -void qdev_property_add_static(DeviceState *dev, Property *prop)
> > +void qdev_property_add_static(DeviceState *dev, const Property *prop)
> > {
> > Object *obj = OBJECT(dev);
> > ObjectProperty *op;
> > @@ -980,7 +980,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
> > field_prop_getter(prop->info),
> > field_prop_setter(prop->info),
> > prop->info->release,
> > - prop);
> > + (Property *)prop);
>
> I like the overall patch idea, but I'm not keen on casting
> const to non-const. Should we adapt the callee -- here
> object_property_add() -- to also take a const argument?
This argument goes into prop->opaque and is passed to all
accessor/resolver/finalizers functions. So it would be a much larger
change because it needs to change all those functions from "void
*opaque" to "const void *opaque".
It would also be an issue because some finalizers write to the opaque
for good reason:
static void object_finalize_child_property(
Object *obj, const char *name, void *opaque)
{
Object *child = opaque;
if (child->class->unparent) {
(child->class->unparent)(child);
}
child->parent = NULL; // <--- here
object_unref(child);
}
So, it's not great but it seems necessary. At least keeping the const
within qdev properties makes things "safer" within that realm.
Paolo
On 22/10/24 02:23, Paolo Bonzini wrote:
> On Tue, Oct 22, 2024 at 6:31 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
>>> -void qdev_property_add_static(DeviceState *dev, Property *prop)
>>> +void qdev_property_add_static(DeviceState *dev, const Property *prop)
>>> {
>>> Object *obj = OBJECT(dev);
>>> ObjectProperty *op;
>>> @@ -980,7 +980,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop)
>>> field_prop_getter(prop->info),
>>> field_prop_setter(prop->info),
>>> prop->info->release,
>>> - prop);
>>> + (Property *)prop);
>>
>> I like the overall patch idea, but I'm not keen on casting
>> const to non-const. Should we adapt the callee -- here
>> object_property_add() -- to also take a const argument?
>
> This argument goes into prop->opaque and is passed to all
> accessor/resolver/finalizers functions. So it would be a much larger
> change because it needs to change all those functions from "void
> *opaque" to "const void *opaque".
>
> It would also be an issue because some finalizers write to the opaque
> for good reason:
>
> static void object_finalize_child_property(
> Object *obj, const char *name, void *opaque)
> {
> Object *child = opaque;
>
> if (child->class->unparent) {
> (child->class->unparent)(child);
> }
> child->parent = NULL; // <--- here
> object_unref(child);
> }
>
> So, it's not great but it seems necessary. At least keeping the const
> within qdev properties makes things "safer" within that realm.
Since it is only within qdev-properties.c, it is indeed reasonable to
accept. Maybe make it explicit via a well-named macro to do the cast?
/* NON_CONST_PROP: Specific macro to this file because ... */
#define NON_CONST_PROP(prop) (Property *)(prop)
Regards,
Phil.
© 2016 - 2024 Red Hat, Inc.