[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 25/28] qapi: Make input visitors detect unvis
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 25/28] qapi: Make input visitors detect unvisited list tails |
Date: |
Sun, 05 Mar 2017 09:06:07 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 03/03/2017 01:50 PM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> On 03/03/2017 06:32 AM, Markus Armbruster wrote:
>>>> Fix the design flaw demonstrated in the previous commit: new method
>>>> check_list() lets input visitors report that unvisited input remains
>>>> for a list, exactly like check_struct() lets them report that
>>>> unvisited input remains for a struct or union.
>>>>
>>>> Implement the method for the qobject input visitor (straightforward),
>>>> and the string input visitor (less so, due to the magic list syntax
>>>> there). The opts visitor's list magic is even more impenetrable, and
>>>> all I can do there today is a stub with a FIXME comment. No worse
>>>> than before.
>>>>
>>>> Signed-off-by: Markus Armbruster <address@hidden>
>>>> ---
>>>
>>> Didn't I already review this one?
>>>
>>> Ah, there's my R-b:
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07614.html
>>
>
>>>>
>>>> --- a/qapi/qobject-input-visitor.c
>>>> +++ b/qapi/qobject-input-visitor.c
>>>> @@ -51,7 +51,8 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
>>>> return container_of(v, QObjectInputVisitor, visitor);
>>>> }
>>>>
>>>> -static const char *full_name(QObjectInputVisitor *qiv, const char *name)
>>>> +static const char *full_name_nth(QObjectInputVisitor *qiv, const char
>>>> *name,
>>>> + int n)
>>>> {
>
> No function comment, so the _nth and int n are guesses on their meaning...
>
>
>>> If I'm reading this right, your use of n-- in the loop followed by the
>>> post-condition is to assert that QSLIST_FOREACH() iterated n times, but
>>> lets see what callers pass for n:
>>
>> At least @n times.
>
> Ah, as in 'use first available result' or 'iterate at least once', based
> on our callers, but could also mean 'iterate at least twice' for a
> caller that passes 2.
>
>
>>> the other passes 1. No other calls. Did we really need an integer,
>>> where we use n--, or would a bool have done as well?
>>
>> Since I actually use only 0 and 1, a bool would do, but would it make
>> the code simpler?
>
> I don't know that a bool would be any simpler,
>
>>
>>> At any rate, since I've already reviewed it once, you can add R-b, but
>>> we may want a followup to make it less confusing.
>>
>> Would renaming the function to full_name_but_n() help?
>
> Or even keep the name unchanged, but add function comments describing
> what 'n' means.
Makes sense. I'll do it on top to avoid delaying merge of this series
and the other stuff that depends on it.
- [Qemu-devel] [PATCH v4 12/28] qapi: Clean up after commit 3d344c2, (continued)
- [Qemu-devel] [PATCH v4 12/28] qapi: Clean up after commit 3d344c2, Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 15/28] qapi: Drop string input visitor method optional(), Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 27/28] qapi: Fix object input visit beyond end of list, Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 17/28] qom: Make object_property_set_qobject()'s input visitor strict, Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 26/28] tests: Cover input visit beyond end of list, Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 23/28] tests: Cover partial input visit of list, Markus Armbruster, 2017/03/03
- [Qemu-devel] [PATCH v4 25/28] qapi: Make input visitors detect unvisited list tails, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 24/28] test-qobject-input-visitor: Cover missing nested struct member, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 28/28] qapi: Improve qobject visitor documentation, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 21/28] test-string-input-visitor: Tear down existing test automatically, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 19/28] qapi: Drop unused non-strict qobject input visitor, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 02/28] libqtest: Work around a "QMP wants a newline" bug, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 22/28] test-string-input-visitor: Improve list coverage, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 14/28] qapi: Improve qobject input visitor error reporting, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 20/28] tests-qobject-input-strict: Merge into test-qobject-input-visitor, Markus Armbruster, 2017/03/03
[Qemu-devel] [PATCH v4 07/28] qmp: Clean up how we enforce capability negotiation, Markus Armbruster, 2017/03/03