Re: [Qemu-devel] [PATCH v2] qom: Reject attempts to add a property that already exists

[Peter Crosthwaite]

On Mon, Oct 8, 2012 at 11:29 PM, Anthony Liguori <address@hidden> wrote:
> Peter Maydell <address@hidden> writes:
>
>> Ping!
>
> This is wrong.
>
> Container properties are added by the user.  You will turn a gracefully
> failure (during hotplug) into an abort().
>

Can we just populate errp with a nice meaningful error (perhaps the
contents of that printf), then the caller can decide if failure is
tolerable?

Regards,
Peter

> Please limit this to static properties as they are not added by a user.
>
> Regards,
>
> Anthony Liguori
>
>>
>> -- PMM
>>
>> On 7 September 2012 14:55, Peter Maydell <address@hidden> wrote:
>>> Reject attempts to add a property to an object if one of
>>> that name already exists. This is always a bug in the caller;
>>> this is merely diagnosing it gracefully rather than behaving
>>> oddly later.
>>>
>>> Signed-off-by: Peter Maydell <address@hidden>
>>> ---
>>> Changes v1->v2:
>>>  * use abort() rather than assert(0), as suggested by Paolo
>>>  * make the diagnostic message a little more helpful by
>>>    including the type name, and adding '' around names
>>>  * the patches fixing bugs which this patch makes fatal errors
>>>    have both now been committed to master, so there's no
>>>    barrier to committing it now
>>>
>>>  qom/object.c |   13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qom/object.c b/qom/object.c
>>> index e3e9242..3da4c0e 100644
>>> --- a/qom/object.c
>>> +++ b/qom/object.c
>>> @@ -620,7 +620,18 @@ void object_property_add(Object *obj, const char 
>>> *name, const char *type,
>>>                           ObjectPropertyRelease *release,
>>>                           void *opaque, Error **errp)
>>>  {
>>> -    ObjectProperty *prop = g_malloc0(sizeof(*prop));
>>> +    ObjectProperty *prop;
>>> +
>>> +    QTAILQ_FOREACH(prop, &obj->properties, node) {
>>> +        if (strcmp(prop->name, name) == 0) {
>>> +            /* This is always a bug in the caller */
>>> +            fprintf(stderr, "attempt to add duplicate property '%s'"
>>> +                    " to object (type '%s')\n", name, 
>>> object_get_typename(obj));
>>> +            abort();
>>> +        }
>>> +    }
>>> +
>>> +    prop = g_malloc0(sizeof(*prop));
>>>
>>>      prop->name = g_strdup(name);
>>>      prop->type = g_strdup(type);
>>> --
>>> 1.7.9.5
>>>
>>>
>
>