[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eric Blake
Subject: [Qemu-devel] [PATCH v16 18/24] qmp: Tighten output visitor rules
Date: Thu, 28 Apr 2016 15:45:26 -0600

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 <address@hidden>

v15: split off qmp_output_visitor_reset(), improve comments,
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
[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 <address@hidden>
 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);
         case QTYPE_QLIST:
+            assert(!name);
             qlist_append_obj(qobject_to_qlist(cur), value);
@@ -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)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]