[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v16 11/24] qmp-input: Refactor when list is adva
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v16 11/24] qmp-input: Refactor when list is advanced |
Date: |
Fri, 29 Apr 2016 15:03:04 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 04/29/2016 02:50 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> 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 <address@hidden>
>>
>> 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.
- Re: [Qemu-devel] [PATCH v16 02/24] qapi: Guarantee NULL obj on input visitor callback error, (continued)
[Qemu-devel] [PATCH v16 17/24] qmp: Add qmp_output_visitor_reset(), Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 05/24] qapi: Consolidate QMP input visitor creation, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 09/24] qom: Wrap prop visit in visit_start_struct, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 11/24] qmp-input: Refactor when list is advanced, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 07/24] qmp-input: Don't consume input when checking has_member, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 10/24] qmp-input: Require struct push to visit members of top dict, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 01/24] qapi-visit: Add visitor.type classification, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 06/24] qapi: Use strict QMP input visitor in more places, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 13/24] tests: Add check-qnull, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 14/24] qapi: Add visit_type_null() visitor, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 15/24] qmp: Support explicit null during visits, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 18/24] qmp: Tighten output visitor rules, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 24/24] qapi: Change visit_type_FOO() to no longer return partial objects, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v16 19/24] qapi: Split visit_end_struct() into pieces, Eric Blake, 2016/04/28