[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: |
Markus Armbruster |
Subject: |
Re: [PATCH 2/6] qapi: Remember alias definitions in qobject-input-visitor |
Date: |
Wed, 27 Jan 2021 14:06:53 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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.
[...]
- Re: [PATCH 2/6] qapi: Remember alias definitions in qobject-input-visitor,
Markus Armbruster <=