[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 06/19] qmp-input: Don't consume input when c
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v14 06/19] qmp-input: Don't consume input when checking has_member |
Date: |
Fri, 15 Apr 2016 17:28:44 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 04/13/2016 10:06 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> Commit e8316d7 mistakenly passed consume=true
>>
>> in qmp_input_optional(), right?
>
> yes
>
>>
>>> when checking if
>>> an optional member was present, but the mistake was silently
>>> ignored since the code happily let us extract a member more than
>>> once. Tighten up the input visitor to ensure that a member is
>>> consumed exactly once.
>
> [1] by fixing qmp_input_optional() to pass consume=false
>
>>> To keep the testsuite happy in the case
>>> of incomplete input, we have to check whether a member exists
>>> in the dictionary before trying to remove it.
>>
>> Sure this is only for the testsuite's benefit?
>
> The testsuite was the only client that failed under the tighter
> semantics; but the better semantics allow later patches to further
> improve the code while guaranteeing that clients remain sane.
Do we know that non-testsuite code can't fail for some input?
>>
>> You fix commit e8316d7's incorrect consume=true, don't you? Recommend
>> to mention that explicitly.
>
> I thought I did, but I can add wording [1] along those lines.
- Re: [Qemu-devel] [PATCH v14 05/19] qmp-input: Clean up stack handling, (continued)
[Qemu-devel] [PATCH v14 09/19] tests: Add check-qnull, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 06/19] qmp-input: Don't consume input when checking has_member, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 11/19] qmp: Support explicit null during visits, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classification, Eric Blake, 2016/04/08