On 04/29/2016 02:50 AM, Markus Armbruster wrote: > Eric Blake <[email protected]> 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
>
>>
>> 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 <[email protected]>
>
> 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
