Re: [Qemu-devel] [PATCH v16 11/24] qmp-input: Refactor when list is advanced

[Eric Blake <eblake@redhat.com>]

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

On 04/29/2016 02:50 AM, Markus Armbruster wrote:
> Eric Blake <eblake@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 <eblake@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]