[Qemu-devel] Re: [PATCH 2/6] qdev/compat: compat property infrastructure.

[Juan Quintela]

Gerd Hoffmann <address@hidden> wrote:
> On 07/13/09 21:36, Michael S. Tsirkin wrote:
>> Some coding style comments
>>> +        return;
>>> +    for (prop = compat_props; prop->driver != NULL; prop++) {
>>
>> != NULL not needed in if
>>
>>> +        if (strcmp(dev->info->name, prop->driver) != 0)
>>
>> != 0 not needed in if
>
> I still prefer to have it explicitly written as it makes the code more
> readable IMHO.
>
>>>   void qdev_prop_set_defaults(DeviceState *dev, Property *props);
>>>
>>> +void qdev_register_compat_props(CompatProperty *props);
>>
>> qedev_set_compat_props might be a better name.
>>
>>> +void qdev_prop_set_compat(DeviceState *dev);
>>
>> qdev_parse_compat_props might be a better name.
>
> Disagree on both.  qdev_prop_set_compat intentionally follows the name
> convention of the other qdev_prop_set_* functions.  The
> qdev_register_compat_props intentionally isn't named something with
> "set" to avoid confusion with the other ones because it doesn't
> actually set properties on devices.  "register" maybe isn't the best
> idea, I'm open to better suggestions.

I didn't liked either. I would prefer to also use qdev_prop_ preffix as
the other functions that deal with properties.

+void qdev_prop_register_compat(CompatProperty *props); ???

(I didn't get any good name with [qdev,prop,compat] in it.

Later, Juan.

> Fixed the other ones.
>
> cheers,
>   Gerd