diff mbox

[PULL,12/15] qapi: Add new visit_complete() function

Message ID 1467796446-25446-13-git-send-email-armbru@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Markus Armbruster July 6, 2016, 9:14 a.m. UTC
From: Eric Blake <eblake@redhat.com>

Making each output visitor provide its own output collection
function was the only remaining reason for exposing visitor
sub-types to the rest of the code base.  Add a polymorphic
visit_complete() function which is a no-op for input visitors,
and which populates an opaque pointer for output visitors.  For
maximum type-safety, also add a parameter to the output visitor
constructors with a type-correct version of the output pointer,
and assert that the two uses match.

This approach was considered superior to either passing the
output parameter only during construction (action at a distance
during visit_free() feels awkward) or only during visit_complete()
(defeating type safety makes it easier to use incorrectly).

Most callers were function-local, and therefore a mechanical
conversion; the testsuite was a bit trickier, but the previous
cleanup patch minimized the churn here.

The visit_complete() function may be called at most once; doing
so lets us use transfer semantics rather than duplication or
ref-count semantics to get the just-built output back to the
caller, even though it means our behavior is not idempotent.

Generated code is simplified as follows for events:

|@@ -26,7 +26,7 @@ void qapi_event_send_acpi_device_ost(ACP
|     QDict *qmp;
|     Error *err = NULL;
|     QMPEventFuncEmit emit;
|-    QmpOutputVisitor *qov;
|+    QObject *obj;
|     Visitor *v;
|     q_obj_ACPI_DEVICE_OST_arg param = {
|         info
|@@ -39,8 +39,7 @@ void qapi_event_send_acpi_device_ost(ACP
|
|     qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
|
|-    qov = qmp_output_visitor_new();
|-    v = qmp_output_get_visitor(qov);
|+    v = qmp_output_visitor_new(&obj);
|
|     visit_start_struct(v, "ACPI_DEVICE_OST", NULL, 0, &err);
|     if (err) {
|@@ -55,7 +54,8 @@ void qapi_event_send_acpi_device_ost(ACP
|         goto out;
|     }
|
|-    qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
|+    visit_complete(v, &obj);
|+    qdict_put_obj(qmp, "data", obj);
|     emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);

and for commands:

| {
|     Error *err = NULL;
|-    QmpOutputVisitor *qov = qmp_output_visitor_new();
|     Visitor *v;
|
|-    v = qmp_output_get_visitor(qov);
|+    v = qmp_output_visitor_new(ret_out);
|     visit_type_AddfdInfo(v, "unused", &ret_in, &err);
|-    if (err) {
|-        goto out;
|+    if (!err) {
|+        visit_complete(v, ret_out);
|     }
|-    *ret_out = qmp_output_get_qobject(qov);
|-
|-out:
|     error_propagate(errp, err);

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-13-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/qapi.c                         |  9 +++----
 blockdev.c                           |  9 +++----
 docs/qapi-code-gen.txt               | 10 +++-----
 hmp.c                                | 11 ++++----
 include/qapi/qmp-output-visitor.h    | 11 +++++---
 include/qapi/string-output-visitor.h | 13 +++++++---
 include/qapi/visitor-impl.h          |  3 +++
 include/qapi/visitor.h               | 50 +++++++++++++++++++++---------------
 net/net.c                            | 11 ++++----
 qapi/qapi-visit-core.c               |  8 ++++++
 qapi/qmp-output-visitor.c            | 21 ++++++++-------
 qapi/string-output-visitor.c         | 22 ++++++++--------
 qemu-img.c                           | 30 +++++++++++-----------
 qom/object.c                         | 28 +++++++++-----------
 qom/qom-qobject.c                    | 10 ++++----
 replay/replay-input.c                |  6 ++---
 scripts/qapi-commands.py             | 10 +++-----
 scripts/qapi-event.py                |  8 +++---
 tests/check-qnull.c                  |  9 +++----
 tests/test-qmp-output-visitor.c      | 11 +++-----
 tests/test-string-output-visitor.c   |  8 ++----
 tests/test-visitor-serialization.c   | 22 ++++++++--------
 util/qemu-sockets.c                  |  6 ++---
 23 files changed, 166 insertions(+), 160 deletions(-)
diff mbox

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 41fe4f9..6f947e3 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -690,16 +690,15 @@  static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
 void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
                                    ImageInfoSpecific *info_spec)
 {
-    QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj, *data;
+    Visitor *v = qmp_output_visitor_new(&obj);
 
-    visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), NULL, &info_spec,
-                                 &error_abort);
-    obj = qmp_output_get_qobject(ov);
+    visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
+    visit_complete(v, &obj);
     assert(qobject_type(obj) == QTYPE_QDICT);
     data = qdict_get(qobject_to_qdict(obj), "data");
     dump_qobject(func_fprintf, f, 1, data);
-    visit_free(qmp_output_get_visitor(ov));
+    visit_free(v);
 }
 
 void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
diff --git a/blockdev.c b/blockdev.c
index c05aaa6..0f8065c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3950,10 +3950,10 @@  out:
 
 void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 {
-    QmpOutputVisitor *ov = qmp_output_visitor_new();
     BlockDriverState *bs;
     BlockBackend *blk = NULL;
     QObject *obj;
+    Visitor *v = qmp_output_visitor_new(&obj);
     QDict *qdict;
     Error *local_err = NULL;
 
@@ -3972,14 +3972,13 @@  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
         }
     }
 
-    visit_type_BlockdevOptions(qmp_output_get_visitor(ov), NULL, &options,
-                               &local_err);
+    visit_type_BlockdevOptions(v, NULL, &options, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto fail;
     }
 
-    obj = qmp_output_get_qobject(ov);
+    visit_complete(v, &obj);
     qdict = qobject_to_qdict(obj);
 
     qdict_flatten(qdict);
@@ -4020,7 +4019,7 @@  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
     }
 
 fail:
