[libvirt] [PATCH 01/15] util: json: add helper to iterate and steal members of json array

[Eric Blake]

On 10/20/2016 10:24 AM, Peter Krempa wrote:
> Simplifies cases where JSON array members need to be transferred to a
> different structure.
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virjson.c       | 36 ++++++++++++++++++++++++++++++++++++
>  src/util/virjson.h       |  6 ++++++
>  3 files changed, 43 insertions(+)
> 

> 
> +/**
> + * virJSONValueArrayForeachSteal:
> + * @array: array to iterate
> + * @cb: callback called on every member of the array
> + * @opaque: custom data for the callback
> + *
> + * Iterates members of the array and calls the callback on every single member.
> + * By returning success, the array claims ownership of given array element and

s/array claims/callback claims/

"returning success" - is that 0?

> + * is responsible for freeing it.
> + *
> + * Returns 0 if all members were successfully stolen by the callback; -1
> + * otherwise. The rest of the members stays in posession of the array.

s/stays/stay/
s/posession/the possession/

Does the iteration stop at the first callback that returns non-zero?  Do
we want something a bit more complex, where a negative return aborts
right away, returning 0 means the callback claimed possession but keep
iterating, and where a positive return means the callback did not claim
possession but wants to keep iterating?

> + */
> +int
> +virJSONValueArrayForeachSteal(virJSONValuePtr array,
> +                              virJSONArrayIteratorFunc cb,
> +                              void *opaque)
> +{
> +    ssize_t i;
> +
> +    if (array->type != VIR_JSON_TYPE_ARRAY)
> +        return -1;
> +
> +    for (i = array->data.array.nvalues - 1; i >= 0; i--) {

Does order of iteration matter?  JSON arrays (can be) order-dependent,
unlike dicts that are order-agnostic.  All other clients of
data.array.nvalues iterate forward, not backward, so we ought to do
likewise.

> +        if (cb(i, array->data.array.values[i], opaque) < 0)
> +            break;

Okay, you are handling < 0 as failure and end iteration, and treating
all non-negative as successfully claiming the element.  But is that
enough power for all uses of this interface?  See my thoughts above
about distinguishing between 0 and positive returns.  Then again, if you
do that, you'd have to memmove remaining elements to close gaps.

> +
> +        array->data.array.values[i] = NULL;
> +    }
> +
> +    array->data.array.nvalues = i + 1;

I see why you iterate backwards; you don't have to memmove elements that
aren't stolen.  But I'm still not convinced it is the best thing to be
doing.

> +
> +    return i < 0 ? 0 : -1;
> +}

The interface sounds like it will be useful, but I want to make sure it
doesn't lock us into design issues that will haunt us later on.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161020/5e2eee66/attachment-0001.sig>