On Mon, Oct 8, 2012 at 11:29 PM, Anthony Liguori <aligu...@us.ibm.com> wrote: > Peter Maydell <peter.mayd...@linaro.org> 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 <peter.mayd...@linaro.org> 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 <peter.mayd...@linaro.org> >>> --- >>> 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 >>> >>> > >