[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v5 22/46] qapi: Don't abuse stack to track qmp-outpu
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v5 22/46] qapi: Don't abuse stack to track qmp-output root |
Date: |
Mon, 21 Sep 2015 15:57:38 -0600 |
The previous commit documented an inconsistency in how we are
using the stack of qmp-output-visitor. Normally, pushing a
single top-level object puts the object on the stack twice:
once as the root, and once as the current container being
appended to; but popping that struct only pops once. However,
qmp_ouput_add() was trying to either set up the added object
as the new root (works if you parse two top-level scalars in a
row: the second replaces the first as the root) or as a member
of the current container (works as long as you have an open
container on the stack; but if you have popped the first
top-level container, it then resolves to the root and still
tries to add into that existing container).
Fix the stupidity by not tracking two separate things in the
stack.
Not done here: maybe qmp_output_get_object() should assert that
the stack is empty, rather than letting users look at the current
root even while the root is still being visited.
---
qapi/qmp-output-visitor.c | 70 +++++++++++++++--------------------------------
1 file changed, 22 insertions(+), 48 deletions(-)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 850c83d..f891e72 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -29,13 +29,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, and the stack of objects
- * still being built. Worse, our behavior is inconsistent:
- * 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. */
- QStack stack;
+ QStack stack; /* Stack of containers still growing */
+ QObject *root; /* Root of the output visit */
};
#define qmp_output_add(qov, name, value) \
@@ -52,6 +47,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) {
@@ -76,28 +72,15 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
/* Grab the root QObject, if any, in preparation to empty the stack */
static QObject *qmp_output_first(QmpOutputVisitor *qov)
{
- QStackEntry *e = QTAILQ_LAST(&qov->stack, QStack);
-
- /*
- * FIXME Wrong, because qmp_output_get_qobject() will increment
- * the refcnt *again*. We need to think through how visitors
- * handle null.
- */
- if (!e) {
- /* No root */
- return NULL;
- }
- assert(e->value);
- return e->value;
+ return qov->root;
}
-/* Grab the most recent QObject from the stack, which must exist */
+/* Grab the most recent QObject from the stack, if any */
static QObject *qmp_output_last(QmpOutputVisitor *qov)
{
QStackEntry *e = QTAILQ_FIRST(&qov->stack);
- assert(e);
- return e->value;
+ return e ? e->value : NULL;
}
/* Add @value to the current QObject being built.
@@ -108,28 +91,23 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov,
const char *name,
{
QObject *cur;
- if (QTAILQ_EMPTY(&qov->stack)) {
- /* Stack was empty, track this object as root */
- qmp_output_push_obj(qov, value);
- return;
- }
-
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 */
- qobject_decref(qmp_output_pop(qov));
- assert(QTAILQ_EMPTY(&qov->stack));
- qmp_output_push_obj(qov, value);
- break;
+ if (!cur) {
+ 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();
+ }
}
}
@@ -242,16 +220,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);
}
--
2.4.3
- [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an implicit type, (continued)
- [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an implicit type, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 14/46] qapi: Detect collisions in C member names, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 16/46] qapi: Detect base class loops, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 15/46] qapi: Defer duplicate member checks to schema check(), Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 13/46] qapi: Track owner of each object member, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 17/46] qapi: Provide nicer array names in introspection, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 18/46] qapi-introspect: Guarantee particular sorting, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 20/46] qapi: Fix alternates that accept 'number' but not 'int', Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 21/46] qmp: Fix reference-counting of qnull on empty output visit, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 19/46] qapi: Simplify visiting of alternate types, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 22/46] qapi: Don't abuse stack to track qmp-output root,
Eric Blake <=
- [Qemu-devel] [PATCH v5 26/46] qapi: Test failure in middle of array parse, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 23/46] qapi: Remove dead visitor code, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 27/46] qapi: Simplify visits of optional fields, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 25/46] qapi: Plug leaks in test-qmp-input-visitor, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 24/46] qapi: Document visitor interfaces, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 28/46] qapi: Rework deallocation of partial struct, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 29/46] qapi: Change visit_type_FOO() to no longer return partial objects, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 30/46] net: use Netdev instead of NetClientOptions in client init, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 33/46] vnc: hoist allocation of VncBasicInfo to callers, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 32/46] qapi: Hide tag_name data member of variants, Eric Blake, 2015/09/21