-    visit_free(qmp_output_get_visitor(ov));
+    visit_free(v);
 }
 
 void qmp_x_blockdev_del(bool has_id, const char *id,
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 7c30762..48b0b31 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -980,17 +980,13 @@  Example:
     static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in, QObject **ret_out, Error **errp)
     {
         Error *err = NULL;
-        QmpOutputVisitor *qov = qmp_output_visitor_new();
         Visitor *v;
 
-        v = qmp_output_get_visitor(qov);
+        v = qmp_output_visitor_new(ret_out);
         visit_type_UserDefOne(v, "unused", &ret_in, &err);
-        if (err) {
-            goto out;
+        if (!err) {
+            visit_complete(v, ret_out);
         }
-        *ret_out = qmp_output_get_qobject(qov);
-
-    out:
         error_propagate(errp, err);
         visit_free(v);
         v = qapi_dealloc_visitor_new();
diff --git a/hmp.c b/hmp.c
index 559cad1..0cf5baa 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1983,15 +1983,14 @@  void hmp_info_memdev(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
     MemdevList *memdev_list = qmp_query_memdev(&err);
     MemdevList *m = memdev_list;
-    StringOutputVisitor *ov;
+    Visitor *v;
     char *str;
     int i = 0;
 
 
     while (m) {
-        ov = string_output_visitor_new(false);
-        visit_type_uint16List(string_output_get_visitor(ov), NULL,
-                              &m->value->host_nodes, NULL);
+        v = string_output_visitor_new(false, &str);
+        visit_type_uint16List(v, NULL, &m->value->host_nodes, NULL);
         monitor_printf(mon, "memory backend: %d\n", i);
         monitor_printf(mon, "  size:  %" PRId64 "\n", m->value->size);
         monitor_printf(mon, "  merge: %s\n",
@@ -2002,11 +2001,11 @@  void hmp_info_memdev(Monitor *mon, const QDict *qdict)
                        m->value->prealloc ? "true" : "false");
         monitor_printf(mon, "  policy: %s\n",
                        HostMemPolicy_lookup[m->value->policy]);
-        str = string_output_get_string(ov);
+        visit_complete(v, &str);
         monitor_printf(mon, "  host nodes: %s\n", str);
 
         g_free(str);
-        visit_free(string_output_get_visitor(ov));
+        visit_free(v);
         m = m->next;
         i++;
     }
diff --git a/include/qapi/qmp-output-visitor.h b/include/qapi/qmp-output-visitor.h
index 29c9a2e..040fdda 100644
--- a/include/qapi/qmp-output-visitor.h
+++ b/include/qapi/qmp-output-visitor.h
@@ -19,9 +19,12 @@ 
 
 typedef struct QmpOutputVisitor QmpOutputVisitor;
 
-QmpOutputVisitor *qmp_output_visitor_new(void);
-
-QObject *qmp_output_get_qobject(QmpOutputVisitor *v);
-Visitor *qmp_output_get_visitor(QmpOutputVisitor *v);
+/*
+ * Create a new QMP output visitor.
+ *
+ * If everything else succeeds, pass @result to visit_complete() to
+ * collect the result of the visit.
+ */
+Visitor *qmp_output_visitor_new(QObject **result);
 
 #endif
diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
index 03e377e..268dfe9 100644
--- a/include/qapi/string-output-visitor.h
+++ b/include/qapi/string-output-visitor.h
@@ -18,13 +18,18 @@ 
 typedef struct StringOutputVisitor StringOutputVisitor;
 
 /*
+ * Create a new string output visitor.
+ *
+ * Using @human creates output that is a bit easier for humans to read
+ * (for example, showing integer values in both decimal and hex).
+ *
+ * If everything else succeeds, pass @result to visit_complete() to
+ * collect the result of the visit.
+ *
  * The string output visitor does not implement support for visiting
  * QAPI structs, alternates, null, or arbitrary QTypes.  It also
  * requires a non-null list argument to visit_start_list().
  */
-StringOutputVisitor *string_output_visitor_new(bool human);
-
-char *string_output_get_string(StringOutputVisitor *v);
-Visitor *string_output_get_visitor(StringOutputVisitor *v);
+Visitor *string_output_visitor_new(bool human, char **result);
 
 #endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 525b068..16e0b86 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -105,6 +105,9 @@  struct Visitor
     /* Must be set */
     VisitorType type;
 
+    /* Must be set for output visitors, optional otherwise. */
+    void (*complete)(Visitor *v, void *opaque);
+
     /* Must be set */
     void (*free)(Visitor *v);
 };
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 2ded852..00a60ea 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -39,21 +39,15 @@ 
  *
  * All of the visitors are created via:
  *
- * Type *subtype_visitor_new(parameters...);
- *
- * where Type is either directly 'Visitor *', or is a subtype that can
- * be trivially upcast to Visitor * via another function:
- *
- * Visitor *subtype_get_visitor(SubtypeVisitor *);
+ * Visitor *subtype_visitor_new(parameters...);
  *
  * A visitor should be used for exactly one top-level visit_type_FOO()
- * or virtual walk, then passed to visit_free() to clean up resources.
+ * or virtual walk; if that is successful, the caller can optionally
+ * call visit_complete() (for now, useful only for output visits, but
+ * safe to call on all visits).  Then, regardless of success or
+ * failure, the user should call visit_free() to clean up resources.
  * It is okay to free the visitor without completing the visit, if
- * some other error is detected in the meantime.  Output visitors
- * provide an additional function, for collecting the final results of
- * a successful visit: string_output_get_string() and
- * qmp_output_get_qobject(); this collection function should not be
- * called if any errors were reported during the visit.
+ * some other error is detected in the meantime.
  *
  * All QAPI types have a corresponding function with a signature
  * roughly compatible with this:
@@ -123,14 +117,14 @@ 
  *  Error *err = NULL;
  *  Visitor *v;
  *
- *  v = ...obtain input visitor...
+ *  v = FOO_visitor_new(...);
  *  visit_type_Foo(v, NULL, &f, &err);
  *  if (err) {
  *      ...handle error...
  *  } else {
  *      ...use f...
  *  }
- *  ...clean up v...
+ *  visit_free(v);
  *  qapi_free_Foo(f);
  * </example>
  *
@@ -140,7 +134,7 @@ 
  *  Error *err = NULL;
  *  Visitor *v;
  *
- *  v = ...obtain input visitor...
+ *  v = FOO_visitor_new(...);
  *  visit_type_FooList(v, NULL, &l, &err);
  *  if (err) {
  *      ...handle error...
@@ -149,7 +143,7 @@ 
  *          ...use l->value...
  *      }
  *  }
- *  ...clean up v...
+ *  visit_free(v);
  *  qapi_free_FooList(l);
  * </example>
  *
@@ -159,13 +153,17 @@ 
  *  Foo *f = ...obtain populated object...
  *  Error *err = NULL;
  *  Visitor *v;
+ *  Type *result;
  *
- *  v = ...obtain output visitor...
+ *  v = FOO_visitor_new(..., &result);
  *  visit_type_Foo(v, NULL, &f, &err);
  *  if (err) {
  *      ...handle error...
+ *  } else {
+ *      visit_complete(v, &result);
+ *      ...use result...
  *  }
- *  ...clean up v...
+ *  visit_free(v);
  * </example>
  *
  * When visiting a real QAPI struct, this file provides several
@@ -191,7 +189,7 @@ 
  *  Error *err = NULL;
  *  int value;
  *
- *  v = ...obtain visitor...
+ *  v = FOO_visitor_new(...);
  *  visit_start_struct(v, NULL, NULL, 0, &err);
  *  if (err) {
  *      goto out;
@@ -219,7 +217,7 @@ 
  *  visit_end_struct(v, NULL);
  * out:
  *  error_propagate(errp, err);
- *  ...clean up v...
+ *  visit_free(v);
  * </example>
  */
 
@@ -243,6 +241,18 @@  typedef struct GenericAlternate {
 /*** Visitor cleanup ***/
 
 /*
+ * Complete the visit, collecting any output.
+ *
+ * May only be called only once after a successful top-level
+ * visit_type_FOO() or visit_end_ITEM(), and marks the end of the
+ * visit.  The @opaque pointer should match the output parameter
+ * passed to the subtype_visitor_new() used to create an output
+ * visitor, or NULL for any other visitor.  Needed for output
+ * visitors, but may also be called with other visitors.
+ */
+void visit_complete(Visitor *v, void *opaque);
+
+/*
  * Free @v and any resources it has tied up.
  *
  * May be called whether or not the visit has been successfully
diff --git a/net/net.c b/net/net.c
index 336469f..019aaad 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1198,7 +1198,7 @@  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
     char *str;
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
-    StringOutputVisitor *ov;
+    Visitor *v;
 
     /* generate info str */
     object_property_iter_init(&iter, OBJECT(nf));
@@ -1206,11 +1206,10 @@  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
         if (!strcmp(prop->name, "type")) {
             continue;
         }
-        ov = string_output_visitor_new(false);
-        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
-                            prop->name, NULL);
-        str = string_output_get_string(ov);
-        visit_free(string_output_get_visitor(ov));
+        v = string_output_visitor_new(false, &str);
+        object_property_get(OBJECT(nf), v, prop->name, NULL);
+        visit_complete(v, &str);
+        visit_free(v);
         monitor_printf(mon, ",%s=%s", prop->name, str);
         g_free(str);
     }
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 5f68c25..eb7dd72 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -20,6 +20,14 @@ 
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"
 
+void visit_complete(Visitor *v, void *opaque)
+{
+    assert(v->type != VISITOR_OUTPUT || v->complete);
+    if (v->complete) {
+        v->complete(v, opaque);
+    }
+}
+
 void visit_free(Visitor *v)
 {
     if (v) {
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 3d12623..0452056 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -33,6 +33,7 @@  struct QmpOutputVisitor
     Visitor visitor;
     QStack stack; /* Stack of containers that haven't yet been finished */
     QObject *root; /* Root of the output visit */
+    QObject **result; /* User's storage location for result */
 };
 
 #define qmp_output_add(qov, name, value) \
@@ -200,18 +201,17 @@  static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
 /* 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)
+static void qmp_output_complete(Visitor *v, void *opaque)
 {
+    QmpOutputVisitor *qov = to_qov(v);
+
     /* A visit must have occurred, with each start paired with end.  */
     assert(qov->root && QTAILQ_EMPTY(&qov->stack));
+    assert(opaque == qov->result);
 
     qobject_incref(qov->root);
-    return qov->root;
-}
-
-Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)
-{
-    return &v->visitor;
+    *qov->result = qov->root;
+    qov->result = NULL;
 }
 
 static void qmp_output_free(Visitor *v)
