1 files changed, 26 insertions, 63 deletions

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4500e3bc49..f47eefa626 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -31,16 +31,8 @@ typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
 struct QmpOutputVisitor
 {
     Visitor visitor;
-    /* FIXME: we are abusing stack to hold two separate pieces of
-     * information: the current root object in slot 0, and the stack
-     * of N objects still being built in slots 1 through N (for N+1
-     * slots in use).  Worse, our behavior is inconsistent:
-     * qmp_output_add_obj() visiting two top-level scalars in a row
-     * discards the first in favor of the second, but visiting two
-     * top-level objects in a row tries to append the second object
-     * into the first (since the first object was placed in the stack
-     * in both slot 0 and 1, but only popped from slot 1).  */
-    QStack stack;
+    QStack stack; /* Stack of containers that haven't yet been finished */
+    QObject *root; /* Root of the output visit */
 };
 
 #define qmp_output_add(qov, name, value) \
@@ -57,6 +49,7 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
 {
     QStackEntry *e = g_malloc0(sizeof(*e));
 
+    assert(qov->root);
     assert(value);
     e->value = value;
     if (qobject_type(e->value) == QTYPE_QLIST) {
@@ -79,60 +72,32 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
     return value;
 }
 
-/* Grab the root QObject, if any */
-static QObject *qmp_output_first(QmpOutputVisitor *qov)
-{
-    QStackEntry *e = QTAILQ_LAST(&qov->stack, QStack);
-
-    if (!e) {
-        /* No root */
-        return NULL;
-    }
-    assert(e->value);
-    return e->value;
-}
-
-/* Peek at the top of the stack of QObjects being built.
- * The stack must not be empty. */
-static QObject *qmp_output_last(QmpOutputVisitor *qov)
-{
-    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
-
-    assert(e && e->value);
-    return e->value;
-}
-
 /* Add @value to the current QObject being built.
  * If the stack is visiting a dictionary or list, @value is now owned
  * by that container. Otherwise, @value is now the root.  */
 static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
                                QObject *value)
 {
-    QObject *cur;
-
-    if (QTAILQ_EMPTY(&qov->stack)) {
-        /* Stack was empty, track this object as root */
-        qmp_output_push_obj(qov, value);
-        return;
-    }
+    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
+    QObject *cur = e ? e->value : NULL;
 
-    cur = qmp_output_last(qov);
-
-    switch (qobject_type(cur)) {
-    case QTYPE_QDICT:
-        assert(name);
-        qdict_put_obj(qobject_to_qdict(cur), name, value);
-        break;
-    case QTYPE_QLIST:
-        qlist_append_obj(qobject_to_qlist(cur), value);
-        break;
-    default:
-        /* The previous root was a scalar, replace it with a new root */
-        /* FIXME this is abusing the stack; see comment above */
-        qobject_decref(qmp_output_pop(qov));
-        assert(QTAILQ_EMPTY(&qov->stack));
-        qmp_output_push_obj(qov, value);
-        break;
+    if (!cur) {
+        /* FIXME we should require the user to reset the visitor, rather
+         * than throwing away the previous root */
+        qobject_decref(qov->root);
+        qov->root = value;
+    } else {
+        switch (qobject_type(cur)) {
+        case QTYPE_QDICT:
+            assert(name);
+            qdict_put_obj(qobject_to_qdict(cur), name, value);
+            break;
+        case QTYPE_QLIST:
+            qlist_append_obj(qobject_to_qlist(cur), value);
+            break;
+        default:
+            g_assert_not_reached();
+        }
     }
 }
 
@@ -233,7 +198,9 @@ static void qmp_output_type_any(Visitor *v, const char *name, QObject **obj,
 /* Finish building, and return the root object. Will not be NULL. */
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
-    QObject *obj = qmp_output_first(qov);
+    /* FIXME: we should require that a visit occurred, and that it is
+     * complete (no starts without a matching end) */
+    QObject *obj = qov->root;
     if (obj) {
         qobject_incref(obj);
     } else {
@@ -251,16 +218,12 @@ void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
 {
     QStackEntry *e, *tmp;
 
-    /* The bottom QStackEntry, if any, owns the root QObject. See the
-     * qmp_output_push_obj() invocations in qmp_output_add_obj(). */
-    QObject *root = QTAILQ_EMPTY(&v->stack) ? NULL : qmp_output_first(v);
-
     QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
         QTAILQ_REMOVE(&v->stack, e, node);
         g_free(e);
     }
 
-    qobject_decref(root);
+    qobject_decref(v->root);
     g_free(v);
 }