[Qemu-devel] [PATCH v16 18/24] qmp: Tighten output visitor rules

[Eric Blake <eblake@redhat.com>]

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 <eblake@redhat.com>

---
v15: split off qmp_output_visitor_reset(), improve comments,
use QTAILQ_EMPTY
v14: no change
v13: no change
v12: rebase to latest, move type_null() into earlier patches,
don't change signature of pop, don't auto-reset after a single
get_qobject
[no v10, v11]
v9: rebase to added patch, squash in more sanity checks, drop
Marc-Andre's R-b
v8: rename qmp_output_reset to qmp_output_visitor_reset
v7: new patch, based on discussion about spapr_drc.c

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi/qmp-output-visitor.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 6c44210..7155bde 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)
-- 
2.5.5