[Qemu-devel] [PATCH v12 16/18] qmp-input: Require struct push to visit members of top dict

[Eric Blake]

Don't embed the root of the visit into the stack of current
containers being visited.  That way, we no longer get confused
on whether the first visit of a dictionary is to the dictionary
itself or to one of the members of the dictionary, and we no
longer have to require the root visit to pass name=NULL.

An audit of all qmp_input_visitor_new* call sites shows that
the only places where the visit starts directly on a QDict,
but where the first visit_type_* within the visit had passed
a non-NULL name, were fixed in the previous two places to
properly push into the object with visit_start_struct().

Signed-off-by: Eric Blake <address@hidden>

---
v12: new patch
---
 include/qapi/visitor.h           |  3 +--
 include/qapi/qmp-input-visitor.h |  8 ------
 qapi/qmp-input-visitor.c         | 54 ++++++++++++++++++++++------------------
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b3ca93c..fb4582a 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -48,8 +48,7 @@
  *
  * The @name parameter of visit_type_FOO() describes the relation
  * between this QAPI value and its parent container.  When visiting
- * the root of a tree, @name is usually ignored (although some
- * visitors require it to be NULL); when visiting a member of an
+ * the root of a tree, @name is ignored; when visiting a member of an
  * object, @name is the key associated with the value; and when
  * visiting a member of a list, @name is NULL.
  *
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index d75ff98..3ed499c 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qmp-input-visitor.h
@@ -19,14 +19,6 @@

 typedef struct QmpInputVisitor QmpInputVisitor;

-/*
- * FIXME: When visiting a QDict, passing a non-NULL @name for the
- * first visit_type_FOO() when the root is a QDict will find that
- * particular key within the QDict.  In the future, the contract may
- * be tightened to require visit_start_struct() with ignored @name as
- * the first visit; in the meantime, the first visit is safest when
- * using NULL for @name.
- */
 QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
 QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 6b69e9a..2daf83f 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -38,9 +38,11 @@ struct QmpInputVisitor
 {
     Visitor visitor;

-    /* Stack of objects being visited.  stack[0] is root of visit,
-     * stack[1] and below correspond to visit_start_struct (nested
-     * QDict) and visit_start_list (nested QList).  */
+    /* Root of visit at visitor creation.  */
+    QObject *root;
+
+    /* Stack of objects being visited (all entries will be either
+     * QDict or QList).  */
     StackObject stack[QIV_STACK_SIZE];
     int nb_stack;

@@ -57,36 +59,40 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
                                      const char *name,
                                      bool consume)
 {
-    StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
-    QObject *qobj = tos->obj;
+    StackObject *tos;
+    QObject *qobj;

+    if (!qiv->nb_stack) {
+        /* Starting at root, name is ignored.  */
+        return qiv->root;
+    }
+
+    /* We are in a container; find the next element */
+    tos = &qiv->stack[qiv->nb_stack - 1];
+    qobj = tos->obj;
     assert(qobj);

-    /* If we have a name, and we're in a dictionary, then return that
-     * value. */
-    if (name && qobject_type(qobj) == QTYPE_QDICT) {
+    if (qobject_type(qobj) == QTYPE_QDICT) {
+        assert(name);
         qobj = qdict_get(qobject_to_qdict(qobj), name);
         if (tos->h && consume && qobj) {
             bool removed = g_hash_table_remove(tos->h, name);
             assert(removed);
         }
-        return qobj;
-    }
-
-    /* If we are in the middle of a list, then return the next element
-     * of the list.  */
-    if (tos->entry) {
+    } else {
         assert(qobject_type(qobj) == QTYPE_QLIST);
-        assert(!tos->first);
-        qobj = qlist_entry_obj(tos->entry);
-        if (consume) {
-            tos->entry = qlist_next(tos->entry);
+        /* FIXME: assertion needs adjustment if we fix visit-core
+         * to pass "name.0" style name during lists.  */
+        assert(!name);
+
+        if (tos->entry) {
+            assert(!tos->first);
+            qobj = qlist_entry_obj(tos->entry);
+            if (consume) {
+                tos->entry = qlist_next(tos->entry);
+            }
         }
-        return qobj;
     }
-
-    /* Otherwise, we are at the root of the visit or the start of a
-     * list, and return the object as-is.  */
     return qobj;
 }

@@ -372,7 +378,7 @@ Visitor *qmp_input_get_visitor(QmpInputVisitor *v)

 void qmp_input_visitor_cleanup(QmpInputVisitor *v)
 {
-    qobject_decref(v->stack[0].obj);
+    qobject_decref(v->root);
     g_free(v);
 }

@@ -399,7 +405,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.type_null = qmp_input_type_null;
     v->visitor.optional = qmp_input_optional;

-    qmp_input_push(v, obj, NULL, NULL);
+    v->root = obj;
     qobject_incref(obj);

     return v;
-- 
2.5.0