Eric Blake <ebl...@redhat.com> writes: > On 04/29/2016 02:50 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> In the QMP input visitor, visiting a list traverses two objects: >>> the QAPI GenericList of the caller (which gets advanced in >>> visit_next_list() regardless of this patch), and the QList input >>> that we are converting to QAPI. For consistency with QDict >>> visits, we want to consume elements from the input QList during >>> the visit_type_FOO() for the list element; that is, we want ALL >>> the code for consuming an input to live in qmp_input_get_object(), >>> rather than having it split according to whether we are visiting >>> a dict or a list. Making qmp_input_get_object() the common point >>> of consumption will make it easier for a later patch to refactor >>> visit_start_list() to cover the GenericList * head of a QAPI list, >>> and in turn will get rid of the 'first' flag (which lived in >>> qmp_input_next_list() pre-patch, and is hoisted to StackObject >>> by this patch). >>> >>> This patch is therefore shifting the post-condition use of >>> 'entry' from: >>> >>> start_list next_list type_ELT ... next_list type_ELT next_list >>> end_list >>> visit 1st elt last elt >>> entry NULL 1st elt 1st elt last elt last elt NULL gone >> >> You added a line "visit", but the introduction still only mentions >> 'entry'. Mildly confusing. If you have an idea on how to improve it, I >> can touch it up on commit. > > [probably word wrapped by my mailer, but how about] > > This patch is therefore altering the post-condition use of 'entry', > while keeping what gets visited unchanged, from: > > start_list next_list type_ELT ... next_list type_ELT next_list > end_list > visits 1st elt last elt > entry NULL 1st elt 1st elt last elt last elt NULL gone
Sold. >>> >>> where type_ELT() returns (entry ? entry : 1st elt) and next_list() steps >>> entry >>> >>> to this usage: >>> >>> start_list next_list type_ELT ... next_list type_ELT next_list >>> end_list >>> visit 1st elt last elt > > and another s/visit/visits/ here > >>> entry 1st elt 1nd elt 2nd elt last elt NULL NULL gone >>> >>> where type_ELT() steps entry and returns the old entry, and next_list() >>> leaves entry alone. >>> >>> Signed-off-by: Eric Blake <ebl...@redhat.com> >> >> Patch looks good. > > I added the visit line to make it a little easier to see that type_ELT > is still visiting the same thing, even though 'entry' ends up on a > different spot (easier than deciphering "type_ELT() ... returns the old > entry"). But if you don't think it adds anything, I'm also okay with > dropping the 'visit[s]' line and sticking to just the phrase after the > diagram for that explanation. Either way works for me, so I'm sticking to your later version, as amended here.