Re: [Qemu-devel] [PATCH 2/3] qdict: Extract non-QDicts in qdict_array_split()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

On 02/21/2014 11:11 AM, Max Reitz wrote:
> Currently, qdict_array_split() only splits off entries with a key prefix
> of "%u.", packing them into a new QDict. This patch makes it support
> entries with the plain key "%u" as well, directly putting them into the
> new QList without creating a QDict.
> 
> If there is both an entry with a key of "%u" and other entries with keys
> prefixed "%u." (for the same index), the function simply terminates.
> 
> To do this, this patch also adds a static function which tests whether a
> given QDict contains any keys with the given prefix. This is used to test
> whether entries with a key prefixed "%u." do exist in the source QDict
> without modifying it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qobject/qdict.c | 60 +++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 14 deletions(-)
> 

> +static bool qdict_has_prefixed_entries(const QDict *src, const char *start)
> +{
> +    const QDictEntry *entry;
> +
> +    for (entry = qdict_first(src); entry; entry = qdict_next(src, entry)) {
> +        if (strstart(entry->key, start, NULL)) {
> +            return true;

Note that if called with start="1" and the dict contains a key "10",
this would return true.

> @@ -617,19 +632,36 @@ void qdict_array_split(QDict *src, QList **dst)
>      *dst = qlist_new();
>  
>      for (i = 0; i < UINT_MAX; i++) {
> +        QObject *subqobj;
> +        bool is_subqdict;
>          QDict *subqdict;
> -        char prefix[32];
> +        char indexstr[32], prefix[32];
>          size_t snprintf_ret;
>  
> +        snprintf_ret = snprintf(indexstr, 32, "%u", i);
> +        assert(snprintf_ret < 32);

This assertion is redundant...

> +
> +        subqobj = qdict_get(src, indexstr);
> +
>          snprintf_ret = snprintf(prefix, 32, "%u.", i);
>          assert(snprintf_ret < 32);

...if this assertion about a longer string holds true.  But it doesn't
hurt my feelings to leave it in.

>  
> -        qdict_extract_subqdict(src, &subqdict, prefix);
> -        if (!qdict_size(subqdict)) {
> -            QDECREF(subqdict);
> +        is_subqdict = qdict_has_prefixed_entries(src, prefix);

Thankfully you always test a prefix with a trailing '.', so this is not
a problem in your usage.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]