Re: [Qemu-devel] [PATCH v7 29/31] qapi: Simplify semantics of visit_next_list()

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Dec 07, 2015 at 08:55:19PM -0700, Eric Blake wrote:
> We have two uses of list visits in the entire code base: one in
> spapr_drc (which completely avoids visit_next_list(), feeding in
> integers from a different source than uint8List), and one in
> qapi-visit.py (that is, all other list visitors are generated
> in qapi-visit.c, and share the same paradigm based on a qapi
> FooList type).  What's more, the semantics of the list visit are
> somewhat baroque, with the following pseudocode when FooList is
> used:
> 
> start()
> prev = head
> while (cur = next(prev)) {
>     visit(cur)
>     prev = &cur
> }
> 
> Note that these semantics (advance before visit) requires that
> the first call to next() return the list head, while all other
> calls return the next element of the list; that is, every visitor
> implementation is required to track extra state to decide whether
> to return the input as-is, or to advance.  It also requires an
> argument of 'GenericList **' to next(), solely because the first
> iteration might need to modify the caller's GenericList head, so
> that all other calls have to do a layer of dereferencing.
> 
> We can greatly simplify things by hoisting the special case
> into the start() routine, and flipping the order in the loop
> to visit before advance:
> 
> start(head)
> element = *head
> while (element) {
>     visit(element)
>     element = next(element)
> }
> 
> With the simpler semantics, visitors have less state to track,
> the argument to next() is reduced to 'GenericList *', and it
> also becomes obvious whether an input visitor is allocating a
> FooList during visit_start_list() (rather than the old way of
> not knowing if an allocation happened until the first
> visit_next_list()).
> 
> The spapr_drc case requires that visit_start_list() has to pay
> attention to whether visit_next_list() will even be used to
> visit a FooList qapi struct; this is done by passing NULL for
> list, similarly to how NULL is passed to visit_start_struct()
> when a qapi type is not used in those visits.  It was easy to
> provide these semantics for qmp-output and dealloc visitors,
> and a bit harder for qmp-input (it required hoisting the
> advance of the current qlist entry out of qmp_input_next_list()
> into qmp_input_get_object()).  But it turned out that the
> string and opts visitors munge enough state during
> visit_next_list() to make those conversions simpler if they
> require a GenericList visit for now; an assertion will remind
> us to adjust things if we need the semantics in the future.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

For the spapr change:

Acked-by: David Gibson <david@gibson.dropbear.id.au>
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]