[PULL,18/23] qmp: Tighten output visitor rules

Commit Message

Markus Armbruster May 12, 2016, 7:59 a.m. UTC

From: Eric Blake <[e-mail]>

Tighten assertions in the QMP output visitor, so that:

- qmp_output_get_qobject() can only be called after pairing a
visit_end_* for every visit_start_* (rather than allowing it on
a partially built object)

- qmp_output_get_qobject() cannot be called unless at least one
visit_type_* or visit_start/visit_end pair has occurred since
creation/reset (the accidental return of NULL fixed by commit
ab8bf1d7 would have been much easier to diagnose)

- ensure that we are encountering the expected object or list
type, to provide protection against mismatched push(struct)/
pop(list) or push(list)/pop(struct), similar to the qmp-input
protection added in commit bdd8e6b5.

- ensure that except for the root, 'name' is non-null inside a
dict, and NULL inside a list (this may need changing later if
we add "name.0" support for better error messages for a list,
but for now it makes sure all users are at least consistent)

Signed-off-by: Eric Blake <[e-mail]>
Message-Id: <[e-mail]>
Signed-off-by: Markus Armbruster <[e-mail]>
---
 qapi/qmp-output-visitor.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Patch

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 5681ad3..4e21f1d 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -82,9 +82,8 @@  static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
     QObject *cur = e ? e->value : NULL;
 
     if (!cur) {
-        /* FIXME we should require the user to reset the visitor, rather
-         * than throwing away the previous root */
-        qobject_decref(qov->root);
+        /* Don't allow reuse of visitor on more than one root */
+        assert(!qov->root);
         qov->root = value;
     } else {
         switch (qobject_type(cur)) {
@@ -93,6 +92,7 @@  static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
             qdict_put_obj(qobject_to_qdict(cur), name, value);
             break;
         case QTYPE_QLIST:
+            assert(!name);
             qlist_append_obj(qobject_to_qlist(cur), value);
             break;
         default:
@@ -114,7 +114,8 @@  static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
 static void qmp_output_end_struct(Visitor *v, Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov);
+    assert(qobject_type(value) == QTYPE_QDICT);
 }
 
 static void qmp_output_start_list(Visitor *v, const char *name, Error **errp)
@@ -145,7 +146,8 @@  static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp,
 static void qmp_output_end_list(Visitor *v)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov);
+    assert(qobject_type(value) == QTYPE_QLIST);
 }
 
 static void qmp_output_type_int64(Visitor *v, const char *name, int64_t *obj,
@@ -202,18 +204,16 @@  static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
     qmp_output_add_obj(qov, name, qnull());
 }
 
-/* Finish building, and return the root object. Will not be NULL. */
+/* Finish building, and return the root object.
+ * The root object is never null. The caller becomes the object's
+ * owner, and should use qobject_decref() when done with it.  */
 QObject *qmp_output_get_qobject(QmpOutputVisitor *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 {
-        obj = qnull();
-    }
-    return obj;
+    /* A visit must have occurred, with each start paired with end.  */
+    assert(qov->root && QTAILQ_EMPTY(&qov->stack));
+
+    qobject_incref(qov->root);
+    return qov->root;
 }
 
 Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)