Re: [Qemu-devel] [PATCH 1/5] qom/object: Add a new function object_initialize_as_child()

[Thomas Huth]

On 13.07.2018 08:26, Paolo Bonzini wrote:
> On 12/07/2018 18:55, Thomas Huth wrote:
>>>> +void object_initialize_as_child(Object *parentobj, const char *propname,
>>>> +                                void *childobj, size_t size, const char 
>>>> *type,
>>>> +                                Error **errp);
>>> Why did you use void* instead of Object*?
>> That's the same what object_initialize() is doing (see above). Otherwise
>> all the callers have to cast their pointers with OBJECT() first.
> 
> Actually I had forgotten about object_new_with_props, which is very
> similar to the new function above.  I think we should follow the model
> and define the new function as
> 
> void object_initialize_with_props(Object *parent, const char *id,
>                            void *childobj,
>                            size_t size, const char *type,
>                            Error **errp,
>                            ...) QEMU_SENTINEL;
> void object_initialize_with_propv(Object *parent, const char *id,
>                            void *childobj,
>                            size_t size, const char *type,
>                            Error **errp,
>                            va_list vargs);
> 
> I actually prefer Thomas's name (or maybe even
> object_initialize_child/object_initialize_childv), but if we adopt it we
> should also rename object_new_with_props and object_new_with_propv, for
> consistency.

Since we are in hard freeze already, I'd rather prefer to keep it
simple, without the variable args for now (we don't need them yet).
Especially since we can easily add them later when we need them,
changing the prototype from

void object_initialize_child(Object *parentobj, const char *propname,
                             void *childobj, size_t size,
                             const char *type, Error **errp);

to

void object_initialize_child(Object *parentobj, const char *propname,
                             void *childobj, size_t size,
                             const char *type, Error **errp, ...);

should not affect any of the existing caller sites later.

And yes, I think these functions should have "child" in their names, so
I'll continue with object_initialize_child now. Renaming
object_new_with_props to object_new_child also sounds like a good idea
to me, but that's something for the time when the hard freeze is over.

 Thomas