From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48898) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu3mR-0005pr-MV for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:22:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vu3mL-00065M-Lr for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:22:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:26300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu3mL-00064I-E2 for qemu-devel@nongnu.org; Fri, 20 Dec 2013 12:22:25 -0500 Message-ID: <52B47D0E.50906@redhat.com> Date: Fri, 20 Dec 2013 18:23:26 +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> <52B47C1F.3080006@redhat.com> In-Reply-To: <52B47C1F.3080006@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 18:19, Max Reitz wrote: > 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. On second thought, that would be even more wrong. I'll leave the else branch and drop the qobject_incref() and qdict_put_obj() above the condition. Max > >>> + qdict_put_obj(target, new_key, value); >>> + } >>> + >>> + g_free(new_key); >>> + } >