From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46602) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UxNR1-00071N-Ax for qemu-devel@nongnu.org; Thu, 11 Jul 2013 16:25:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UxNQz-0005IU-Ty for qemu-devel@nongnu.org; Thu, 11 Jul 2013 16:25:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20840) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UxNQz-0005IP-Hm for qemu-devel@nongnu.org; Thu, 11 Jul 2013 16:25:49 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6BKPnBC026903 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 11 Jul 2013 16:25:49 -0400 Message-ID: <51DF14CC.9000609@redhat.com> Date: Thu, 11 Jul 2013 14:25:48 -0600 From: Eric Blake MIME-Version: 1.0 References: <1373363617-4723-1-git-send-email-kwolf@redhat.com> <1373363617-4723-10-git-send-email-kwolf@redhat.com> In-Reply-To: <1373363617-4723-10-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2AFODCDXQDXSFWMWTOVQL" Subject: Re: [Qemu-devel] [RFC PATCH 09/11] Implement qdict_flatten() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: armbru@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2AFODCDXQDXSFWMWTOVQL Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/09/2013 03:53 AM, Kevin Wolf wrote: Worth repeating this comment from the code into the commit message? > + * qdict_flatten(): For each nested QDict with key x, all fields with key y > + * are moved to this QDict and their key is renamed to "x.y". Otherwise, I had to read nearly the entire patch to learn what I was supposed to be reviewing. > Signed-off-by: Kevin Wolf > --- > include/qapi/qmp/qdict.h | 1 + > qobject/qdict.c | 47 ++++++++++++++++++++++++++++++++++++++++= +++++++ > 2 files changed, 48 insertions(+) >=20 > +++ b/qobject/qdict.c > @@ -476,3 +476,50 @@ static void qdict_destroy_obj(QObject *obj) > =20 > g_free(qdict); > } > + > +static void qdict_do_flatten(QDict *qdict, QDict *target, const char *= prefix) > +{ > + QObject *value; > + const QDictEntry *entry, *next; > + const char *new_key; > + bool delete; > + > + entry =3D qdict_first(qdict); > + > + while (entry !=3D NULL) { > + > + next =3D qdict_next(qdict, entry); > + value =3D qdict_entry_value(entry); > + new_key =3D NULL; > + delete =3D false; > + > + if (prefix) { > + qobject_incref(value); > + new_key =3D g_strdup_printf("%s.%s", prefix, entry->key); > + qdict_put_obj(target, new_key, value); You are calling this function with the same parameter for both qdict and target. Doesn't that mean you are modifying qdict while iterating it? Is that safe? [/me re-reads] - oh, you recursively call this function, and this modification of target happens _only_ if prefix is non-null, which happens only: > + delete =3D true; > + } > + > + if (qobject_type(value) =3D=3D QTYPE_QDICT) { > + qdict_do_flatten(qobject_to_qdict(value), target, > + new_key ? new_key : entry->key); when passing two different dicts into the function. Maybe add an assert(!prefix || qdict !=3D target). > + delete =3D true; > + } > + > + if (delete) { > + qdict_del(qdict, entry->key); > + } > + > + entry =3D next; > + } > +} > + > +/** > + * qdict_flatten(): For each nested QDict with key x, all fields with = key y > + * are moved to this QDict and their key is renamed to "x.y". > + */ > +void qdict_flatten(QObject *obj) > +{ > + QDict *qdict =3D qobject_to_qdict(obj); > + qdict_do_flatten(qdict, qdict, NULL); > +} >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2AFODCDXQDXSFWMWTOVQL Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR3xTMAAoJEKeha0olJ0NqS0UH/jOt9kXcYoIEG+4R1J+YgaWK DVOUKcf7fr9mpylaIRwNF7eYp95uj0+h6bKOWeRIl/rNhiwyMHi+XN5Bgi4SnJug u61e5Y6ccn+u8dFWf3COlBWrBT/C2Vw2JWyuNRRJxjXSzWCSftLK3u6M3uDvX9ez XFeBS/YxjSSUNPEXAw9XFLs2KKJYfDKEI6KutDGIXOoFSmKq0Xa7F9UgUoJUpV8h NfmL5HJMOZE6l/LMCX2zhLPgntUFOWghosdGlOdIA6LX19a2TZARqVgIBMVDoeBG hafwf6h3cvgZvZk2WO52md8bC1po4S67mkNrc+LYmuxWJJmLAd9SoNl3ba4uXk4= =fpU8 -----END PGP SIGNATURE----- ------enig2AFODCDXQDXSFWMWTOVQL--