Re: [PATCH v5 03/10] qdev: device module support

[Christophe de Dinechin]

On 2020-07-22 at 13:05 CEST, Gerd Hoffmann wrote...
> On Wed, Jul 22, 2020 at 10:05:51AM +0200, Christophe de Dinechin wrote:
>>
>> On 2020-07-21 at 16:27 CEST, Gerd Hoffmann wrote...
>> >   Hi,
>> >
>> >> >  DeviceState *qdev_new(const char *name)
>> >> >  {
>> >> > +    if (!object_class_by_name(name)) {
>> >> > +        module_load_qom_one(name);
>> >> > +    }
>> >>
>> >> Curious why you don't you call module_object_class_by_name here?
>> >
>> > Because object_new() wants a name not an ObjectClass ...
>>
>> I'm talking about the two lines above.
>>
>>     if (!object_class_by_name(name)) {
>>         module_load_qom_one(name);
>>     }
>>
>> Thi9s code looks very similar to the code below:
>>
>>     ObjectClass *module_object_class_by_name(const char *typename)
>>     {
>>         ObjectClass *oc;
>>
>>         oc = object_class_by_name(typename);
>>     #ifdef CONFIG_MODULES
>>         if (!oc) {
>>             module_load_qom_one(typename);
>>             oc = object_class_by_name(typename);
>>         }
>>     #endif
>>         return oc;
>>     }
>>
>> Both call module_load_qom_one and object_class_by_name using the name as
>> input, so I don't see the difference (except for the order).
>
> Yes, calling module_object_class_by_name then throw away the result
> would work too.  I don't like the idea to hide the module loading
> though.

Why do you consider calling a function called "module_object_class_by_name"
as hiding the module loading?

More importantly, why is it better to have two ways to do the same thing
that are slightly different? The reason for the slight difference is really
unclear to me. If we later do a change to module_object_class_by_name, are
there cases where we won't need the same change in qdev_new?

>
> take care,
>   Gerd


--
Cheers,
Christophe de Dinechin (IRC c3d)