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(+)
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.
+ */
+ const char **src;
+
+ /* StackObject in which the alias should be looked for */
+ struct StackObject *alias_so;
+
+ /*
+ * The alias remains valid as long as the containing StackObject has
+ * 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 */
+
QSLIST_ENTRY(StackObject) node; /* parent */
} StackObject;
@@ -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.
+ *
+ * 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
+ * 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;
+ }
+ if (a->src[1] || !a->alias) {
+ 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);
+ }
} else {
assert(qlist);
tos->entry = qlist_first(qlist);
@@ -257,10 +318,17 @@ static bool qobject_input_check_struct(Visitor *v, Error **errp)
static void qobject_input_stack_object_free(StackObject *tos)
{
+ InputVisitorAlias *a;
+
if (tos->h) {
g_hash_table_unref(tos->h);
}
+ while ((a = QSLIST_FIRST(&tos->aliases))) {
+ QSLIST_REMOVE_HEAD(&tos->aliases, next);
+ g_free(a);
+ }
+
g_free(tos);
}
@@ -274,6 +342,50 @@ static void qobject_input_pop(Visitor *v, void **obj)
qobject_input_stack_object_free(tos);
}
+static void qobject_input_start_alias_scope(Visitor *v)
+{
+ QObjectInputVisitor *qiv = to_qiv(v);
+ StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+ tos->alias_scope_nesting++;
+}
+
+static void qobject_input_end_alias_scope(Visitor *v)
+{
+ QObjectInputVisitor *qiv = to_qiv(v);
+ StackObject *tos = QSLIST_FIRST(&qiv->stack);
+ InputVisitorAlias *a, *next;
+
+ assert(tos->alias_scope_nesting > 0);
+ tos->alias_scope_nesting--;
+
+ QSLIST_FOREACH_SAFE(a, &tos->aliases, next, next) {
+ if (a->scope_nesting > tos->alias_scope_nesting) {
+ QSLIST_REMOVE(&tos->aliases, a, InputVisitorAlias, next);
+ g_free(a);
+ }
+ }
+}
+
+static void qobject_input_define_alias(Visitor *v, const char *alias_name,
+ const char **source)
+{
+ QObjectInputVisitor *qiv = to_qiv(v);
+ StackObject *tos = QSLIST_FIRST(&qiv->stack);
+ InputVisitorAlias *alias = g_new(InputVisitorAlias, 1);
+
+ /* The source path can only be empty for wildcard aliases */
+ assert(source[0] || !alias_name);
+
+ *alias = (InputVisitorAlias) {
+ .alias = alias_name,
+ .alias_so = tos,
+ .src = source,
+ };
+
+ QSLIST_INSERT_HEAD(&tos->aliases, alias, next);
+}
+
static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj,
size_t size, Error **errp)
{
@@ -696,6 +808,9 @@ static QObjectInputVisitor *qobject_input_visitor_base_new(QObject *obj)
v->visitor.end_list = qobject_input_end_list;
v->visitor.start_alternate = qobject_input_start_alternate;
v->visitor.optional = qobject_input_optional;
+ v->visitor.define_alias = qobject_input_define_alias;
+ v->visitor.start_alias_scope = qobject_input_start_alias_scope;
+ v->visitor.end_alias_scope = qobject_input_end_alias_scope;
v->visitor.free = qobject_input_free;
v->root = qobject_ref(obj);
--
2.28.0
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(+)
>
> 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.
[...]
Am 27.01.2021 um 14:06 hat Markus Armbruster geschrieben:
> 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(+)
> >
> > 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?
Indeed, this doesn't make sense when @alias_so is the currently
processed StackObject. I guess qobject_input_define_alias() should just
forbid this case.
But see below for the relevant case.
> > + */
> > + 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.
It is the object where the alias was defined.
.alias = NULL, .src[] empty happens after propagating the alias to a
child object, i.e. alias_so is different from the current StackObject.
Let's take a simple SocketAddressLegacy object as an example. Without
aliases, it might look like this:
{ 'type': 'inet',
'data': { 'host': 'localhost', 'port': 1234 } }
We want to eliminate 'data' from the external representation with an
alias, so we map everything in 'data' to the top level. So the actual
external representation that we want to parse in the example is this:
{ 'type': 'inet', 'host': 'localhost', 'port': 1234 }
For the implementation this alias means that while visiting the top
level SocketAddressLegacy object (with StackObject A) we define an
InputVisitorAlias like this:
{
.alias = NULL,
.src = ['data', NULL],
.alias_so = A,
}
When we proceed to visit the 'data' member, we call start_struct, which
creates a new StackObject B. The alias is propagated, stripping 'data'
from the start of .src:
{
.alias = NULL,
.src = [NULL],
.alias_so = A, /* This still refers to A, not B! */
}
Next we're parsing the members of InetSocketAddress (the type of
'data'). The visitor wants to visit 'host' now, but it's not present in
the QDict to parse (StackObject.obj, which is actually an empty QDict in
this case because 'data' wasn't given at all).
So what happens is that it looks at aliases that could provide a value
for this key. Its alias list contains the alias=NULL,src=[NULL] item,
which makes it check if 'host' exists in .alias_so (which is the
StackObject that belongs to the top level SocketAddressLegacy) instead,
and indeed we have a 'host' key there, so we can take the value from
there.
Does this make the mechanism a bit clearer?
> > +
> > + /*
> > + * 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.
Yes.
> > + * 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?
Works for me.
> > +
> > 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.
I hope the above helps to resolve the confusion. Feel free to come back
with more questions or ping me on IRC if necessary.
Kevin
Kevin Wolf <kwolf@redhat.com> writes:
> Am 27.01.2021 um 14:06 hat Markus Armbruster geschrieben:
>> 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(+)
>> >
>> > 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?
>
> Indeed, this doesn't make sense when @alias_so is the currently
> processed StackObject. I guess qobject_input_define_alias() should just
> forbid this case.
Yes, please.
> But see below for the relevant case.
>
>> > + */
>> > + 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.
>
> It is the object where the alias was defined.
>
> .alias = NULL, .src[] empty happens after propagating the alias to a
> child object, i.e. alias_so is different from the current StackObject.
>
> Let's take a simple SocketAddressLegacy object as an example. Without
> aliases, it might look like this:
>
> { 'type': 'inet',
> 'data': { 'host': 'localhost', 'port': 1234 } }
>
> We want to eliminate 'data' from the external representation with an
> alias, so we map everything in 'data' to the top level. So the actual
> external representation that we want to parse in the example is this:
>
> { 'type': 'inet', 'host': 'localhost', 'port': 1234 }
>
> For the implementation this alias means that while visiting the top
> level SocketAddressLegacy object (with StackObject A) we define an
> InputVisitorAlias like this:
>
> {
> .alias = NULL,
Make all members of ...
> .src = ['data', NULL],
... the object .data also available in ...
> .alias_so = A,
the object corresponding to A, which is the object we're visiting right
now (A is on top of the stack).
Correct?
> }
>
> When we proceed to visit the 'data' member, we call start_struct, which
> creates a new StackObject B. The alias is propagated, stripping 'data'
> from the start of .src:
>
> {
> .alias = NULL,
Make all members of ...
> .src = [NULL],
... the current object also available in ...
> .alias_so = A, /* This still refers to A, not B! */
the object corresponding to A, which is the object containing the one
we're visiting right now (A right below the top of the stack).
Correct?
> }
>
> Next we're parsing the members of InetSocketAddress (the type of
> 'data'). The visitor wants to visit 'host' now, but it's not present in
> the QDict to parse (StackObject.obj, which is actually an empty QDict in
> this case because 'data' wasn't given at all).
Wait a sec! Doesn't qobject_input_start_struct(v, "data", ...) *fail*
when "data" is absent? Oh, looks like you're messing with that in PATCH
4. Forward reference only in your explanation, not in this patch, which
"doesn't actually allow using the aliases yet".
> So what happens is that it looks at aliases that could provide a value
> for this key. Its alias list contains the alias=NULL,src=[NULL] item,
> which makes it check if 'host' exists in .alias_so (which is the
> StackObject that belongs to the top level SocketAddressLegacy) instead,
> and indeed we have a 'host' key there, so we can take the value from
> there.
When src[0] isn't null, this InputVisitorAlias is to be applied deeper
in the visit, and we ignore it here.
Correct?
> Does this make the mechanism a bit clearer?
Feels like it :)
>> > +
>> > + /*
>> > + * 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.
>
> Yes.
>
>> > + * 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;
We need a list because any number of aliases may apply here (src[0]
null) or deeper in the stack (not null).
Correct?
>> > + 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?
>
> Works for me.
>
>> > +
>> > 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.
>
> I hope the above helps to resolve the confusion. Feel free to come back
> with more questions or ping me on IRC if necessary.
Thanks!
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(+)
>
> 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.
> + */
> + const char **src;
> +
> + /* StackObject in which the alias should be looked for */
> + struct StackObject *alias_so;
> +
> + /*
> + * The alias remains valid as long as the containing StackObject has
> + * 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 */
> +
> QSLIST_ENTRY(StackObject) node; /* parent */
> } StackObject;
>
> @@ -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?
> + 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?
> } else {
> assert(qlist);
> tos->entry = qlist_first(qlist);
> @@ -257,10 +318,17 @@ static bool qobject_input_check_struct(Visitor *v, Error **errp)
>
> static void qobject_input_stack_object_free(StackObject *tos)
> {
> + InputVisitorAlias *a;
> +
> if (tos->h) {
> g_hash_table_unref(tos->h);
> }
>
> + while ((a = QSLIST_FIRST(&tos->aliases))) {
> + QSLIST_REMOVE_HEAD(&tos->aliases, next);
> + g_free(a);
> + }
> +
> g_free(tos);
> }
>
> @@ -274,6 +342,50 @@ static void qobject_input_pop(Visitor *v, void **obj)
> qobject_input_stack_object_free(tos);
> }
>
> +static void qobject_input_start_alias_scope(Visitor *v)
> +{
> + QObjectInputVisitor *qiv = to_qiv(v);
> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> +
> + tos->alias_scope_nesting++;
> +}
> +
> +static void qobject_input_end_alias_scope(Visitor *v)
> +{
> + QObjectInputVisitor *qiv = to_qiv(v);
> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> + InputVisitorAlias *a, *next;
> +
> + assert(tos->alias_scope_nesting > 0);
> + tos->alias_scope_nesting--;
> +
> + QSLIST_FOREACH_SAFE(a, &tos->aliases, next, next) {
> + if (a->scope_nesting > tos->alias_scope_nesting) {
> + QSLIST_REMOVE(&tos->aliases, a, InputVisitorAlias, next);
> + g_free(a);
> + }
> + }
> +}
> +
> +static void qobject_input_define_alias(Visitor *v, const char *alias_name,
> + const char **source)
> +{
> + QObjectInputVisitor *qiv = to_qiv(v);
> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> + InputVisitorAlias *alias = g_new(InputVisitorAlias, 1);
> +
> + /* The source path can only be empty for wildcard aliases */
> + assert(source[0] || !alias_name);
Possibly related: the "What does .alias = NULL, .src[] empty mean?" we
discussed previously.
> +
> + *alias = (InputVisitorAlias) {
> + .alias = alias_name,
> + .alias_so = tos,
> + .src = source,
> + };
> +
> + QSLIST_INSERT_HEAD(&tos->aliases, alias, next);
> +}
> +
> static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj,
> size_t size, Error **errp)
> {
> @@ -696,6 +808,9 @@ static QObjectInputVisitor *qobject_input_visitor_base_new(QObject *obj)
> v->visitor.end_list = qobject_input_end_list;
> v->visitor.start_alternate = qobject_input_start_alternate;
> v->visitor.optional = qobject_input_optional;
> + v->visitor.define_alias = qobject_input_define_alias;
> + v->visitor.start_alias_scope = qobject_input_start_alias_scope;
> + v->visitor.end_alias_scope = qobject_input_end_alias_scope;
> v->visitor.free = qobject_input_free;
>
> v->root = qobject_ref(obj);
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
© 2016 - 2026 Red Hat, Inc.