On 03/03/2017 01:50 PM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com> >>> --- >> >> 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
