From patchwork Tue Jul 24 14:30:18 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 10228 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 2819D23E56 for ; Tue, 24 Jul 2012 14:30:30 +0000 (UTC) Received: from mail-yx0-f180.google.com (mail-yx0-f180.google.com [209.85.213.180]) by fiordland.canonical.com (Postfix) with ESMTP id DB590A188FD for ; Tue, 24 Jul 2012 14:30:29 +0000 (UTC) Received: by yenq6 with SMTP id q6so7029031yen.11 for ; Tue, 24 Jul 2012 07:30:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:from:to:cc :subject:date:message-id:x-mailer:x-gm-message-state; bh=s7k/1qhe3qYZ2QMF48eR4Qm/OPxDrIqzhi8ej9I5HrI=; b=EN5nwoIHXFWxQ6mEAXW8kJSUc7nE3FKYEkyLzuWcndDMTZ0h+N+ntLeR3ZHF0EiS0e JQbN6EbHRBild9kD8ywjsNGqXJpPNSz3SnoG76gOEIzadVncQYr+TyTUmUOZOG/9T8oO p2g45TheEwwRMOcv2MtHYxGL/fLEWJCXj9CeUvsLQwSXilUubikLcCPoJqT1W/03b1pH J8UAjHYgS3l3m5nXEUyfbMdI7rQUVbzk5FcJJHqjDF+L20DVrh3tQhoLPW9lX4O4AYxU UotO8BGG+nzvoNdmLRsSh2mHU1c7tpYXlYAjYb2qyXMFEVoQg6kBQgSe++vAt4iuY+gG VkWA== Received: by 10.42.54.133 with SMTP id r5mr11314043icg.9.1343140228905; Tue, 24 Jul 2012 07:30:28 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.153.7 with SMTP id i7csp74492ibw; Tue, 24 Jul 2012 07:30:27 -0700 (PDT) Received: by 10.216.29.10 with SMTP id h10mr10348408wea.126.1343140227075; Tue, 24 Jul 2012 07:30:27 -0700 (PDT) Received: from mnementh.archaic.org.uk (mnementh.archaic.org.uk. [81.2.115.146]) by mx.google.com with ESMTPS id m55si20504518wee.29.2012.07.24.07.30.26 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 24 Jul 2012 07:30:27 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 81.2.115.146 as permitted sender) client-ip=81.2.115.146; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 81.2.115.146 as permitted sender) smtp.mail=pm215@archaic.org.uk Received: from pm215 by mnementh.archaic.org.uk with local (Exim 4.72) (envelope-from ) id 1Stg7u-0006BM-Nd; Tue, 24 Jul 2012 15:30:18 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org, Anthony Liguori , Paolo Bonzini , Markus Armbruster Subject: [PATCH] qom: Reject attempts to add a property that already exists Date: Tue, 24 Jul 2012 15:30:18 +0100 Message-Id: <1343140218-23741-1-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 1.7.2.5 X-Gm-Message-State: ALoCoQmjtarukY6FuIDd4bNYh+evHNhQ58yzeSN6iMwXoOI8E4sesSGespNF1G8VTIxcd9NwAw// 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 --- 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(-) 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);