From patchwork Fri Aug 17 17:19:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 959053 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41sVPy5cMDz9sCP for ; Sat, 18 Aug 2018 03:22:30 +1000 (AEST) Received: from localhost ([::1]:35577 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqiSC-0008Nj-CZ for incoming@patchwork.ozlabs.org; Fri, 17 Aug 2018 13:22:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqiPd-0004vc-5k for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:19:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fqiPc-0005Hg-DO for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:19:49 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47378 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fqiPa-0005GS-Fd; Fri, 17 Aug 2018 13:19:46 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2BE694022414; Fri, 17 Aug 2018 17:19:46 +0000 (UTC) Received: from localhost (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by smtp.corp.redhat.com (Postfix) with ESMTP id BB1AB100F37D; Fri, 17 Aug 2018 17:19:43 +0000 (UTC) From: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= To: qemu-devel@nongnu.org Date: Fri, 17 Aug 2018 19:19:31 +0200 Message-Id: <20180817171932.31976-2-marcandre.lureau@redhat.com> In-Reply-To: <20180817171932.31976-1-marcandre.lureau@redhat.com> References: <20180817171932.31976-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 17 Aug 2018 17:19:46 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 17 Aug 2018 17:19:46 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'marcandre.lureau@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 1/2] qdict: add qdict_steal() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , "open list:Block layer core" , armbru@redhat.com, Max Reitz , =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , "Dr. David Alan Gilbert" Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add a new function qdict_steal(), that deletes a key from a dict, and returns the associated value, if any. Simplify related code. Signed-off-by: Marc-André Lureau --- include/qapi/qmp/qdict.h | 1 + monitor.c | 3 +-- qobject/block-qdict.c | 7 ++----- qobject/qdict.c | 22 ++++++++++++++++++++++ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 7f3ec10a10..84644bfba6 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -37,6 +37,7 @@ QObject *qdict_entry_value(const QDictEntry *entry); size_t qdict_size(const QDict *qdict); void qdict_put_obj(QDict *qdict, const char *key, QObject *value); void qdict_del(QDict *qdict, const char *key); +QObject *qdict_steal(QDict *qdict, const char *key); int qdict_haskey(const QDict *qdict, const char *key); QObject *qdict_get(const QDict *qdict, const char *key); bool qdict_is_equal(const QObject *x, const QObject *y); diff --git a/monitor.c b/monitor.c index a1999e396c..eab2dc7b7b 100644 --- a/monitor.c +++ b/monitor.c @@ -4262,8 +4262,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) qdict = qobject_to(QDict, req); if (qdict) { - id = qobject_ref(qdict_get(qdict, "id")); - qdict_del(qdict, "id"); + id = qdict_steal(qdict, "id"); } /* else will fail qmp_dispatch() */ if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) { diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c index 42054cc274..a07664ee6a 100644 --- a/qobject/block-qdict.c +++ b/qobject/block-qdict.c @@ -692,8 +692,6 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite) */ bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error **errp) { - QObject *qobj; - while (renames->from) { if (qdict_haskey(qdict, renames->from)) { if (qdict_haskey(qdict, renames->to)) { @@ -702,9 +700,8 @@ bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error **errp) return false; } - qobj = qdict_get(qdict, renames->from); - qdict_put_obj(qdict, renames->to, qobject_ref(qobj)); - qdict_del(qdict, renames->from); + qdict_put_obj(qdict, renames->to, + qdict_steal(qdict, renames->from)); } renames++; diff --git a/qobject/qdict.c b/qobject/qdict.c index 3d8c2f7bbc..322f23f094 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -406,6 +406,28 @@ void qdict_del(QDict *qdict, const char *key) } } +/** + * qdict_steal(): Steal a 'key:value' pair from the dictionary + * + * Return the associated entry value and remove it, or NULL. + */ +QObject *qdict_steal(QDict *qdict, const char *key) +{ + QDictEntry *entry; + QObject *val = NULL; + + entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX); + if (entry) { + QLIST_REMOVE(entry, next); + val = qobject_ref(entry->value); + qentry_destroy(entry); + qdict->size--; + } + + return val; +} + + /** * qdict_is_equal(): Test whether the two QDicts are equal * From patchwork Fri Aug 17 17:19:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 959054 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41sVS93WZpz9s47 for ; Sat, 18 Aug 2018 03:24:25 +1000 (AEST) Received: from localhost ([::1]:35585 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqiU3-0001SU-5J for incoming@patchwork.ozlabs.org; Fri, 17 Aug 2018 13:24:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34583) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqiPm-0005ag-0u for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:19:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fqiPl-0005Le-6I for qemu-devel@nongnu.org; Fri, 17 Aug 2018 13:19:58 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:60746 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fqiPf-0005JD-KO; Fri, 17 Aug 2018 13:19:51 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 48C9826364; Fri, 17 Aug 2018 17:19:51 +0000 (UTC) Received: from localhost (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by smtp.corp.redhat.com (Postfix) with ESMTP id D95ED2026D6C; Fri, 17 Aug 2018 17:19:47 +0000 (UTC) From: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= To: qemu-devel@nongnu.org Date: Fri, 17 Aug 2018 19:19:32 +0200 Message-Id: <20180817171932.31976-3-marcandre.lureau@redhat.com> In-Reply-To: <20180817171932.31976-1-marcandre.lureau@redhat.com> References: <20180817171932.31976-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 17 Aug 2018 17:19:51 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 17 Aug 2018 17:19:51 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'marcandre.lureau@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 2/2] qobject: modify qobject_ref() to assert on NULL X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alberto Garcia , "open list:Block layer core" , armbru@redhat.com, Max Reitz , =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , "Dr. David Alan Gilbert" Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" While it may be convenient to accept NULL value in qobject_unref() (for similar reasons as free() accepts NULL), it is not such a good idea for qobject_ref(): now assert() on NULL. Some code relied on that behaviour, but it's best to be explicit that NULL is accepted. We have to rely on testing, and manual inspection of qobject_ref() usage: * block.c: - bdrv_refresh_filename(): guarded - append_open_options(): it depends if qdict values could be NULL, handled for extra safety, might be unnecessary * block/blkdebug.c: - blkdebug_refresh_filename(): depends if qdict values could be NULL, full_open_options could be NULL apparently, handled * block/blkverify.c: guarded * block/{null,nvme}.c: guarded, previous qdict_del() (actually qdict_find()) guarantee non-NULL) * block/quorum.c: full_open_options could be NULL, handled for extra safety, might be unnecessary * monitor: events have associated qdict, but may not have 'data' dict entry. Command 'id' is already guarded. A queued response is non-NULL. * qapi/qmp-dispatch.c: if "arguments" exists, it can't be NULL during json parsing * qapi/qobject-input-visitor.c: guarded by assert in visit_type_any() * qapi/qobject-output-visitor.c: guarded by assert() in visit_type_any() qobject_output_complete(): guarded by pre-condition assert() * qmp.c: guarded, error out before if NULL * qobject/q{list,dict}.c: can accept NULL values apparently, what's the reason? how are you supposed to handle that? no test? Some code, such as qdict_flatten_qdict(), assume the value is always non-NULL for example. - tests/*: considered to be covered by make check, not critical - util/keyval.c: guarded, if (!elt[i]) before Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia --- include/qapi/qmp/qobject.h | 7 ++++--- block.c | 6 ++++-- block/blkdebug.c | 3 ++- block/quorum.c | 3 ++- monitor.c | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h index fcfd549220..2fe5b42579 100644 --- a/include/qapi/qmp/qobject.h +++ b/include/qapi/qmp/qobject.h @@ -74,9 +74,8 @@ static inline void qobject_init(QObject *obj, QType type) static inline void qobject_ref_impl(QObject *obj) { - if (obj) { - obj->base.refcnt++; - } + assert(obj); + obj->base.refcnt++; } /** @@ -103,6 +102,7 @@ static inline void qobject_unref_impl(QObject *obj) /** * qobject_ref(): Increment QObject's reference count + * @obj: a #QObject or child type instance (must not be NULL) * * Returns: the same @obj. The type of @obj will be propagated to the * return type. @@ -116,6 +116,7 @@ static inline void qobject_unref_impl(QObject *obj) /** * qobject_unref(): Decrement QObject's reference count, deallocate * when it reaches zero + * @obj: a #QObject or child type instance (can be NULL) */ #define qobject_unref(obj) qobject_unref_impl(QOBJECT(obj)) diff --git a/block.c b/block.c index 6161dbe3eb..f1e35c3c1e 100644 --- a/block.c +++ b/block.c @@ -5154,6 +5154,8 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) for (entry = qdict_first(bs->options); entry; entry = qdict_next(bs->options, entry)) { + QObject *val; + /* Exclude node-name references to children */ QLIST_FOREACH(child, &bs->children, next) { if (!strcmp(entry->key, child->name)) { @@ -5174,8 +5176,8 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) continue; } - qdict_put_obj(d, qdict_entry_key(entry), - qobject_ref(qdict_entry_value(entry))); + val = qdict_entry_value(entry); + qdict_put_obj(d, qdict_entry_key(entry), val ? qobject_ref(val) : NULL); found_any = true; } diff --git a/block/blkdebug.c b/block/blkdebug.c index 0759452925..062263f7e1 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -846,7 +846,8 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options) opts = qdict_new(); qdict_put_str(opts, "driver", "blkdebug"); - qdict_put(opts, "image", qobject_ref(bs->file->bs->full_open_options)); + qdict_put(opts, "image", bs->file->bs->full_open_options ? + qobject_ref(bs->file->bs->full_open_options) : NULL); for (e = qdict_first(options); e; e = qdict_next(options, e)) { if (strcmp(qdict_entry_key(e), "x-image")) { diff --git a/block/quorum.c b/block/quorum.c index 9152da8c58..96cd094ede 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1089,7 +1089,8 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options) children = qlist_new(); for (i = 0; i < s->num_children; i++) { qlist_append(children, - qobject_ref(s->children[i]->bs->full_open_options)); + s->children[i]->bs->full_open_options ? + qobject_ref(s->children[i]->bs->full_open_options) : NULL); } opts = qdict_new(); diff --git a/monitor.c b/monitor.c index eab2dc7b7b..be4bcd82a0 100644 --- a/monitor.c +++ b/monitor.c @@ -675,7 +675,7 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict) evstate = g_new(MonitorQAPIEventState, 1); evstate->event = event; - evstate->data = qobject_ref(data); + evstate->data = data ? qobject_ref(data) : NULL; evstate->qdict = NULL; evstate->timer = timer_new_ns(monitor_get_event_clock(), monitor_qapi_event_handler,