@@ -228,7 +228,7 @@  static void qmp_output_free(Visitor *v)
     g_free(qov);
 }
 
-QmpOutputVisitor *qmp_output_visitor_new(void)
+Visitor *qmp_output_visitor_new(QObject **result)
 {
     QmpOutputVisitor *v;
 
@@ -247,9 +247,12 @@  QmpOutputVisitor *qmp_output_visitor_new(void)
     v->visitor.type_number = qmp_output_type_number;
     v->visitor.type_any = qmp_output_type_any;
     v->visitor.type_null = qmp_output_type_null;
+    v->visitor.complete = qmp_output_complete;
     v->visitor.free = qmp_output_free;
 
     QTAILQ_INIT(&v->stack);
+    *result = NULL;
+    v->result = result;
 
-    return v;
+    return &v->visitor;
 }
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 78aab87..94ac821 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -58,6 +58,7 @@  struct StringOutputVisitor
     Visitor visitor;
     bool human;
     GString *string;
+    char **result;
     ListMode list_mode;
     union {
         int64_t s;
@@ -305,16 +306,13 @@  static void end_list(Visitor *v, void **obj)
     sov->list_mode = LM_NONE;
 }
 
-char *string_output_get_string(StringOutputVisitor *sov)
+static void string_output_complete(Visitor *v, void *opaque)
 {
-    char *string = g_string_free(sov->string, false);
+    StringOutputVisitor *sov = to_sov(v);
+
+    assert(opaque == sov->result);
+    *sov->result = g_string_free(sov->string, false);
     sov->string = NULL;
-    return string;
-}
-
-Visitor *string_output_get_visitor(StringOutputVisitor *sov)
-{
-    return &sov->visitor;
 }
 
 static void free_range(void *range, void *dummy)
