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 > >> >> 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature