Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries()

[Max Reitz <mreitz@redhat.com>]

On 08.05.2015 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   include/qapi/qmp/qdict.h |  1 +
>   qobject/qdict.c          | 68 +++++++++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index d68f4eb..d20db94 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -70,6 +70,7 @@ void qdict_flatten(QDict *qdict);
>   
>   void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
>   void qdict_array_split(QDict *src, QList **dst);
> +int qdict_array_entries(QDict *src, const char *subqdict);
>   
>   void qdict_join(QDict *dest, QDict *src, bool overwrite);
>   
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index ea239f0..2fcb7fe 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -597,17 +597,18 @@ void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start)
>       }
>   }
>   
> -static bool qdict_has_prefixed_entries(const QDict *src, const char *start)
> +static int qdict_count_prefixed_entries(const QDict *src, const char *start)
>   {
>       const QDictEntry *entry;
> +    int count = 0;
>   
>       for (entry = qdict_first(src); entry; entry = qdict_next(src, entry)) {
>           if (strstart(entry->key, start, NULL)) {
> -            return true;
> +            count++;

On one hand a dumb question, but on the other hand we probably do need 
to consider it: What about overflows? I think we can avoid thinking 
about them by using size_t for all the counters.

>           }
>       }
>   
> -    return false;
> +    return count;
>   }
>   
>   /**
> @@ -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);
>   
>           // There may be either a single subordinate object (named "%u") or
>           // multiple objects (each with a key prefixed "%u."), but not both.
> @@ -667,6 +668,65 @@ void qdict_array_split(QDict *src, QList **dst)
>   }
>   
>   /**
> + * qdict_array_valid(): Returns the number of direct array entries if the

(As Eric said, should be "qdict_array_entries():")

> + * 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)
> +{
> +    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];
> +        size_t snprintf_ret;
> +
> +        snprintf_ret = snprintf(indexstr, slen, "%s%u", subqdict, i);
> +        assert(snprintf_ret < slen);
> +
> +        subqobj = qdict_get(src, indexstr);
> +
> +        snprintf_ret = snprintf(prefix, slen, "%s%u.", subqdict, i);
> +        assert(snprintf_ret < slen);
> +
> +        subqdict_entries = qdict_count_prefixed_entries(src, prefix);
> +
> +        /* There may be either a single subordinate object (named "%u") or
> +         * multiple objects (each with a key prefixed "%u."), but not both. */
> +        if (subqobj && subqdict_entries) {
> +            return -1;
> +        } else if (!subqobj && !subqdict_entries) {
> +            break;
> +        }
> +
> +        entries += subqdict_entries ? subqdict_entries : 1;
> +    }
> +
> +    /* Consider everything handled that isn't part of the given sub-QDict */
> +    for (entry = qdict_first(src); entry; entry = qdict_next(src, entry)) {
> +        if (!strstart(qdict_entry_key(entry), subqdict, NULL)) {
> +            entries++;
> +        }
> +    }
> +
> +    /* Anything left in the sub-QDict that wasn't handled? */
> +    if (qdict_size(src) != entries) {
> +        return -1;
> +    }
> +
> +    return i;

Again, possible overflow that can be avoided by returning size_t.

Max

> +}
> +
> +/**
>    * qdict_join(): Absorb the src QDict into the dest QDict, that is, move all
>    * elements from src to dest.
>    *