Re: [Qemu-devel] [PATCH 1/5] qom: add a generic mechanism to resolve paths

["Andreas Färber" <afaerber@suse.de>]

Am 17.06.2014 17:07, schrieb Paolo Bonzini:
> Il 17/06/2014 16:18, Andreas Färber ha scritto:
>>> +void object_property_add_full(Object *obj, const char *name, const
>>> char *type,
>>> +                              ObjectPropertyAccessor *get,
>>> +                              ObjectPropertyAccessor *set,
>>> +                              ObjectPropertyResolve *resolve,
>>> +                              ObjectPropertyRelease *release,
>>> +                              void *opaque, Error **errp);
>>
>> I'm a bit concerned about the duplication going on here, e.g. of the
>> forbidden characters. I think we should either just add the new argument
>> to object_property_add() and add NULL arguments at old call sites as
>> done before, or we should (to avoid future _really_full,
>> _really_really_full versions) return the resulting ObjectProperty * for
>> modification by the caller (in this case: ->resolve = foo).
> 
> The reason I went with "_full" is that the new argument is really needed
> only in a minority of cases.  There are ~50 occurrences right now, and I
> expect only a handful of them to need a ->resolve callback (and so far
> all of them would be in qom/object.c).
> 
> There are many examples in glib's GSource (g_timeout_add_full,
> g_main_context_invoke_full, etc.) or elsewhere in glib
> (g_format_size_full).
> 
> Since we do not have an ABI to follow, we could add arguments to the
> _full routine while keeping the shorthand version as is.
> 
> I can change the 50 occurrences if you want though.

So what about my alternative suggestion of changing _add's void ->
ObjectProperty*? That would limit future updating to the struct itself
while still avoiding to touch the 50.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg