Kevin Wolf <kw...@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 <kw...@redhat.com> > --- > qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++++++++++++ > 1 file changed, 115 insertions(+) > > diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c > index 23843b242e..a00ac32682 100644 > --- a/qapi/qobject-input-visitor.c > +++ b/qapi/qobject-input-visitor.c > @@ -29,6 +29,29 @@ > #include "qemu/cutils.h" > #include "qemu/option.h" > > +typedef struct InputVisitorAlias { > + /* Alias name. NULL for any (wildcard alias). */ > + const char *alias; > + > + /* > + * NULL terminated array representing a path. > + * Must contain at least one non-NULL element if alias is not NULL.
What does .alias = NULL, .src[] empty mean? The previous patch's contract for visit_define_alias(): /* * Defines a new alias rule. * * If @alias is non-NULL, the member named @alias in the external * representation of the current struct is defined as an alias for the * member described by @source. * * If @alias is NULL, all members of the struct described by @source are * considered to have alias members with the same key in the current * struct. * * @source is a NULL-terminated array of names that describe the path to * a member, starting from the currently visited struct. * * The alias stays valid until the current alias scope ends. * visit_start/end_struct() implicitly start/end an alias scope. * Additionally, visit_start/end_alias_scope() can be used to explicitly * create a nested alias scope. */ If I read this correctly, then empty .src[] denotes "the current struct", and therefore .alias = NULL, .src[] empty means "all members of the current struct are considered to have alias members with the same key in the current struct". Which is be a complicated way to accomplish nothing. Am I confused? > + */ > + const char **src; > + > + /* StackObject in which the alias should be looked for */ > + struct StackObject *alias_so; Clear as mud. Is it "the current struct"? If not, what else? Perhaps an example would help me understand. > + > + /* > + * The alias remains valid as long as the containing StackObject has What's "the containing StackObject"? I guess it's the one that has this InputVisitorAlias in .aliases. > + * StackObject.alias_scope_nesting >= InputVisitorAlias.scope_nesting > + * or until the whole StackObject is removed. > + */ > + int scope_nesting; > + > + QSLIST_ENTRY(InputVisitorAlias) next; > +} InputVisitorAlias; > + > typedef struct StackObject { > const char *name; /* Name of @obj in its parent, if any */ > QObject *obj; /* QDict or QList being visited */ > @@ -38,6 +61,9 @@ typedef struct StackObject { > const QListEntry *entry; /* If @obj is QList: unvisited tail */ > unsigned index; /* If @obj is QList: list index of @entry */ > > + QSLIST_HEAD(, InputVisitorAlias) aliases; > + int alias_scope_nesting; /* Increase on scope start, decrease on end > */ I get what the comment means, but imperative mood is odd for a variable. "Number of open scopes", perhaps? > + > QSLIST_ENTRY(StackObject) node; /* parent */ > } StackObject; > I'm afraid I'm too confused about the interface (previous patch) and the data structures to continue review with reasonable efficiency. I don't mean to imply either is bad! I'm just confused, that's all. [...]