Re: [PATCH 2/6] qapi: Remember alias definitions in qobject-input-visitor

[Kevin Wolf <kwolf@redhat.com>]

Am 09.02.2021 um 13:57 hat Markus Armbruster geschrieben:
> Let me look at the actual code now Kevin reduced my confusion about the
> interface and the data structures.
> 
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > This makes qobject-input-visitor remember the currently valid aliases in
> > each StackObject. It doesn't actually allow using the aliases yet.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 115 insertions(+)

> > @@ -203,6 +229,38 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
> >      return qstring_get_str(qstr);
> >  }
> >  
> > +/*
> > + * Propagates aliases from the parent StackObject @src to its direct
> > + * child StackObject @dst, which is representing the child struct @name.
> 
> @name must equal dst->name, I think.  Drop the parameter?
> 
> > + *
> > + * Every alias whose source path begins with @name and which still
> > + * applies in @dst (i.e. it is either a wildcard alias or has at least
> > + * one more source path element) is propagated to @dst with the first
> 
> I'm not sure I get the parenthesis.  Perhaps the code will enlighten me.
> 
> > + * element (i.e. @name) removed from the source path.
> > + */
> > +static void propagate_aliases(StackObject *dst, StackObject *src,
> > +                              const char *name)
> > +{
> > +    InputVisitorAlias *a;
> > +
> > +    QSLIST_FOREACH(a, &src->aliases, next) {
> > +        if (!a->src[0] || strcmp(a->src[0], name)) {
> > +            continue;
> > +        }
> 
> We only look at the aliases that apply to @dst or below.  They do only
> when ->src[0] equals dst->name.  Makes sense.
> 
> > +        if (a->src[1] || !a->alias) {
> 
> If a->src[1], the alias applies below @dst, not to @dst.  To get it to
> the place where it applies, we first need to propagate to @dst.
> 
> Else, the alias applies to @dst.  If !a->alias, we're looking at a
> wildcard alias, i.e. all members of the object described by @dst are
> aliased.  Why do we need to propagate only wildcard aliases to @dst?

If it's not a wildcard alias and a->src[1] == NULL, then the alias
referred to @dst's name inside @src. It's not valid inside @dst any
more.

This is what the parenthesis above tried to tell you.

I've added another comment in the code to explain this case more
explicitly.

> > +            InputVisitorAlias *alias = g_new(InputVisitorAlias, 1);
> > +
> > +            *alias = (InputVisitorAlias) {
> > +                .alias      = a->alias,
> > +                .alias_so   = a->alias_so,
> > +                .src        = &a->src[1],
> > +            };
> > +
> > +            QSLIST_INSERT_HEAD(&dst->aliases, alias, next);
> > +        }
> > +    }
> > +}
> > +
> >  static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
> >                                              const char *name,
> >                                              QObject *obj, void *qapi)
> > @@ -226,6 +284,9 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
> >              g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
> >          }
> >          tos->h = h;
> > +        if (!QSLIST_EMPTY(&qiv->stack)) {
> > +            propagate_aliases(tos, QSLIST_FIRST(&qiv->stack), name);
> > +        }
> 
> What if QSLIST_EMPTY(&qiv->stack) and tos->aliases contains aliases that
> apply deeper?

tos->aliases doesn't contain any aliases, we only created it a few lines
above.

We would normally propagate aliases from the parent StackObject (which
is QSLIST_FIRST(&qiv->stack)), but if there is no parent StackObject,
then there can't be aliases to be inherited from the parent either.

Kevin