[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries()
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries() |
Date: |
Fri, 08 May 2015 14:06:35 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 05/08/2015 11:21 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
Might want to include mention of what it will be used for in the commit
body.
> include/qapi/qmp/qdict.h | 1 +
> qobject/qdict.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 65 insertions(+), 4 deletions(-)
>
> @@ -646,7 +647,7 @@ void qdict_array_split(QDict *src, QList **dst)
> snprintf_ret = snprintf(prefix, 32, "%u.", i);
> assert(snprintf_ret < 32);
>
> - is_subqdict = qdict_has_prefixed_entries(src, prefix);
> + is_subqdict = qdict_count_prefixed_entries(src, prefix);
Good thing we require a C99 compiler, where 'bool = int' does the right
thing for integers > 1.
> /**
> + * 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 == "") is valid as an array, i.e. the length of the created 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)
Comment doesn't match function name.
> +{
> + const QDictEntry *entry;
> + unsigned i;
> + unsigned entries = 0;
> + size_t subqdict_len = strlen(subqdict);
> +
> + assert(!subqdict_len || subqdict[subqdict_len - 1] == '.');
> +
> + for (i = 0; i < UINT_MAX; i++) {
> + QObject *subqobj;
> + int subqdict_entries;
> + size_t slen = 32 + subqdict_len;
> + char indexstr[slen], prefix[slen];
And more dependence on a working C99 compiler, thanks to variable length
array (VLA).
> + size_t snprintf_ret;
> +
> + snprintf_ret = snprintf(indexstr, slen, "%s%u", subqdict, i);
> + assert(snprintf_ret < slen);
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature