From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32820) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YrotA-0004FR-T1 for qemu-devel@nongnu.org; Mon, 11 May 2015 10:41:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yrot5-0002zF-01 for qemu-devel@nongnu.org; Mon, 11 May 2015 10:41:00 -0400 Date: Mon, 11 May 2015 16:40:44 +0200 From: Kevin Wolf Message-ID: <20150511144044.GH4962@noname.redhat.com> References: <1431105726-3682-1-git-send-email-kwolf@redhat.com> <1431105726-3682-2-git-send-email-kwolf@redhat.com> <554D174B.4010200@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6sX45UoQRIJXqkqR" Content-Disposition: inline In-Reply-To: <554D174B.4010200@redhat.com> Subject: Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: mreitz@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, armbru@redhat.com --6sX45UoQRIJXqkqR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 08.05.2015 um 22:06 hat Eric Blake geschrieben: > On 05/08/2015 11:21 AM, Kevin Wolf wrote: > > Signed-off-by: Kevin Wolf > > --- >=20 > Might want to include mention of what it will be used for in the commit > body. You're right. This is the new commit message: This counts the entries in a flattened array in a QDict without actually splitting the QDict into a QList. bdrv_open_image() doesn't take a QList, but rather a QDict and a key prefix string, so this is more convenient for block drivers which have a dynamically sized list of child nodes (e.g. Quorum) and are to be converted to using bdrv_open_image() as the standard interface for opening child nodes. > > include/qapi/qmp/qdict.h | 1 + > > qobject/qdict.c | 68 ++++++++++++++++++++++++++++++++++++++++= +++++--- > > 2 files changed, 65 insertions(+), 4 deletions(-) > >=20 >=20 > > @@ -646,7 +647,7 @@ void qdict_array_split(QDict *src, QList **dst) > > snprintf_ret =3D snprintf(prefix, 32, "%u.", i); > > assert(snprintf_ret < 32); > > =20 > > - is_subqdict =3D qdict_has_prefixed_entries(src, prefix); > > + is_subqdict =3D qdict_count_prefixed_entries(src, prefix); >=20 > Good thing we require a C99 compiler, where 'bool =3D int' does the right > thing for integers > 1. >=20 > > /** > > + * qdict_array_valid(): Returns the number of direct array entries if = the > > + * sub-QDict of src specified by the prefix in subqdict (or src itself= for > > + * prefix =3D=3D "") is valid as an array, i.e. the length of the crea= ted list if > > + * the sub-QDict would become empty after calling qdict_array_split() = on it. If > > + * the array is not valid, -1 is returned. > > + */ > > +int qdict_array_entries(QDict *src, const char *subqdict) >=20 > Comment doesn't match function name. Thanks, fixed. > > +{ > > + const QDictEntry *entry; > > + unsigned i; > > + unsigned entries =3D 0; > > + size_t subqdict_len =3D strlen(subqdict); > > + > > + assert(!subqdict_len || subqdict[subqdict_len - 1] =3D=3D '.'); > > + > > + for (i =3D 0; i < UINT_MAX; i++) { > > + QObject *subqobj; > > + int subqdict_entries; > > + size_t slen =3D 32 + subqdict_len; > > + char indexstr[slen], prefix[slen]; >=20 > And more dependence on a working C99 compiler, thanks to variable length > array (VLA). >=20 > > + size_t snprintf_ret; > > + > > + snprintf_ret =3D snprintf(indexstr, slen, "%s%u", subqdict, i); > > + assert(snprintf_ret < slen); >=20 > Since gcc may compile the allocation of indexstr into a malloc() > anyways, would it be any simpler to just use g_strdup_printf() directly, > instead of futzing around with VLA and snprintf() ourselves? It might > mean less code, as some of the error checking is taken care of on your > behalf. This code parallels the code in qdict_array_split(), which looks almos the same, except that the latter doesn't support subqict and therefore has a fixed-size array rather than a VLA. If you think that g_strdup_printf() is preferable, I would do that on top of this series and change both functions. Kevin --6sX45UoQRIJXqkqR Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVUL9sAAoJEH8JsnLIjy/WBV8QAKnT38z06dpAeG5vl9L0qL6r UuR9xJMUwBTuzWveNmqCKZ72XufjEJrGNXNnzFsWoNUIYxL6C5s/iDjACL+YkopO uU5/J3tzT+ciSunw9kX8shQ07bMM0/zBV5gEGK7PLHVF6zFRKJi1tyGRhYK9747Z 0PUsLWpqgZoK1RWAoo2u9I/An4cHMUbt0jhqEI8hebS8eq76+ksmQZzYyKm5FcRO APN+8iNmspePkEoeAbwWVphLgcAO9iDMU0d7EquyPo0U2qs/Lz+yLGrNBprb3dqt XAWwCIrZm6D8uRJrM2SpWZ+t7s90fFL7ULf4vi1tpOO68CdIf73eiPt8BRA5p4P7 RsBhqIs4XC80ewMEabL83ORed3yaUiCj/6HZFSNZ3zL/rsMUnJkNt2klDBFNOoeM YCj9txciD+NdmAgwQ2XSORsB8QG5LiaQ85QfPTeVCwMP9hIhK9YKrxmfjEXpMdgq IhX3B7q2MM7OetsrI1zeUIWtUEXmxyQtBGKBWkLt5CLmXNW8ckv6+3rfiIMJSsuL tPIZHCU6mEZQ3IanjL6gIcQ0y78JeFhhQSpJWJZRItb5Gjt2v/8v1dVtlpMRnGBI NU9YFob8NKCqIcoQIqnTu2J02PkTwWTeGQcuqKAQBjjGgNrAL70RFi3dSqEPStIJ dhWZvOoTsbG4q9LkETTT =7sGM -----END PGP SIGNATURE----- --6sX45UoQRIJXqkqR--