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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list