@@ -335,7 +333,7 @@  static void string_output_free(Visitor *v)
     g_free(sov);
 }
 
-StringOutputVisitor *string_output_visitor_new(bool human)
+Visitor *string_output_visitor_new(bool human, char **result)
 {
     StringOutputVisitor *v;
 
@@ -343,6 +341,9 @@  StringOutputVisitor *string_output_visitor_new(bool human)
 
     v->string = g_string_new(NULL);
     v->human = human;
+    v->result = result;
+    *result = NULL;
+
     v->visitor.type = VISITOR_OUTPUT;
     v->visitor.type_int64 = print_type_int64;
     v->visitor.type_uint64 = print_type_uint64;
@@ -353,7 +354,8 @@  StringOutputVisitor *string_output_visitor_new(bool human)
     v->visitor.start_list = start_list;
     v->visitor.next_list = next_list;
     v->visitor.end_list = end_list;
+    v->visitor.complete = string_output_complete;
     v->visitor.free = string_output_free;
 
-    return v;
+    return &v->visitor;
 }
diff --git a/qemu-img.c b/qemu-img.c
index 728f471..4dfb27d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -491,16 +491,16 @@  fail:
 static void dump_json_image_check(ImageCheck *check, bool quiet)
 {
     QString *str;
-    QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
-    visit_type_ImageCheck(qmp_output_get_visitor(ov), NULL, &check,
-                          &error_abort);
-    obj = qmp_output_get_qobject(ov);
+    Visitor *v = qmp_output_visitor_new(&obj);
+
+    visit_type_ImageCheck(v, NULL, &check, &error_abort);
+    visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
     qprintf(quiet, "%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    visit_free(qmp_output_get_visitor(ov));
+    visit_free(v);
     QDECREF(str);
 }
 
@@ -2181,32 +2181,32 @@  static void dump_snapshots(BlockDriverState *bs)
 static void dump_json_image_info_list(ImageInfoList *list)
 {
     QString *str;
-    QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
-    visit_type_ImageInfoList(qmp_output_get_visitor(ov), NULL, &list,
-                             &error_abort);
-    obj = qmp_output_get_qobject(ov);
+    Visitor *v = qmp_output_visitor_new(&obj);
+
+    visit_type_ImageInfoList(v, NULL, &list, &error_abort);
+    visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    visit_free(qmp_output_get_visitor(ov));
+    visit_free(v);
     QDECREF(str);
 }
 
 static void dump_json_image_info(ImageInfo *info)
 {
     QString *str;
-    QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
-    visit_type_ImageInfo(qmp_output_get_visitor(ov), NULL, &info,
-                         &error_abort);
-    obj = qmp_output_get_qobject(ov);
+    Visitor *v = qmp_output_visitor_new(&obj);
+
+    visit_type_ImageInfo(v, NULL, &info, &error_abort);
+    visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    visit_free(qmp_output_get_visitor(ov));
+    visit_free(v);
     QDECREF(str);
 }
 
diff --git a/qom/object.c b/qom/object.c
index 2407b66..8166b7d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1221,7 +1221,6 @@  int object_property_get_enum(Object *obj, const char *name,
                              const char *typename, Error **errp)
 {
     Error *err = NULL;
-    StringOutputVisitor *sov;
     Visitor *v;
     char *str;
     int ret;
@@ -1241,15 +1240,14 @@  int object_property_get_enum(Object *obj, const char *name,
 
     enumprop = prop->opaque;
 
-    sov = string_output_visitor_new(false);
-    v = string_output_get_visitor(sov);
+    v = string_output_visitor_new(false, &str);
     object_property_get(obj, v, name, &err);
     if (err) {
         error_propagate(errp, err);
         visit_free(v);
         return 0;
     }
-    str = string_output_get_string(sov);
+    visit_complete(v, &str);
     visit_free(v);
     v = string_input_visitor_new(str);
     visit_type_enum(v, name, &ret, enumprop->strings, errp);
@@ -1264,25 +1262,23 @@  void object_property_get_uint16List(Object *obj, const char *name,
                                     uint16List **list, Error **errp)
 {
     Error *err = NULL;
-    StringOutputVisitor *ov;
     Visitor *v;
     char *str;
 
-    ov = string_output_visitor_new(false);
-    object_property_get(obj, string_output_get_visitor(ov),
-                        name, &err);
+    v = string_output_visitor_new(false, &str);
+    object_property_get(obj, v, name, &err);
     if (err) {
         error_propagate(errp, err);
         goto out;
     }
-    str = string_output_get_string(ov);
+    visit_complete(v, &str);
+    visit_free(v);
     v = string_input_visitor_new(str);
     visit_type_uint16List(v, NULL, list, errp);
 
     g_free(str);
-    visit_free(v);
 out:
-    visit_free(string_output_get_visitor(ov));
+    visit_free(v);
 }
 
 void object_property_parse(Object *obj, const char *string,
@@ -1296,21 +1292,21 @@  void object_property_parse(Object *obj, const char *string,
 char *object_property_print(Object *obj, const char *name, bool human,
                             Error **errp)
 {
-    StringOutputVisitor *sov;
+    Visitor *v;
     char *string = NULL;
     Error *local_err = NULL;
 
-    sov = string_output_visitor_new(human);
-    object_property_get(obj, string_output_get_visitor(sov), name, &local_err);
+    v = string_output_visitor_new(human, &string);
+    object_property_get(obj, v, name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out;
     }
 
-    string = string_output_get_string(sov);
+    visit_complete(v, &string);
 
 out:
-    visit_free(string_output_get_visitor(sov));
+    visit_free(v);
     return string;
 }
 
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index 6714565..c225abc 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -33,14 +33,14 @@  QObject *object_property_get_qobject(Object *obj, const char *name,
 {
     QObject *ret = NULL;
     Error *local_err = NULL;
-    QmpOutputVisitor *qov;
+    Visitor *v;
 
-    qov = qmp_output_visitor_new();
-    object_property_get(obj, qmp_output_get_visitor(qov), name, &local_err);
+    v = qmp_output_visitor_new(&ret);
+    object_property_get(obj, v, name, &local_err);
     if (!local_err) {
-        ret = qmp_output_get_qobject(qov);
+        visit_complete(v, &ret);
     }
     error_propagate(errp, local_err);
-    visit_free(qmp_output_get_visitor(qov));
+    visit_free(v);
     return ret;
 }
diff --git a/replay/replay-input.c b/replay/replay-input.c
index 9cbb4c1..296399c 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -22,15 +22,13 @@ 
 
 static InputEvent *qapi_clone_InputEvent(InputEvent *src)
 {
-    QmpOutputVisitor *qov;
     Visitor *ov, *iv;
     QObject *obj;
     InputEvent *dst = NULL;
 
-    qov = qmp_output_visitor_new();
-    ov = qmp_output_get_visitor(qov);
+    ov = qmp_output_visitor_new(&obj);
     visit_type_InputEvent(ov, NULL, &src, &error_abort);
-    obj = qmp_output_get_qobject(qov);
+    visit_complete(ov, &obj);
     visit_free(ov);
     if (!obj) {
         return NULL;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 49d4ce2..34b6a3a 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -61,17 +61,13 @@  def gen_marshal_output(ret_type):
 static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
 {
     Error *err = NULL;
-    QmpOutputVisitor *qov = qmp_output_visitor_new();
     Visitor *v;
 
-    v = qmp_output_get_visitor(qov);
+    v = qmp_output_visitor_new(ret_out);
     visit_type_%(c_name)s(v, "unused", &ret_in, &err);
-    if (err) {
-        goto out;
+    if (!err) {
+        visit_complete(v, ret_out);
     }
-    *ret_out = qmp_output_get_qobject(qov);
-
-out:
     error_propagate(errp, err);
     visit_free(v);
     v = qapi_dealloc_visitor_new();
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 909e8d6..9c88627 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -71,7 +71,7 @@  def gen_event_send(name, arg_type):
 
     if arg_type and arg_type.members:
         ret += mcgen('''
-    QmpOutputVisitor *qov;
+    QObject *obj;
     Visitor *v;
 ''')
         ret += gen_param_var(arg_type)
@@ -90,8 +90,7 @@  def gen_event_send(name, arg_type):
 
     if arg_type and arg_type.members:
         ret += mcgen('''
-    qov = qmp_output_visitor_new();
-    v = qmp_output_get_visitor(qov);
+    v = qmp_output_visitor_new(&obj);
 
     visit_start_struct(v, "%(name)s", NULL, 0, &err);
     if (err) {
@@ -106,7 +105,8 @@  def gen_event_send(name, arg_type):
         goto out;
     }
 
-    qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
+    visit_complete(v, &obj);
+    qdict_put_obj(qmp, "data", obj);
 ''',
                      name=name, c_name=arg_type.c_name())
 
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index 6310bf7..dc906b1 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -37,7 +37,6 @@  static void qnull_ref_test(void)
 static void qnull_visit_test(void)
 {
     QObject *obj;
-    QmpOutputVisitor *qov;
     Visitor *v;
 
     /*
@@ -53,12 +52,12 @@  static void qnull_visit_test(void)
     visit_type_null(v, NULL, &error_abort);
     visit_free(v);
 
-    qov = qmp_output_visitor_new();
-    visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort);
-    obj = qmp_output_get_qobject(qov);
+    v = qmp_output_visitor_new(&obj);
+    visit_type_null(v, NULL, &error_abort);
+    visit_complete(v, &obj);
     g_assert(obj == &qnull_);
     qobject_decref(obj);
-    visit_free(qmp_output_get_visitor(qov));
+    visit_free(v);
 
     g_assert(qnull_.refcnt == 1);
 }
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index af24b10..513d71f 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -21,7 +21,6 @@ 
 #include "qapi/qmp/qjson.h"
 
 typedef struct TestOutputVisitorData {
-    QmpOutputVisitor *qov;
     Visitor *ov;
     QObject *obj;
 } TestOutputVisitorData;
@@ -29,18 +28,14 @@  typedef struct TestOutputVisitorData {
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->qov = qmp_output_visitor_new();
-    g_assert(data->qov != NULL);
-
-    data->ov = qmp_output_get_visitor(data->qov);
-    g_assert(data->ov != NULL);
+    data->ov = qmp_output_visitor_new(&data->obj);
+    g_assert(data->ov);
 }
 
 static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
     visit_free(data->ov);
-    data->qov = NULL;
     data->ov = NULL;
     qobject_decref(data->obj);
     data->obj = NULL;
@@ -48,7 +43,7 @@  static void visitor_output_teardown(TestOutputVisitorData *data,
 
 static QObject *visitor_get(TestOutputVisitorData *data)
 {
-    data->obj = qmp_output_get_qobject(data->qov);
+    visit_complete(data->ov, &data->obj);
     g_assert(data->obj);
     return data->obj;
 }
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index e17035b..444844a 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -20,7 +20,6 @@ 
 #include "qapi/qmp/types.h"
 
 typedef struct TestOutputVisitorData {
-    StringOutputVisitor *sov;
     Visitor *ov;
     char *str;
     bool human;
@@ -30,9 +29,7 @@  static void visitor_output_setup_internal(TestOutputVisitorData *data,
                                           bool human)
 {
     data->human = human;
-    data->sov = string_output_visitor_new(human);
-    g_assert(data->sov);
-    data->ov = string_output_get_visitor(data->sov);
+    data->ov = string_output_visitor_new(human, &data->str);
     g_assert(data->ov);
 }
 
@@ -52,7 +49,6 @@  static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
     visit_free(data->ov);
-    data->sov = NULL;
     data->ov = NULL;
     g_free(data->str);
     data->str = NULL;
@@ -60,7 +56,7 @@  static void visitor_output_teardown(TestOutputVisitorData *data,
 
 static char *visitor_get(TestOutputVisitorData *data)
 {
-    data->str = string_output_get_string(data->sov);
+    visit_complete(data->ov, &data->str);
     g_assert(data->str);
     return data->str;
 }
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 377ee3e..dba4670 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1012,7 +1012,8 @@  static PrimitiveType pt_values[] = {
 /* visitor-specific op implementations */
 
 typedef struct QmpSerializeData {
-    QmpOutputVisitor *qov;
+    Visitor *qov;
+    QObject *obj;
     Visitor *qiv;
 } QmpSerializeData;
 
@@ -1021,8 +1022,8 @@  static void qmp_serialize(void *native_in, void **datap,
 {
     QmpSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->qov = qmp_output_visitor_new();
-    visit(qmp_output_get_visitor(d->qov), &native_in, errp);
+    d->qov = qmp_output_visitor_new(&d->obj);
+    visit(d->qov, &native_in, errp);
     *datap = d;
 }
 
@@ -1033,7 +1034,8 @@  static void qmp_deserialize(void **native_out, void *datap,
     QString *output_json;
     QObject *obj_orig, *obj;
 
-    obj_orig = qmp_output_get_qobject(d->qov);
+    visit_complete(d->qov, &d->obj);
+    obj_orig = d->obj;
     output_json = qobject_to_json(obj_orig);
     obj = qobject_from_json(qstring_get_str(output_json));
 
@@ -1047,7 +1049,7 @@  static void qmp_deserialize(void **native_out, void *datap,
 static void qmp_cleanup(void *datap)
 {
     QmpSerializeData *d = datap;
-    visit_free(qmp_output_get_visitor(d->qov));
+    visit_free(d->qov);
     visit_free(d->qiv);
 
     g_free(d);
@@ -1055,7 +1057,7 @@  static void qmp_cleanup(void *datap)
 
 typedef struct StringSerializeData {
     char *string;
-    StringOutputVisitor *sov;
+    Visitor *sov;
     Visitor *siv;
 } StringSerializeData;
 
@@ -1064,8 +1066,8 @@  static void string_serialize(void *native_in, void **datap,
 {
     StringSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->sov = string_output_visitor_new(false);
-    visit(string_output_get_visitor(d->sov), &native_in, errp);
+    d->sov = string_output_visitor_new(false, &d->string);
+    visit(d->sov, &native_in, errp);
     *datap = d;
 }
 
@@ -1074,7 +1076,7 @@  static void string_deserialize(void **native_out, void *datap,
 {
     StringSerializeData *d = datap;
 
-    d->string = string_output_get_string(d->sov);
+    visit_complete(d->sov, &d->string);
     d->siv = string_input_visitor_new(d->string);
     visit(d->siv, native_out, errp);
 }
@@ -1083,7 +1085,7 @@  static void string_cleanup(void *datap)
 {
     StringSerializeData *d = datap;
 
-    visit_free(string_output_get_visitor(d->sov));
+    visit_free(d->sov);
     visit_free(d->siv);
     g_free(d->string);
     g_free(d);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 01fc154..a0ca6d4 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1147,16 +1147,14 @@  SocketAddress *socket_remote_address(int fd, Error **errp)
 void qapi_copy_SocketAddress(SocketAddress **p_dest,
                              SocketAddress *src)
 {
-    QmpOutputVisitor *qov;
     Visitor *ov, *iv;
     QObject *obj;
 
     *p_dest = NULL;
 
-    qov = qmp_output_visitor_new();
-    ov = qmp_output_get_visitor(qov);
+    ov = qmp_output_visitor_new(&obj);
     visit_type_SocketAddress(ov, NULL, &src, &error_abort);
-    obj = qmp_output_get_qobject(qov);
+    visit_complete(ov, &obj);
     visit_free(ov);
     if (!obj) {
         return;