qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH 1/5] qom: add a generic mechanism to resolve paths
Date: Tue, 17 Jun 2014 17:19:32 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]