From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47915) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu3if-0004Ov-Nr for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:18:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vu3iV-0004u0-8O for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:18:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:27306) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu3iU-0004tu-W1 for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:18:27 -0500 Message-ID: <52B47C1F.3080006@redhat.com> Date: Fri, 20 Dec 2013 18:19:27 +0100 From: Max Reitz MIME-Version: 1.0 References: <1387482443-10633-1-git-send-email-mreitz@redhat.com> <1387482443-10633-5-git-send-email-mreitz@redhat.com> <20131220094613.GB27021@stefanha-thinkpad.redhat.com> In-Reply-To: <20131220094613.GB27021@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 04/22] qapi: extend qdict_flatten() for QLists List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi On 20.12.2013 10:46, Stefan Hajnoczi wrote: > On Thu, Dec 19, 2013 at 08:47:05PM +0100, Max Reitz wrote: >> Reversing qdict_array_split(), qdict_flatten() should flatten QLists as >> well by interpreting them as QDicts where every entry's key is its >> index. >> >> This allows bringing QDicts with QLists from QMP commands to the same >> form as they would be given as command-line options, thereby allowing >> them to be parsed the same way. >> >> Signed-off-by: Max Reitz >> --- >> qobject/qdict.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 51 insertions(+), 6 deletions(-) > Please add a tests/check-qdict.c test case. > >> diff --git a/qobject/qdict.c b/qobject/qdict.c >> index fca1902..7b6b08a 100644 >> --- a/qobject/qdict.c >> +++ b/qobject/qdict.c >> @@ -477,7 +477,46 @@ static void qdict_destroy_obj(QObject *obj) >> g_free(qdict); >> } >> >> -static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix) >> +static void qdict_flatten_qdict(QDict *qdict, QDict *target, >> + const char *prefix); >> + >> +static void qdict_flatten_qlist(QList *qlist, QDict *target, const char *prefix) >> +{ >> + QObject *value; >> + const QListEntry *entry; >> + char *new_key; >> + int i; >> + >> + /* This function is never called with prefix == NULL, i.e., it is always >> + * called from within qdict_flatten_q(list|dict)(). Therefore, it does not >> + * need to remove list entries during the iteration (the whole list will be >> + * deleted eventually anyway from qdict_flatten_qdict()). */ >> + assert(prefix); >> + >> + entry = qlist_first(qlist); >> + >> + for (i = 0; entry; entry = qlist_next(entry), i++) { >> + value = qlist_entry_obj(entry); >> + >> + qobject_incref(value); >> + new_key = g_strdup_printf("%s.%i", prefix, i); >> + qdict_put_obj(target, new_key, value); > It seems this operation is clobbered by what follows and should be > deleted. Is the incref also superfluous or... > >> + >> + if (qobject_type(value) == QTYPE_QDICT) { >> + qdict_flatten_qdict(qobject_to_qdict(value), target, new_key); >> + } else if (qobject_type(value) == QTYPE_QLIST) { >> + qdict_flatten_qlist(qobject_to_qlist(value), target, new_key); >> + } else { >> + /* All other types are moved to the target unchanged. */ >> + qobject_incref(value); > ...should this one be deleted? Oh great, I've been fixing working code, then. *g* I added the else branch in v5, I guess, it was correct not to have it (however, the condition in the else if was missing in v4). I'll drop it again, thanks. >> + qdict_put_obj(target, new_key, value); >> + } >> + >> + g_free(new_key); >> + }