| Message ID | 1343140218-23741-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Il 24/07/2012 16:30, Peter Maydell ha scritto: > + 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 set duplicate property %s on object\n", > + name); > + assert(0); abort(); That's all I have to say. :) Paolo > + } > + } > + > + prop = g_malloc0(sizeof(*prop));
diff --git a/qom/object.c b/qom/object.c index 00bb3b0..dceabc0 100644 --- a/qom/object.c +++ b/qom/object.c @@ -659,7 +659,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 set duplicate property %s on object\n", + name); + assert(0); + } + } + + prop = g_malloc0(sizeof(*prop)); prop->name = g_strdup(name); prop->type = g_strdup(type);
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.maydell@linaro.org> --- This patch can't be applied until these two have been: http://patchwork.ozlabs.org/patch/172885/ http://patchwork.ozlabs.org/patch/172820/ but I think we're probably going to argue about it anyway. The question really is whether we want to treat attempts to add a duplicate property as a programming bug (ie the calling code should either know there are no duplicates or check first with object_property_find) or whether we want to return a helpful failure code from this function. In the code at the moment: * object_property_add() makes no attempt to cope with duplicate adds (they get added at the end of the list so will be never found on a subsequent search, and can't be dereferenced without first deleting the earlier of the two duplicates) * there's no attempt to handle failure-to-add in any of the utility helpers like object_property_add_child() (which will ref the child object regardless) * no caller of object_property_add() that I can find passes in anything except NULL for the Error** so if we were to do an error_set() here rather than assert() then the error just vanishes into the ether. * the documentation in object.h for these functions doesnt' define semantics for attempts to add duplicate properties So in theory we could define some semantics (eg "return error if property already exists"), define a new QERR_* constants since as usual the existing cast of thousands of QERR_* constants are unsuitable, fix all the callers to correctly handle and propagate the error, make device_initfn() print an error, and so on. But I think this patch is much simpler and more effective :-) qom/object.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)