Eric Blake <ebl...@redhat.com> writes: > 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.
Makes sense. I'll do it on top to avoid delaying merge of this series and the other stuff that depends on it.
