[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v8 20/35] qmp: Don't abuse stack to track qmp-output
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v8 20/35] qmp: Don't abuse stack to track qmp-output root |
Date: |
Mon, 21 Dec 2015 10:08:25 -0700 |
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. Drop the now-useless qmp_output_first() while at it.
Saved for a later patch: we still are rather sloppy in that
qmp_output_get_object() can be called in the middle of a parse,
rather than requiring that a visit is complete.
Signed-off-by: Eric Blake <address@hidden>
---
v8: rebase to earlier changes
v7: retitle; rebase to earlier changes, drop qmp_output_first()
v6: no change
---
qapi/qmp-output-visitor.c | 79 ++++++++++++++++-------------------------------
1 file changed, 26 insertions(+), 53 deletions(-)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 316f4e4..df22999 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -29,16 +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 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) \
@@ -55,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,26 +69,12 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
return value;
}
-/* 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);
-
- if (!e) {
- /* No root */
- return NULL;
- }
- assert(e->value);
- return e->value;
-}
-
-/* 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 && e->value);
- return e->value;
+ return e ? e->value : NULL;
}
/* Add @value to the current QObject being built.
@@ -106,29 +85,25 @@ 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 */
- /* 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();
+ }
}
}
@@ -230,7 +205,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 {
@@ -248,16 +225,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 v8 12/35] qapi: Don't cast Enum* to int*, (continued)
- [Qemu-devel] [PATCH v8 12/35] qapi: Don't cast Enum* to int*, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 08/35] qapi: Track all failures between visit_start/stop, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 04/35] hmp: Improve use of qapi visitor, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 02/35] qapi: Avoid use of misnamed DO_UPCAST(), Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 09/35] qapi: Prefer type_int64 over type_int in visitors, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 10/35] qapi: Make all visitors supply uint64 callbacks, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 11/35] qapi: Consolidate visitor small integer callbacks, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 07/35] qapi: Improve generated event use of qapi visitor, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 17/35] qapi: Drop unused 'kind' for struct/enum visit, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 13/35] qom: Use typedef for Visitor, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 20/35] qmp: Don't abuse stack to track qmp-output root,
Eric Blake <=
- [Qemu-devel] [PATCH v8 18/35] qapi: Drop unused error argument for list and implicit struct, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 27/35] qapi: Fix command with named empty argument type, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 01/35] qobject: Document more shortcomings in our number handling, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 22/35] qapi: Add visit_type_null() visitor, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 25/35] qapi: Simplify excess input reporting in input visitors, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 19/35] qmp: Fix reference-counting of qnull on empty output visit, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 21/35] qapi: Document visitor interfaces, add assertions, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 28/35] qapi: Eliminate empty visit_type_FOO_fields, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 24/35] spapr_drc: Expose 'null' in qom-get when there is no fdt, Eric Blake, 2015/12/21
- [Qemu-devel] [PATCH v8 31/35] qapi: Rework deallocation of partial struct, Eric Blake, 2015/12/21