Re: [PATCH v2 10/13] qdev: make properties array "const"

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.