Am 28.01.2021 um 08:43 hat Markus Armbruster geschrieben:
> Kevin Wolf <kw...@redhat.com> writes:
>
> > Am 27.01.2021 um 14:56 hat Markus Armbruster geschrieben:
> >> Kevin Wolf <kw...@redhat.com> writes:
> >>
> >> > Instead of counting how many elements from the top of the stack we need
> >> > to ignore until we find the thing we're interested in, we can just
> >> > directly pass the StackObject pointer because all callers already know
> >> > it.
> >> >
> >> > We only need a different way now to tell if we want to know the name of
> >> > something contained in the given StackObject or of the StackObject
> >> > itself. Passing name = NULL is the obvious way to request the latter.
> >> >
> >> > This simplifies the interface and makes it easier to use in cases where
> >> > we have the StackObject, but don't know how many steps down the stack it
> >> > is.
> >> >
> >> > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> >> > ---
> >> > qapi/qobject-input-visitor.c | 38 ++++++++++++++++++------------------
> >> > 1 file changed, 19 insertions(+), 19 deletions(-)
> >> >
> >> > diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> >> > index a00ac32682..1415561828 100644
> >> > --- a/qapi/qobject-input-visitor.c
> >> > +++ b/qapi/qobject-input-visitor.c
> >> > @@ -87,20 +87,16 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
> >> > }
> >> >
> >> > /*
> >> > - * Find the full name of something @qiv is currently visiting.
> >> > - * @qiv is visiting something named @name in the stack of containers
> >> > - * @qiv->stack.
> >> > - * If @n is zero, return its full name.
> >> > - * If @n is positive, return the full name of the @n-th container
> >> > - * counting from the top. The stack of containers must have at least
> >> > - * @n elements.
> >> > - * The returned string is valid until the next full_name_nth(@v) or
> >> > - * destruction of @v.
> >> > + * Find the full name of something named @name in @so which @qiv is
> >> > + * currently visiting. If @name is NULL, find the full name of @so
> >> > + * itself.
> >> > + *
> >> > + * The returned string is valid until the next full_name_so(@qiv) or
> >> > + * destruction of @qiv.
> >>
> >> How can this distinguish between a list and its member?
> >>
> >> Before the patch:
> >>
> >> * list member: n = 0, name = NULL
> >> * list: n = 1, name = NULL
> >
> > Oh. These two lines were more helpful than the whole function comment
> > before this patch (which doesn't talk about name = NULL at all).
>
> See, I can write impenetrable comments with the best of them!
>
> The spot that talks about @name is in visitor.h:
>
> * The @name parameter of visit_type_FOO() describes the relation
> * between this QAPI value and its parent container. When visiting
> * the root of a tree, @name is ignored; when visiting a member of an
> * object, @name is the key associated with the value; when visiting a
> * member of a list, @name is NULL; and when visiting the member of an
> * alternate, @name should equal the name used for visiting the
> * alternate.
>
> Many contracts in the same file refer back to it like this:
>
> * @name expresses the relationship of this object to its parent
> * container; see the general description of @name above.
>
> The contract here doesn't.
>
> >> Afterwards?
> >>
> >> Checking... yes, regression. Test case:
> >>
> >> {"execute": "blockdev-add", "arguments": {"driver": "file",
> >> "node-name": "blk0", "filename": "tmp.img"}}
> >> {"return": {}}
> >> {"execute": "blockdev-add", "arguments": {"driver": "blkdebug",
> >> "node-name": "blk1", "image": "blk0", "take-child-perms": [0]}}
> >> {"error": {"class": "GenericError", "desc": "Invalid parameter type
> >> for 'take-child-perms', expected: string"}}
> >>
> >> The second command's reply changes from
> >>
> >> {"error": {"class": "GenericError", "desc": "Invalid parameter type
> >> for 'take-child-perms[0]', expected: string"}}
> >>
> >> to
> >>
> >> {"error": {"class": "GenericError", "desc": "Invalid parameter type
> >> for 'take-child-perms', expected: string"}}
> >>
> >> The idea of using @so instead of @n may be salvagable.
> >
> > I can always add a bool parameter that tells (independently from @name)
> > whether we want the name of a member or of the container.
> >
> > Though do we really need the name of the container anywhere? The n = 1
> > case exists in qobject_input_check_list(), but is this a case that can
> > fail? The pattern how lists are intended to be visited seems to be
> > calling visit_next_list() until it returns NULL.
>
> Yes, the generated visitors always exhaust the list. But virtual walks
> needn't.
Ah. Okay, I'll add the bool parameter then.
Kevin
