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

[Markus Armbruster <armbru@redhat.com>]

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 | 145 +++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
>
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 23843b242e..aa95cd49bd 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -29,6 +29,50 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  
> +/*
> + * Describes an alias that is relevant for the current StackObject,
> + * either because it aliases a member of the currently visited object
> + * or because it aliases a member of a nested object.
> + *
> + * When processing a nested object, all InputVisitorAlias objects that
> + * are relevant for the nested object are propagated, i.e. copied with
> + * the name of the nested object removed from @source.
> + */
> +typedef struct InputVisitorAlias {
> +    /* StackObject in which the alias was defined */
> +    struct StackObject *alias_so;
> +
> +    /*
> +     * Alias name as defined for @alias_so.
> +     * NULL means that this is a wildcard alias, i.e. all members of
> +     * @src get an alias in @alias_so with the same name.
> +     */
> +    const char *name;
> +
> +    /*
> +     * NULL terminated array representing a path to the source member

NULL-terminated

> +     * that the alias refers to.
> +     *
> +     * Must contain at least one non-NULL element if @alias is not NULL.
> +     *
> +     * If it contains no non-NULL element, @alias_so must be different
> +     * from the StackObject which contains this InputVisitorAlias in
> +     * its aliases list.  In this case, all elements in the currently
> +     * visited object have an alias with the same name in @alias_so.
> +     */
> +    const char **src;
> +
> +    /*
> +     * The alias remains valid as long as the StackObject which
> +     * contains this InputVisitorAlias in its aliases list 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 +82,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;    /* Number of open alias scopes */
> +
>      QSLIST_ENTRY(StackObject) node; /* parent */
>  } StackObject;
>  
> @@ -203,6 +250,43 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
>      return qstring_get_str(qstr);
>  }
>  
> +/*
> + * Propagate 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 @dst->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. @dst->name) removed from the source path.
> + */
> +static void propagate_aliases(StackObject *dst, StackObject *src)
> +{
> +    InputVisitorAlias *a;
> +
> +    QSLIST_FOREACH(a, &src->aliases, next) {
> +        if (!a->src[0] || strcmp(a->src[0], dst->name)) {
> +            continue;
> +        }
> +
> +        /*
> +         * If this is not a wildcard alias, but a->src[1] is NULL,
> +         * then it referred to dst->name in src and doesn't apply
> +         * inside dst any more.
> +         */
> +        if (a->src[1] || !a->name) {

The comment explains "if COND then there is nothing to do".  The code
that follows it does "if (!COND) { do stuff }".  Works, but I had to
stop and re-read to get it.

How do you like

           if (a->name && !a->src[1]) {
               continue;
           }
           do stuff

?

> +            InputVisitorAlias *alias = g_new(InputVisitorAlias, 1);
> +
> +            *alias = (InputVisitorAlias) {
> +                .name       = a->name,
> +                .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 +310,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));
> +        }
>      } else {
>          assert(qlist);
>          tos->entry = qlist_first(qlist);
> @@ -257,10 +344,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 +368,54 @@ 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 *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 become empty during alias propagation for
> +     * wildcard aliases, but not when defining an alias (it would map
> +     * all names onto themselves, which doesn't make sense).
> +     */
> +    assert(source[0]);
> +
> +    *alias = (InputVisitorAlias) {
> +        .name       = 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 +838,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);