[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/6] qapi: Remember alias definitions in qobject-input-visito
From: |
Kevin Wolf |
Subject: |
Re: [PATCH 2/6] qapi: Remember alias definitions in qobject-input-visitor |
Date: |
Thu, 11 Feb 2021 17:27:40 +0100 |
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