qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06
@ 2016-07-06  9:13 Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 01/15] qapi: Improve use of qmp/types.h Markus Armbruster
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 791b7d2340cfafcac9af7864343cf23504d57804:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-07-05 16:48:24 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2016-07-06

for you to fetch changes up to b6954712abea03afd686b724060f9873e2c61f2b:

  replay: Use new QAPI cloning (2016-07-06 10:52:04 +0200)

----------------------------------------------------------------
QAPI patches for 2016-07-06

----------------------------------------------------------------
Eric Blake (15):
      qapi: Improve use of qmp/types.h
      qemu-img: Don't leak errors when outputting JSON
      qapi: Add parameter to visit_end_*
      qapi: Add new visit_free() function
      opts-visitor: Favor new visit_free() function
      string-input-visitor: Favor new visit_free() function
      qmp-input-visitor: Favor new visit_free() function
      string-output-visitor: Favor new visit_free() function
      qmp-output-visitor: Favor new visit_free() function
      tests: Clean up test-string-output-visitor
      tests: Factor out common code in qapi output tests
      qapi: Add new visit_complete() function
      qapi: Add new clone visitor
      sockets: Use new QAPI cloning
      replay: Use new QAPI cloning

 block/crypto.c                       |  30 +++--
 block/qapi.c                         |   9 +-
 blockdev.c                           |   9 +-
 docs/qapi-code-gen.txt               |  49 +++------
 hmp.c                                |  19 ++--
 hw/acpi/core.c                       |   8 +-
 hw/pci/pcie_aer.c                    |   1 +
 hw/ppc/spapr_drc.c                   |   4 +-
 hw/virtio/virtio-balloon.c           |   4 +-
 include/io/task.h                    |   2 +-
 include/qapi/clone-visitor.h         |  39 +++++++
 include/qapi/dealloc-visitor.h       |   5 +-
 include/qapi/opts-visitor.h          |   4 +-
 include/qapi/qmp-input-visitor.h     |   6 +-
 include/qapi/qmp-output-visitor.h    |  12 +-
 include/qapi/qmp/types.h             |   1 -
 include/qapi/string-input-visitor.h  |   5 +-
 include/qapi/string-output-visitor.h |  14 ++-
 include/qapi/visitor-impl.h          |  26 +++--
 include/qapi/visitor.h               | 161 ++++++++++++++++++---------
 include/qemu/sockets.h               |   4 -
 io/channel-socket.c                  |   9 +-
 monitor.c                            |   6 +-
 net/net.c                            |  16 ++-
 numa.c                               |   6 +-
 qapi/Makefile.objs                   |   2 +-
 qapi/opts-visitor.c                  |  38 +++----
 qapi/qapi-clone-visitor.c            | 182 +++++++++++++++++++++++++++++++
 qapi/qapi-dealloc-visitor.c          |  61 ++---------
 qapi/qapi-visit-core.c               |  55 +++++++---
 qapi/qmp-dispatch.c                  |   1 +
 qapi/qmp-input-visitor.c             |  27 ++---
 qapi/qmp-output-visitor.c            |  56 ++++++----
 qapi/string-input-visitor.c          |  25 +++--
 qapi/string-output-visitor.c         |  32 ++++--
 qemu-char.c                          |   5 +-
 qemu-img.c                           |  32 +++---
 qmp.c                                |   9 +-
 qobject/json-parser.c                |   7 +-
 qobject/qjson.c                      |   6 +-
 qobject/qobject.c                    |   7 +-
 qom/object.c                         |  58 +++++-----
 qom/object_interfaces.c              |  12 +-
 qom/qom-qobject.c                    |  19 ++--
 replay/replay-input.c                |  34 +-----
 scripts/qapi-commands.py             |  33 ++----
 scripts/qapi-event.py                |  12 +-
 scripts/qapi-types.py                |   6 +-
 scripts/qapi-visit.py                |   8 +-
 tests/.gitignore                     |   1 +
 tests/Makefile.include               |   4 +
 tests/check-qjson.c                  |   8 +-
 tests/check-qnull.c                  |  17 ++-
 tests/test-clone-visitor.c           | 206 +++++++++++++++++++++++++++++++++++
 tests/test-opts-visitor.c            |   9 +-
 tests/test-qmp-commands.c            |   8 +-
 tests/test-qmp-input-strict.c        |  13 +--
 tests/test-qmp-input-visitor.c       |  15 +--
 tests/test-qmp-output-visitor.c      |  92 ++++++----------
 tests/test-string-input-visitor.c    |  22 ++--
 tests/test-string-output-visitor.c   |  99 ++++++++---------
 tests/test-visitor-serialization.c   |  41 +++----
 util/qemu-sockets.c                  |  27 -----
 63 files changed, 1025 insertions(+), 713 deletions(-)
 create mode 100644 include/qapi/clone-visitor.h
 create mode 100644 qapi/qapi-clone-visitor.c
 create mode 100644 tests/test-clone-visitor.c

-- 
2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 01/15] qapi: Improve use of qmp/types.h
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 02/15] qemu-img: Don't leak errors when outputting JSON Markus Armbruster
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

'qjson.h' is not a QObject subtype; include this file directly in
.c files that are using it, rather than abusing qmp/types.h for
that purpose.

Meanwhile, for files that include a list of individual QObject
subtypes, it's easier to just use qmp/types.h for that purpose.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-2-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pcie_aer.c                  | 1 +
 include/qapi/qmp/types.h           | 1 -
 monitor.c                          | 6 +-----
 qapi/qmp-dispatch.c                | 1 +
 qobject/json-parser.c              | 7 +------
 qobject/qjson.c                    | 6 +-----
 qobject/qobject.c                  | 7 +------
 tests/check-qjson.c                | 8 +-------
 tests/test-qmp-input-strict.c      | 1 +
 tests/test-qmp-input-visitor.c     | 1 +
 tests/test-qmp-output-visitor.c    | 1 +
 tests/test-visitor-serialization.c | 1 +
 12 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index e2d4e68..048ce6a 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qjson.h"
 #include "monitor/monitor.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pcie.h"
diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
index 7782ec5..f21ecf4 100644
--- a/include/qapi/qmp/types.h
+++ b/include/qapi/qmp/types.h
@@ -20,6 +20,5 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qjson.h"
 
 #endif /* QEMU_OBJECTS_H */
diff --git a/monitor.c b/monitor.c
index 6f960f1..ed09cdc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -54,11 +54,7 @@
 #include "qemu/acl.h"
 #include "sysemu/tpm.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qint.h"
-#include "qapi/qmp/qfloat.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/types.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 08faf85..505eb41 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -16,6 +16,7 @@
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/dispatch.h"
 #include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/qjson.h"
 #include "qapi-types.h"
 #include "qapi/qmp/qerror.h"
 
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 67ed727..c18e48a 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -14,12 +14,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qint.h"
-#include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qfloat.h"
-#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/types.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-lexer.h"
 #include "qapi/qmp/json-streamer.h"
diff --git a/qobject/qjson.c b/qobject/qjson.c
index ef160d2..9a0de89 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -16,11 +16,7 @@
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qint.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qfloat.h"
-#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/types.h"
 #include "qemu/unicode.h"
 
 typedef struct JSONParsingState
diff --git a/qobject/qobject.c b/qobject/qobject.c
index cd41fb9..fe4fa10 100644
--- a/qobject/qobject.c
+++ b/qobject/qobject.c
@@ -9,12 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qfloat.h"
-#include "qapi/qmp/qint.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/types.h"
 
 static void (*qdestroy[QTYPE__MAX])(QObject *) = {
     [QTYPE_NONE] = NULL,               /* No such object exists */
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 0e158f6..8595574 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -12,14 +12,8 @@
  */
 #include "qemu/osdep.h"
 
-#include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qint.h"
-#include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qfloat.h"
-#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/types.h"
 #include "qapi/qmp/qjson.h"
-
 #include "qemu-common.h"
 
 static void escaped_string(void)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index d5f57c5..79739e9 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -19,6 +19,7 @@
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qjson.h"
 #include "test-qmp-introspect.h"
 #include "qmp-introspect.h"
 #include "qapi-visit.h"
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 1a4585c..17cda5a 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -18,6 +18,7 @@
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qjson.h"
 
 typedef struct TestInputVisitorData {
     QObject *obj;
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index f8a7a27..e030c67 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -18,6 +18,7 @@
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qjson.h"
 
 typedef struct TestOutputVisitorData {
     QmpOutputVisitor *qov;
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 777469f..db781c0 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -19,6 +19,7 @@
 #include "test-qapi-visit.h"
 #include "qapi/error.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qjson.h"
 #include "qapi/qmp-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/string-input-visitor.h"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 02/15] qemu-img: Don't leak errors when outputting JSON
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 01/15] qapi: Improve use of qmp/types.h Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 03/15] qapi: Add parameter to visit_end_* Markus Armbruster
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

If our JSON output ever encounters an error, we would just silently
leak the error object.  Instead, assert that our usage won't fail.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-3-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-img.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3322a1e..debd7f1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -490,12 +490,11 @@ fail:
 
 static void dump_json_image_check(ImageCheck *check, bool quiet)
 {
-    Error *local_err = NULL;
     QString *str;
     QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
     visit_type_ImageCheck(qmp_output_get_visitor(ov), NULL, &check,
-                          &local_err);
+                          &error_abort);
     obj = qmp_output_get_qobject(ov);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
@@ -2181,12 +2180,11 @@ static void dump_snapshots(BlockDriverState *bs)
 
 static void dump_json_image_info_list(ImageInfoList *list)
 {
-    Error *local_err = NULL;
     QString *str;
     QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
     visit_type_ImageInfoList(qmp_output_get_visitor(ov), NULL, &list,
-                             &local_err);
+                             &error_abort);
     obj = qmp_output_get_qobject(ov);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
@@ -2198,11 +2196,11 @@ static void dump_json_image_info_list(ImageInfoList *list)
 
 static void dump_json_image_info(ImageInfo *info)
 {
-    Error *local_err = NULL;
     QString *str;
     QmpOutputVisitor *ov = qmp_output_visitor_new();
     QObject *obj;
-    visit_type_ImageInfo(qmp_output_get_visitor(ov), NULL, &info, &local_err);
+    visit_type_ImageInfo(qmp_output_get_visitor(ov), NULL, &info,
+                         &error_abort);
     obj = qmp_output_get_qobject(ov);
     str = qobject_to_json_pretty(obj);
     assert(str != NULL);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 03/15] qapi: Add parameter to visit_end_*
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 01/15] qapi: Improve use of qmp/types.h Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 02/15] qemu-img: Don't leak errors when outputting JSON Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 04/15] qapi: Add new visit_free() function Markus Armbruster
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Rather than making the dealloc visitor track of stack of pointers
remembered during visit_start_* in order to free them during
visit_end_*, it's a lot easier to just make all callers pass the
same pointer to visit_end_*.  The generated code has access to the
same pointer, while all other users are doing virtual walks and
can pass NULL.  The dealloc visitor is then greatly simplified.

All three visit_end_*() functions intentionally take a void**,
even though the visit_start_*() functions differ between void**,
GenericList**, and GenericAlternate**.  This is done for several
reasons: when doing a virtual walk, passing NULL doesn't care
what the type is, but when doing a generated walk, we already
have to cast the caller's specific FOO* to call visit_start,
while using void** lets us use visit_end without a cast. Also,
an upcoming patch will add a clone visitor that wants to use
the same implementation for all three visit_end callbacks,
which is made easier if all three share the same signature.

For visitors with already track per-object state (the QMP visitors
via a stack, and the string visitors which do not allow nesting),
add an assertion that the caller is indeed passing the same
pointer to paired calls.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-4-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/crypto.c                  |  4 ++--
 docs/qapi-code-gen.txt          |  8 +++----
 hw/ppc/spapr_drc.c              |  4 ++--
 hw/virtio/virtio-balloon.c      |  4 ++--
 include/qapi/visitor-impl.h     |  6 +++---
 include/qapi/visitor.h          | 32 ++++++++++++++++------------
 qapi/opts-visitor.c             |  4 ++--
 qapi/qapi-dealloc-visitor.c     | 47 +++--------------------------------------
 qapi/qapi-visit-core.c          | 12 +++++------
 qapi/qmp-input-visitor.c        | 11 ++++++----
 qapi/qmp-output-visitor.c       | 23 ++++++++++++--------
 qapi/string-input-visitor.c     |  7 +++++-
 qapi/string-output-visitor.c    |  5 ++++-
 qom/object.c                    |  2 +-
 qom/object_interfaces.c         |  4 ++--
 scripts/qapi-commands.py        |  4 ++--
 scripts/qapi-event.py           |  2 +-
 scripts/qapi-visit.py           |  8 +++----
 tests/test-qmp-input-visitor.c  |  2 +-
 tests/test-qmp-output-visitor.c |  2 +-
 20 files changed, 86 insertions(+), 105 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 758e14e..5f0ab4d 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -222,7 +222,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
         visit_check_struct(opts_get_visitor(ov), &local_err);
     }
 
-    visit_end_struct(opts_get_visitor(ov));
+    visit_end_struct(opts_get_visitor(ov), NULL);
 
  out:
     if (local_err) {
@@ -269,7 +269,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
         visit_check_struct(opts_get_visitor(ov), &local_err);
     }
 
-    visit_end_struct(opts_get_visitor(ov));
+    visit_end_struct(opts_get_visitor(ov), NULL);
 
  out:
     if (local_err) {
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index eff2075..0ae239a 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -904,7 +904,7 @@ Example:
         }
         visit_check_struct(v, &err);
     out_obj:
-        visit_end_struct(v);
+        visit_end_struct(v, (void **)obj);
         if (err && visit_is_input(v)) {
             qapi_free_UserDefOne(*obj);
             *obj = NULL;
@@ -932,7 +932,7 @@ Example:
             }
         }
 
-        visit_end_list(v);
+        visit_end_list(v, (void **)obj);
         if (err && visit_is_input(v)) {
             qapi_free_UserDefOneList(*obj);
             *obj = NULL;
@@ -1022,7 +1022,7 @@ Example:
         if (!err) {
             visit_check_struct(v, &err);
         }
-        visit_end_struct(v);
+        visit_end_struct(v, NULL);
         if (err) {
             goto out;
         }
@@ -1041,7 +1041,7 @@ Example:
         v = qapi_dealloc_get_visitor(qdv);
         visit_start_struct(v, NULL, NULL, 0, NULL);
         visit_type_UserDefOneList(v, "arg1", &arg1, NULL);
-        visit_end_struct(v);
+        visit_end_struct(v, NULL);
         qapi_dealloc_visitor_cleanup(qdv);
     }
 
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index d276db3a..26a0679 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -300,7 +300,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
             /* shouldn't ever see an FDT_END_NODE before FDT_BEGIN_NODE */
             g_assert(fdt_depth > 0);
             visit_check_struct(v, &err);
-            visit_end_struct(v);
+            visit_end_struct(v, NULL);
             if (err) {
                 error_propagate(errp, err);
                 return;
@@ -323,7 +323,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
                     return;
                 }
             }
-            visit_end_list(v);
+            visit_end_list(v, NULL);
             break;
         }
         default:
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 557d3f9..1a22e6d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -139,13 +139,13 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
     }
     visit_check_struct(v, &err);
 out_nested:
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
 
     if (!err) {
         visit_check_struct(v, &err);
     }
 out_end:
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
 out:
     error_propagate(errp, err);
 }
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 145afd0..a495bf0 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -47,7 +47,7 @@ struct Visitor
     void (*check_struct)(Visitor *v, Error **errp);
 
     /* Must be set to visit structs */
-    void (*end_struct)(Visitor *v);
+    void (*end_struct)(Visitor *v, void **obj);
 
     /* Must be set; implementations may require @list to be non-null,
      * but must document it. */
@@ -58,7 +58,7 @@ struct Visitor
     GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
 
     /* Must be set */
-    void (*end_list)(Visitor *v);
+    void (*end_list)(Visitor *v, void **list);
 
     /* Must be set by input and dealloc visitors to visit alternates;
      * optional for output visitors. */
@@ -67,7 +67,7 @@ struct Visitor
                             bool promote_int, Error **errp);
 
     /* Optional, needed for dealloc visitor */
-    void (*end_alternate)(Visitor *v);
+    void (*end_alternate)(Visitor *v, void **obj);
 
     /* Must be set */
     void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4d12167..25d0cc2 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -193,12 +193,12 @@
  *      goto outlist;
  *  }
  * outlist:
- *  visit_end_list(v);
+ *  visit_end_list(v, NULL);
  *  if (!err) {
  *      visit_check_struct(v, &err);
  *  }
  * outobj:
- *  visit_end_struct(v);
+ *  visit_end_struct(v, NULL);
  * out:
  *  error_propagate(errp, err);
  *  ...clean up v...
@@ -242,8 +242,8 @@ typedef struct GenericAlternate {
  * After visit_start_struct() succeeds, the caller may visit its
  * members one after the other, passing the member's name and address
  * within the struct.  Finally, visit_end_struct() needs to be called
- * to clean up, even if intermediate visits fail.  See the examples
- * above.
+ * with the same @obj to clean up, even if intermediate visits fail.
+ * See the examples above.
  *
  * FIXME Should this be named visit_start_object, since it is also
  * used for QAPI unions, and maps to JSON objects?
@@ -267,12 +267,14 @@ void visit_check_struct(Visitor *v, Error **errp);
 /*
  * Complete an object visit started earlier.
  *
+ * @obj must match what was passed to the paired visit_start_struct().
+ *
  * Must be called after any successful use of visit_start_struct(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
  * behaves as if this was implicitly called.
  */
-void visit_end_struct(Visitor *v);
+void visit_end_struct(Visitor *v, void **obj);
 
 
 /*** Visiting lists ***/
@@ -299,8 +301,9 @@ void visit_end_struct(Visitor *v);
  * visit (where @obj is NULL) uses other means.  For each list
  * element, call the appropriate visit_type_FOO() with name set to
  * NULL and obj set to the address of the value member of the list
- * element.  Finally, visit_end_list() needs to be called to clean up,
- * even if intermediate visits fail.  See the examples above.
+ * element.  Finally, visit_end_list() needs to be called with the
+ * same @list to clean up, even if intermediate visits fail.  See the
+ * examples above.
  */
 void visit_start_list(Visitor *v, const char *name, GenericList **list,
                       size_t size, Error **errp);
@@ -324,12 +327,14 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
 /*
  * Complete a list visit started earlier.
  *
+ * @list must match what was passed to the paired visit_start_list().
+ *
  * Must be called after any successful use of visit_start_list(), even
  * if intermediate processing was skipped due to errors, to allow the
  * backend to release any resources.  Destroying the visitor early
  * behaves as if this was implicitly called.
  */
-void visit_end_list(Visitor *v);
+void visit_end_list(Visitor *v, void **list);
 
 
 /*** Visiting alternates ***/
@@ -347,8 +352,9 @@ void visit_end_list(Visitor *v);
  *
  * If @promote_int, treat integers as QTYPE_FLOAT.
  *
- * If successful, this must be paired with visit_end_alternate() to
- * clean up, even if visiting the contents of the alternate fails.
+ * If successful, this must be paired with visit_end_alternate() with
+ * the same @obj to clean up, even if visiting the contents of the
+ * alternate fails.
  */
 void visit_start_alternate(Visitor *v, const char *name,
                            GenericAlternate **obj, size_t size,
@@ -357,15 +363,15 @@ void visit_start_alternate(Visitor *v, const char *name,
 /*
  * Finish visiting an alternate type.
  *
+ * @obj must match what was passed to the paired visit_start_alternate().
+ *
  * Must be called after any successful use of visit_start_alternate(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
  * behaves as if this was implicitly called.
  *
- * TODO: Should all the visit_end_* interfaces take obj parameter, so
- * that dealloc visitor need not track what was passed in visit_start?
  */
-void visit_end_alternate(Visitor *v);
+void visit_end_alternate(Visitor *v, void **obj);
 
 
 /*** Other helpers ***/
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 4cf1cf8..dcfbf92 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -180,7 +180,7 @@ opts_check_struct(Visitor *v, Error **errp)
 
 
 static void
-opts_end_struct(Visitor *v)
+opts_end_struct(Visitor *v, void **obj)
 {
     OptsVisitor *ov = to_ov(v);
 
@@ -273,7 +273,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
 
 
 static void
-opts_end_list(Visitor *v)
+opts_end_list(Visitor *v, void **obj)
 {
     OptsVisitor *ov = to_ov(v);
 
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index cd68b55..9391dea 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -19,53 +19,18 @@
 #include "qapi/qmp/types.h"
 #include "qapi/visitor-impl.h"
 
-typedef struct StackEntry
-{
-    void *value;
-    QTAILQ_ENTRY(StackEntry) node;
-} StackEntry;
-
 struct QapiDeallocVisitor
 {
     Visitor visitor;
-    QTAILQ_HEAD(, StackEntry) stack;
 };
 
-static QapiDeallocVisitor *to_qov(Visitor *v)
-{
-    return container_of(v, QapiDeallocVisitor, visitor);
-}
-
-static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
-{
-    StackEntry *e = g_malloc0(sizeof(*e));
-
-    e->value = value;
-
-    QTAILQ_INSERT_HEAD(&qov->stack, e, node);
-}
-
-static void *qapi_dealloc_pop(QapiDeallocVisitor *qov)
-{
-    StackEntry *e = QTAILQ_FIRST(&qov->stack);
-    QObject *value;
-    QTAILQ_REMOVE(&qov->stack, e, node);
-    value = e->value;
-    g_free(e);
-    return value;
-}
-
 static void qapi_dealloc_start_struct(Visitor *v, const char *name, void **obj,
                                       size_t unused, Error **errp)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    qapi_dealloc_push(qov, obj);
 }
 
-static void qapi_dealloc_end_struct(Visitor *v)
+static void qapi_dealloc_end_struct(Visitor *v, void **obj)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    void **obj = qapi_dealloc_pop(qov);
     if (obj) {
         g_free(*obj);
     }
@@ -75,14 +40,10 @@ static void qapi_dealloc_start_alternate(Visitor *v, const char *name,
                                          GenericAlternate **obj, size_t size,
                                          bool promote_int, Error **errp)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    qapi_dealloc_push(qov, obj);
 }
 
-static void qapi_dealloc_end_alternate(Visitor *v)
+static void qapi_dealloc_end_alternate(Visitor *v, void **obj)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    void **obj = qapi_dealloc_pop(qov);
     if (obj) {
         g_free(*obj);
     }
@@ -102,7 +63,7 @@ static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList *tail,
     return next;
 }
 
-static void qapi_dealloc_end_list(Visitor *v)
+static void qapi_dealloc_end_list(Visitor *v, void **obj)
 {
 }
 
@@ -178,7 +139,5 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.type_any = qapi_dealloc_type_anything;
     v->visitor.type_null = qapi_dealloc_type_null;
 
-    QTAILQ_INIT(&v->stack);
-
     return v;
 }
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index eada467..dba11c6 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -43,9 +43,9 @@ void visit_check_struct(Visitor *v, Error **errp)
     }
 }
 
-void visit_end_struct(Visitor *v)
+void visit_end_struct(Visitor *v, void **obj)
 {
-    v->end_struct(v);
+    v->end_struct(v, obj);
 }
 
 void visit_start_list(Visitor *v, const char *name, GenericList **list,
@@ -67,9 +67,9 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size)
     return v->next_list(v, tail, size);
 }
 
-void visit_end_list(Visitor *v)
+void visit_end_list(Visitor *v, void **obj)
 {
-    v->end_list(v);
+    v->end_list(v, obj);
 }
 
 void visit_start_alternate(Visitor *v, const char *name,
@@ -89,10 +89,10 @@ void visit_start_alternate(Visitor *v, const char *name,
     error_propagate(errp, err);
 }
 
-void visit_end_alternate(Visitor *v)
+void visit_end_alternate(Visitor *v, void **obj)
 {
     if (v->end_alternate) {
-        v->end_alternate(v);
+        v->end_alternate(v, obj);
     }
 }
 
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index aea90a1..e16b4b0 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -26,6 +26,7 @@
 typedef struct StackObject
 {
     QObject *obj; /* Object being visited */
+    void *qapi; /* sanity check that caller uses same pointer */
 
     GHashTable *h;           /* If obj is dict: unvisited keys */
     const QListEntry *entry; /* If obj is list: unvisited tail */
@@ -96,7 +97,7 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
 }
 
 static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
-                                        Error **errp)
+                                        void *qapi, Error **errp)
 {
     GHashTable *h;
     StackObject *tos = &qiv->stack[qiv->nb_stack];
@@ -108,6 +109,7 @@ static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
     }
 
     tos->obj = obj;
+    tos->qapi = qapi;
     assert(!tos->h);
     assert(!tos->entry);
 
@@ -145,12 +147,13 @@ static void qmp_input_check_struct(Visitor *v, Error **errp)
     }
 }
 
-static void qmp_input_pop(Visitor *v)
+static void qmp_input_pop(Visitor *v, void **obj)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
 
     assert(qiv->nb_stack > 0);
+    assert(tos->qapi == obj);
 
     if (qiv->strict) {
         GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
@@ -179,7 +182,7 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
         return;
     }
 
-    qmp_input_push(qiv, qobj, &err);
+    qmp_input_push(qiv, qobj, obj, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -207,7 +210,7 @@ static void qmp_input_start_list(Visitor *v, const char *name,
         return;
     }
 
-    entry = qmp_input_push(qiv, qobj, errp);
+    entry = qmp_input_push(qiv, qobj, list, errp);
     if (list) {
         if (entry) {
             *list = g_malloc0(size);
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4d3cf78..dca8935 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -22,6 +22,7 @@
 typedef struct QStackEntry
 {
     QObject *value;
+    void *qapi; /* sanity check that caller uses same pointer */
     QTAILQ_ENTRY(QStackEntry) node;
 } QStackEntry;
 
@@ -36,7 +37,8 @@ struct QmpOutputVisitor
 
 #define qmp_output_add(qov, name, value) \
     qmp_output_add_obj(qov, name, QOBJECT(value))
-#define qmp_output_push(qov, value) qmp_output_push_obj(qov, QOBJECT(value))
+#define qmp_output_push(qov, value, qapi) \
+    qmp_output_push_obj(qov, QOBJECT(value), qapi)
 
 static QmpOutputVisitor *to_qov(Visitor *v)
 {
@@ -44,23 +46,26 @@ static QmpOutputVisitor *to_qov(Visitor *v)
 }
 
 /* Push @value onto the stack of current QObjects being built */
-static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
+static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value,
+                                void *qapi)
 {
     QStackEntry *e = g_malloc0(sizeof(*e));
 
     assert(qov->root);
     assert(value);
     e->value = value;
+    e->qapi = qapi;
     QTAILQ_INSERT_HEAD(&qov->stack, e, node);
 }
 
 /* Pop a value off the stack of QObjects being built, and return it. */
-static QObject *qmp_output_pop(QmpOutputVisitor *qov)
+static QObject *qmp_output_pop(QmpOutputVisitor *qov, void *qapi)
 {
     QStackEntry *e = QTAILQ_FIRST(&qov->stack);
     QObject *value;
 
     assert(e);
+    assert(e->qapi == qapi);
     QTAILQ_REMOVE(&qov->stack, e, node);
     value = e->value;
     assert(value);
@@ -104,13 +109,13 @@ static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
     QDict *dict = qdict_new();
 
     qmp_output_add(qov, name, dict);
-    qmp_output_push(qov, dict);
+    qmp_output_push(qov, dict, obj);
 }
 
-static void qmp_output_end_struct(Visitor *v)
+static void qmp_output_end_struct(Visitor *v, void **obj)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    QObject *value = qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov, obj);
     assert(qobject_type(value) == QTYPE_QDICT);
 }
 
@@ -122,7 +127,7 @@ static void qmp_output_start_list(Visitor *v, const char *name,
     QList *list = qlist_new();
 
     qmp_output_add(qov, name, list);
-    qmp_output_push(qov, list);
+    qmp_output_push(qov, list, listp);
 }
 
 static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
@@ -131,10 +136,10 @@ static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
     return tail->next;
 }
 
-static void qmp_output_end_list(Visitor *v)
+static void qmp_output_end_list(Visitor *v, void **obj)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    QObject *value = qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov, obj);
     assert(qobject_type(value) == QTYPE_QLIST);
 }
 
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 0690abb..d7f3c2b 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -30,6 +30,7 @@ struct StringInputVisitor
     int64_t cur;
 
     const char *string;
+    void *list; /* Only needed for sanity checking the caller */
 };
 
 static StringInputVisitor *to_siv(Visitor *v)
@@ -120,6 +121,7 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
 
     /* We don't support visits without a list */
     assert(list);
+    siv->list = list;
 
     if (parse_str(siv, name, errp) < 0) {
         *list = NULL;
@@ -168,8 +170,11 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
     return tail->next;
 }
 
-static void end_list(Visitor *v)
+static void end_list(Visitor *v, void **obj)
 {
+    StringInputVisitor *siv = to_siv(v);
+
+    assert(siv->list == obj);
 }
 
 static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index c6f01f9..bdc0fd0 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -64,6 +64,7 @@ struct StringOutputVisitor
         uint64_t u;
     } range_start, range_end;
     GList *ranges;
+    void *list; /* Only needed for sanity checking the caller */
 };
 
 static StringOutputVisitor *to_sov(Visitor *v)
@@ -274,6 +275,7 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
     assert(sov->list_mode == LM_NONE);
     /* We don't support visits without a list */
     assert(list);
+    sov->list = list;
     /* List handling is only needed if there are at least two elements */
     if (*list && (*list)->next) {
         sov->list_mode = LM_STARTED;
@@ -291,10 +293,11 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
     return ret;
 }
 
-static void end_list(Visitor *v)
+static void end_list(Visitor *v, void **obj)
 {
     StringOutputVisitor *sov = to_sov(v);
 
+    assert(sov->list == obj);
     assert(sov->list_mode == LM_STARTED ||
            sov->list_mode == LM_END ||
            sov->list_mode == LM_NONE ||
diff --git a/qom/object.c b/qom/object.c
index 9743ea4..3d6a955 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2044,7 +2044,7 @@ static void property_get_tm(Object *obj, Visitor *v, const char *name,
     }
     visit_check_struct(v, &err);
 out_end:
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
 out:
     error_propagate(errp, err);
 
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 51e62e2..926ec68 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -71,7 +71,7 @@ Object *user_creatable_add(const QDict *qdict,
     obj = user_creatable_add_type(type, id, pdict, v, &local_err);
 
 out_visit:
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
 
 out:
     QDECREF(pdict);
@@ -127,7 +127,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
     if (!local_err) {
         visit_check_struct(v, &local_err);
     }
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
     if (local_err) {
         goto out;
     }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8c6acb3..971dc4e 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -129,7 +129,7 @@ def gen_marshal(name, arg_type, ret_type):
     if (!err) {
         visit_check_struct(v, &err);
     }
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
     if (err) {
         goto out;
     }
@@ -160,7 +160,7 @@ out:
     v = qapi_dealloc_get_visitor(qdv);
     visit_start_struct(v, NULL, NULL, 0, NULL);
     visit_type_%(c_name)s_members(v, &arg, NULL);
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
     qapi_dealloc_visitor_cleanup(qdv);
 ''',
                      c_name=arg_type.c_name())
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 21fb167..084c40a 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -101,7 +101,7 @@ def gen_event_send(name, arg_type):
     if (!err) {
         visit_check_struct(v, &err);
     }
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
     if (err) {
         goto out;
     }
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index ffb635c..0b9e298 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -129,7 +129,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
         }
     }
 
-    visit_end_list(v);
+    visit_end_list(v, (void **)obj);
     if (err && visit_is_input(v)) {
         qapi_free_%(c_name)s(*obj);
         *obj = NULL;
@@ -194,7 +194,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
         if (!err) {
             visit_check_struct(v, &err);
         }
-        visit_end_struct(v);
+        visit_end_struct(v, NULL);
 ''',
                          c_type=var.type.c_name(),
                          c_name=c_name(var.name))
@@ -216,7 +216,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
                    "%(name)s");
     }
 out_obj:
-    visit_end_alternate(v);
+    visit_end_alternate(v, (void **)obj);
     if (err && visit_is_input(v)) {
         qapi_free_%(c_name)s(*obj);
         *obj = NULL;
@@ -250,7 +250,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
     }
     visit_check_struct(v, &err);
 out_obj:
-    visit_end_struct(v);
+    visit_end_struct(v, (void **)obj);
     if (err && visit_is_input(v)) {
         qapi_free_%(c_name)s(*obj);
         *obj = NULL;
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 17cda5a..b236183 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -304,7 +304,7 @@ static void test_visitor_in_null(TestInputVisitorData *data,
     visit_type_null(v, "b", &err);
     error_free_or_abort(&err);
     visit_check_struct(v, &error_abort);
-    visit_end_struct(v);
+    visit_end_struct(v, NULL);
 }
 
 static void test_visitor_in_union_flat(TestInputVisitorData *data,
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index e030c67..75c8e1b 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -499,7 +499,7 @@ static void test_visitor_out_null(TestOutputVisitorData *data,
     visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
     visit_type_null(data->ov, "a", &error_abort);
     visit_check_struct(data->ov, &error_abort);
-    visit_end_struct(data->ov);
+    visit_end_struct(data->ov, NULL);
     arg = qmp_output_get_qobject(data->qov);
     g_assert(qobject_type(arg) == QTYPE_QDICT);
     qdict = qobject_to_qdict(arg);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 04/15] qapi: Add new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (2 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 03/15] qapi: Add parameter to visit_end_* Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 05/15] opts-visitor: Favor " Markus Armbruster
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Making each visitor provide its own (awkwardly-named) FOO_cleanup()
is unusual, when we can instead have a polymorphic visit_free()
interface.  Over the next few patches, we can use the polymorphic
functions to eliminate the need for a FOO_get_visitor() function
for accessing specific visitor functionality, once everything can
be accessed directly through the Visitor* interfaces.

The dealloc visitor is the first one converted to completely use
the new entry point, since qapi_dealloc_visitor_cleanup() was the
only reason that qapi_dealloc_get_visitor() existed, and only
generated and testsuite code was even using it.  With the new
visit_free() entry point in place, we no longer need to expose
the QapiDeallocVisitor subtype through qapi_dealloc_visitor_new(),
and can get by with less generated code, with diffs that look like:

| void qapi_free_ACPIOSTInfo(ACPIOSTInfo *obj)
| {
|-    QapiDeallocVisitor *qdv;
|     Visitor *v;
|
|     if (!obj) {
|         return;
|     }
|
|-    qdv = qapi_dealloc_visitor_new();
|-    v = qapi_dealloc_get_visitor(qdv);
|+    v = qapi_dealloc_visitor_new();
|     visit_type_ACPIOSTInfo(v, NULL, &obj, NULL);
|-    qapi_dealloc_visitor_cleanup(qdv);
|+    visit_free(v);
|}

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-5-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt             | 28 ++++++++++------------------
 include/qapi/dealloc-visitor.h     |  5 +----
 include/qapi/visitor-impl.h        |  3 +++
 include/qapi/visitor.h             | 37 ++++++++++++++++++++++++++++++++++---
 qapi/opts-visitor.c                | 10 ++++++++++
 qapi/qapi-dealloc-visitor.c        | 14 +++++---------
 qapi/qapi-visit-core.c             |  7 +++++++
 qapi/qmp-input-visitor.c           |  8 ++++++++
 qapi/qmp-output-visitor.c          |  8 ++++++++
 qapi/string-input-visitor.c        |  8 ++++++++
 qapi/string-output-visitor.c       |  8 ++++++++
 scripts/qapi-commands.py           | 16 ++++++----------
 scripts/qapi-event.py              |  2 +-
 scripts/qapi-types.py              |  6 ++----
 tests/test-visitor-serialization.c |  6 +++---
 15 files changed, 114 insertions(+), 52 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0ae239a..db9d5b6 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -802,32 +802,28 @@ Example:
 
     void qapi_free_UserDefOne(UserDefOne *obj)
     {
-        QapiDeallocVisitor *qdv;
         Visitor *v;
 
         if (!obj) {
             return;
         }
 
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
+        v = qapi_dealloc_visitor_new();
         visit_type_UserDefOne(v, NULL, &obj, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
+        visit_free(v);
     }
 
     void qapi_free_UserDefOneList(UserDefOneList *obj)
     {
-        QapiDeallocVisitor *qdv;
         Visitor *v;
 
         if (!obj) {
             return;
         }
 
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
+        v = qapi_dealloc_visitor_new();
         visit_type_UserDefOneList(v, NULL, &obj, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
+        visit_free(v);
     }
 
 === scripts/qapi-visit.py ===
@@ -985,7 +981,6 @@ Example:
     {
         Error *err = NULL;
         QmpOutputVisitor *qov = qmp_output_visitor_new();
-        QapiDeallocVisitor *qdv;
         Visitor *v;
 
         v = qmp_output_get_visitor(qov);
@@ -997,11 +992,10 @@ Example:
 
     out:
         error_propagate(errp, err);
-        qmp_output_visitor_cleanup(qov);
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
+        visit_free(v);
+        v = qapi_dealloc_visitor_new();
         visit_type_UserDefOne(v, "unused", &ret_in, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
+        visit_free(v);
     }
 
     static void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp)
@@ -1009,7 +1003,6 @@ Example:
         Error *err = NULL;
         UserDefOne *retval;
         QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
-        QapiDeallocVisitor *qdv;
         Visitor *v;
         UserDefOneList *arg1 = NULL;
 
@@ -1036,13 +1029,12 @@ Example:
 
     out:
         error_propagate(errp, err);
-        qmp_input_visitor_cleanup(qiv);
-        qdv = qapi_dealloc_visitor_new();
-        v = qapi_dealloc_get_visitor(qdv);
+        visit_free(v);
+        v = qapi_dealloc_visitor_new();
         visit_start_struct(v, NULL, NULL, 0, NULL);
         visit_type_UserDefOneList(v, "arg1", &arg1, NULL);
         visit_end_struct(v, NULL);
-        qapi_dealloc_visitor_cleanup(qdv);
+        visit_free(v);
     }
 
     static void qmp_init_marshal(void)
diff --git a/include/qapi/dealloc-visitor.h b/include/qapi/dealloc-visitor.h
index 45b06b2..b3e5c85 100644
--- a/include/qapi/dealloc-visitor.h
+++ b/include/qapi/dealloc-visitor.h
@@ -23,9 +23,6 @@ typedef struct QapiDeallocVisitor QapiDeallocVisitor;
  * qapi_free_FOO() functions, and is the only visitor designed to work
  * correctly in the face of a partially-constructed QAPI tree.
  */
-QapiDeallocVisitor *qapi_dealloc_visitor_new(void);
-void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *d);
-
-Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v);
+Visitor *qapi_dealloc_visitor_new(void);
 
 #endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index a495bf0..525b068 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -104,6 +104,9 @@ struct Visitor
 
     /* Must be set */
     VisitorType type;
+
+    /* Must be set */
+    void (*free)(Visitor *v);
 };
 
 #endif
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 25d0cc2..2ded852 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -37,6 +37,24 @@
  * implemented by each visitor, and docs/qapi-code-gen.txt for more
  * about the QAPI code generator.
  *
+ * 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 *);
+ *
+ * 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.
+ * 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.
+ *
  * All QAPI types have a corresponding function with a signature
  * roughly compatible with this:
  *
@@ -222,6 +240,19 @@ typedef struct GenericAlternate {
     char padding[];
 } GenericAlternate;
 
+/*** Visitor cleanup ***/
+
+/*
+ * Free @v and any resources it has tied up.
+ *
+ * May be called whether or not the visit has been successfully
+ * completed, but should not be called until a top-level
+ * visit_type_FOO() or visit_start_ITEM() has been performed on the
+ * visitor.  Safe if @v is NULL.
+ */
+void visit_free(Visitor *v);
+
+
 /*** Visiting structures ***/
 
 /*
@@ -272,7 +303,7 @@ void visit_check_struct(Visitor *v, Error **errp);
  * Must be called after any successful use of visit_start_struct(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  */
 void visit_end_struct(Visitor *v, void **obj);
 
@@ -332,7 +363,7 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
  * Must be called after any successful use of visit_start_list(), even
  * if intermediate processing was skipped due to errors, to allow the
  * backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  */
 void visit_end_list(Visitor *v, void **list);
 
@@ -368,7 +399,7 @@ void visit_start_alternate(Visitor *v, const char *name,
  * Must be called after any successful use of visit_start_alternate(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  *
  */
 void visit_end_alternate(Visitor *v, void **obj);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index dcfbf92..72c95ac 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -513,6 +513,15 @@ opts_optional(Visitor *v, const char *name, bool *present)
 }
 
 
+static void
+opts_free(Visitor *v)
+{
+    OptsVisitor *ov = to_ov(v);
+
+    opts_visitor_cleanup(ov);
+}
+
+
 OptsVisitor *
 opts_visitor_new(const QemuOpts *opts)
 {
@@ -540,6 +549,7 @@ opts_visitor_new(const QemuOpts *opts)
      * skip some mandatory methods... */
 
     ov->visitor.optional = &opts_optional;
+    ov->visitor.free = opts_free;
 
     ov->opts_root = opts;
 
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 9391dea..e39457b 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -107,17 +107,12 @@ static void qapi_dealloc_type_null(Visitor *v, const char *name, Error **errp)
 {
 }
 
-Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
+static void qapi_dealloc_free(Visitor *v)
 {
-    return &v->visitor;
+    g_free(container_of(v, QapiDeallocVisitor, visitor));
 }
 
-void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *v)
-{
-    g_free(v);
-}
-
-QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
+Visitor *qapi_dealloc_visitor_new(void)
 {
     QapiDeallocVisitor *v;
 
@@ -138,6 +133,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.type_number = qapi_dealloc_type_number;
     v->visitor.type_any = qapi_dealloc_type_anything;
     v->visitor.type_null = qapi_dealloc_type_null;
+    v->visitor.free = qapi_dealloc_free;
 
-    return v;
+    return &v->visitor;
 }
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index dba11c6..5f68c25 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -20,6 +20,13 @@
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"
 
+void visit_free(Visitor *v)
+{
+    if (v) {
+        v->free(v);
+    }
+}
+
 void visit_start_struct(Visitor *v, const char *name, void **obj,
                         size_t size, Error **errp)
 {
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index e16b4b0..41b7f87 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -378,6 +378,13 @@ Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
     return &v->visitor;
 }
 
+static void qmp_input_free(Visitor *v)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+
+    qmp_input_visitor_cleanup(qiv);
+}
+
 void qmp_input_visitor_cleanup(QmpInputVisitor *v)
 {
     qobject_decref(v->root);
@@ -406,6 +413,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
     v->visitor.type_any = qmp_input_type_any;
     v->visitor.type_null = qmp_input_type_null;
     v->visitor.optional = qmp_input_optional;
+    v->visitor.free = qmp_input_free;
     v->strict = strict;
 
     v->root = obj;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index dca8935..5f0035c 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -214,6 +214,13 @@ Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)
     return &v->visitor;
 }
 
+static void qmp_output_free(Visitor *v)
+{
+    QmpOutputVisitor *qov = to_qov(v);
+
+    qmp_output_visitor_cleanup(qov);
+}
+
 void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
 {
     QStackEntry *e, *tmp;
@@ -246,6 +253,7 @@ 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.free = qmp_output_free;
 
     QTAILQ_INIT(&v->stack);
 
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index d7f3c2b..6fb1229 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -331,6 +331,13 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
     return &v->visitor;
 }
 
+static void string_input_free(Visitor *v)
+{
+    StringInputVisitor *siv = to_siv(v);
+
+    string_input_visitor_cleanup(siv);
+}
+
 void string_input_visitor_cleanup(StringInputVisitor *v)
 {
     g_list_foreach(v->ranges, free_range, NULL);
@@ -355,6 +362,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.next_list = next_list;
     v->visitor.end_list = end_list;
     v->visitor.optional = parse_optional;
+    v->visitor.free = string_input_free;
 
     v->string = str;
     return v;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index bdc0fd0..ff9ddf0 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -322,6 +322,13 @@ static void free_range(void *range, void *dummy)
     g_free(range);
 }
 
+static void string_output_free(Visitor *v)
+{
+    StringOutputVisitor *sov = to_sov(v);
+
+    string_output_visitor_cleanup(sov);
+}
+
 void string_output_visitor_cleanup(StringOutputVisitor *sov)
 {
     if (sov->string) {
@@ -351,6 +358,7 @@ 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.free = string_output_free;
 
     return v;
 }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 971dc4e..77ecd47 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -62,7 +62,6 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
 {
     Error *err = NULL;
     QmpOutputVisitor *qov = qmp_output_visitor_new();
-    QapiDeallocVisitor *qdv;
     Visitor *v;
 
     v = qmp_output_get_visitor(qov);
@@ -74,11 +73,10 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
 
 out:
     error_propagate(errp, err);
-    qmp_output_visitor_cleanup(qov);
-    qdv = qapi_dealloc_visitor_new();
-    v = qapi_dealloc_get_visitor(qdv);
+    visit_free(v);
+    v = qapi_dealloc_visitor_new();
     visit_type_%(c_name)s(v, "unused", &ret_in, NULL);
-    qapi_dealloc_visitor_cleanup(qdv);
+    visit_free(v);
 }
 ''',
                  c_type=ret_type.c_type(), c_name=ret_type.c_name())
@@ -116,7 +114,6 @@ def gen_marshal(name, arg_type, ret_type):
     if arg_type and arg_type.members:
         ret += mcgen('''
     QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
-    QapiDeallocVisitor *qdv;
     Visitor *v;
     %(c_name)s arg = {0};
 
@@ -155,13 +152,12 @@ out:
 ''')
     if arg_type and arg_type.members:
         ret += mcgen('''
-    qmp_input_visitor_cleanup(qiv);
-    qdv = qapi_dealloc_visitor_new();
-    v = qapi_dealloc_get_visitor(qdv);
+    visit_free(v);
+    v = qapi_dealloc_visitor_new();
     visit_start_struct(v, NULL, NULL, 0, NULL);
     visit_type_%(c_name)s_members(v, &arg, NULL);
     visit_end_struct(v, NULL);
-    qapi_dealloc_visitor_cleanup(qdv);
+    visit_free(v);
 ''',
                      c_name=arg_type.c_name())
 
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 084c40a..909e8d6 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -119,7 +119,7 @@ def gen_event_send(name, arg_type):
     if arg_type and arg_type.members:
         ret += mcgen('''
 out:
-    qmp_output_visitor_cleanup(qov);
+    visit_free(v);
 ''')
     ret += mcgen('''
     error_propagate(errp, err);
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 437cf6c..5ace2cf 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -150,17 +150,15 @@ def gen_type_cleanup(name):
 
 void qapi_free_%(c_name)s(%(c_name)s *obj)
 {
-    QapiDeallocVisitor *qdv;
     Visitor *v;
 
     if (!obj) {
         return;
     }
 
-    qdv = qapi_dealloc_visitor_new();
-    v = qapi_dealloc_get_visitor(qdv);
+    v = qapi_dealloc_visitor_new();
     visit_type_%(c_name)s(v, NULL, &obj, NULL);
-    qapi_dealloc_visitor_cleanup(qdv);
+    visit_free(v);
 }
 ''',
                 c_name=c_name(name))
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index db781c0..bcbfd2a 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -89,11 +89,11 @@ typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp);
 
 static void dealloc_helper(void *native_in, VisitorFunc visit, Error **errp)
 {
-    QapiDeallocVisitor *qdv = qapi_dealloc_visitor_new();
+    Visitor *v = qapi_dealloc_visitor_new();
 
-    visit(qapi_dealloc_get_visitor(qdv), &native_in, errp);
+    visit(v, &native_in, errp);
 
-    qapi_dealloc_visitor_cleanup(qdv);
+    visit_free(v);
 }
 
 static void visit_primitive_type(Visitor *v, void **native, Error **errp)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 05/15] opts-visitor: Favor new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (3 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 04/15] qapi: Add new visit_free() function Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 06/15] string-input-visitor: " Markus Armbruster
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Now that we have a polymorphic visit_free(), we no longer need
opts_visitor_cleanup(); which in turn means we no longer need
to return a subtype from opts_visitor_new() nor a public upcast
function.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-6-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/crypto.c              | 30 ++++++++++++++----------------
 hmp.c                       |  8 ++++----
 hw/acpi/core.c              |  8 ++++----
 include/qapi/opts-visitor.h |  4 +---
 net/net.c                   |  5 ++---
 numa.c                      |  6 +++---
 qapi/opts-visitor.c         | 26 ++++++--------------------
 qom/object_interfaces.c     |  8 ++++----
 tests/test-opts-visitor.c   |  9 ++++-----
 9 files changed, 42 insertions(+), 62 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 5f0ab4d..9bb55d3 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -193,17 +193,16 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
                             QemuOpts *opts,
                             Error **errp)
 {
-    OptsVisitor *ov;
+    Visitor *v;
     QCryptoBlockOpenOptions *ret = NULL;
     Error *local_err = NULL;
 
     ret = g_new0(QCryptoBlockOpenOptions, 1);
     ret->format = format;
 
-    ov = opts_visitor_new(opts);
+    v = opts_visitor_new(opts);
 
-    visit_start_struct(opts_get_visitor(ov),
-                       NULL, NULL, 0, &local_err);
+    visit_start_struct(v, NULL, NULL, 0, &local_err);
     if (local_err) {
         goto out;
     }
@@ -211,7 +210,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
     switch (format) {
     case Q_CRYPTO_BLOCK_FORMAT_LUKS:
         visit_type_QCryptoBlockOptionsLUKS_members(
-            opts_get_visitor(ov), &ret->u.luks, &local_err);
+            v, &ret->u.luks, &local_err);
         break;
 
     default:
@@ -219,10 +218,10 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
         break;
     }
     if (!local_err) {
-        visit_check_struct(opts_get_visitor(ov), &local_err);
+        visit_check_struct(v, &local_err);
     }
 
-    visit_end_struct(opts_get_visitor(ov), NULL);
+    visit_end_struct(v, NULL);
 
  out:
     if (local_err) {
@@ -230,7 +229,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
         qapi_free_QCryptoBlockOpenOptions(ret);
         ret = NULL;
     }
-    opts_visitor_cleanup(ov);
+    visit_free(v);
     return ret;
 }
 
@@ -240,17 +239,16 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
                               QemuOpts *opts,
                               Error **errp)
 {
-    OptsVisitor *ov;
+    Visitor *v;
     QCryptoBlockCreateOptions *ret = NULL;
     Error *local_err = NULL;
 
     ret = g_new0(QCryptoBlockCreateOptions, 1);
     ret->format = format;
 
-    ov = opts_visitor_new(opts);
+    v = opts_visitor_new(opts);
 
-    visit_start_struct(opts_get_visitor(ov),
-                       NULL, NULL, 0, &local_err);
+    visit_start_struct(v, NULL, NULL, 0, &local_err);
     if (local_err) {
         goto out;
     }
@@ -258,7 +256,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
     switch (format) {
     case Q_CRYPTO_BLOCK_FORMAT_LUKS:
         visit_type_QCryptoBlockCreateOptionsLUKS_members(
-            opts_get_visitor(ov), &ret->u.luks, &local_err);
+            v, &ret->u.luks, &local_err);
         break;
 
     default:
@@ -266,10 +264,10 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
         break;
     }
     if (!local_err) {
-        visit_check_struct(opts_get_visitor(ov), &local_err);
+        visit_check_struct(v, &local_err);
     }
 
-    visit_end_struct(opts_get_visitor(ov), NULL);
+    visit_end_struct(v, NULL);
 
  out:
     if (local_err) {
@@ -277,7 +275,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
         qapi_free_QCryptoBlockCreateOptions(ret);
         ret = NULL;
     }
-    opts_visitor_cleanup(ov);
+    visit_free(v);
     return ret;
 }
 
diff --git a/hmp.c b/hmp.c
index 925601a..fd5daa7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1722,7 +1722,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     QemuOpts *opts;
-    OptsVisitor *ov;
+    Visitor *v;
     Object *obj = NULL;
 
     opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
@@ -1731,9 +1731,9 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    ov = opts_visitor_new(opts);
-    obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
-    opts_visitor_cleanup(ov);
+    v = opts_visitor_new(opts);
+    obj = user_creatable_add(qdict, v, &err);
+    visit_free(v);
     qemu_opts_del(opts);
 
     if (err) {
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index d24b9a9..e890a5d 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -239,11 +239,11 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
     char unsigned *blob = NULL;
 
     {
-        OptsVisitor *ov;
+        Visitor *v;
 
-        ov = opts_visitor_new(opts);
-        visit_type_AcpiTableOptions(opts_get_visitor(ov), NULL, &hdrs, &err);
-        opts_visitor_cleanup(ov);
+        v = opts_visitor_new(opts);
+        visit_type_AcpiTableOptions(v, NULL, &hdrs, &err);
+        visit_free(v);
     }
 
     if (err) {
diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index ae1bf7c..6462c96 100644
--- a/include/qapi/opts-visitor.h
+++ b/include/qapi/opts-visitor.h
@@ -35,8 +35,6 @@ typedef struct OptsVisitor OptsVisitor;
  * QTypes.  It also requires a non-null list argument to
  * visit_start_list().
  */
-OptsVisitor *opts_visitor_new(const QemuOpts *opts);
-void opts_visitor_cleanup(OptsVisitor *nv);
-Visitor *opts_get_visitor(OptsVisitor *nv);
+Visitor *opts_visitor_new(const QemuOpts *opts);
 
 #endif
diff --git a/net/net.c b/net/net.c
index 75bb177..d25f802 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1024,8 +1024,7 @@ int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
     void *object = NULL;
     Error *err = NULL;
     int ret = -1;
-    OptsVisitor *ov = opts_visitor_new(opts);
-    Visitor *v = opts_get_visitor(ov);
+    Visitor *v = opts_visitor_new(opts);
 
     {
         /* Parse convenience option format ip6-net=fec0::0[/64] */
@@ -1075,7 +1074,7 @@ int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
     }
 
     error_propagate(errp, err);
-    opts_visitor_cleanup(ov);
+    visit_free(v);
     return ret;
 }
 
diff --git a/numa.c b/numa.c
index 572712c..cbae430 100644
--- a/numa.c
+++ b/numa.c
@@ -217,9 +217,9 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
     Error *err = NULL;
 
     {
-        OptsVisitor *ov = opts_visitor_new(opts);
-        visit_type_NumaOptions(opts_get_visitor(ov), NULL, &object, &err);
-        opts_visitor_cleanup(ov);
+        Visitor *v = opts_visitor_new(opts);
+        visit_type_NumaOptions(v, NULL, &object, &err);
+        visit_free(v);
     }
 
     if (err) {
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 72c95ac..1048bbc 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -518,11 +518,15 @@ opts_free(Visitor *v)
 {
     OptsVisitor *ov = to_ov(v);
 
-    opts_visitor_cleanup(ov);
+    if (ov->unprocessed_opts != NULL) {
+        g_hash_table_destroy(ov->unprocessed_opts);
+    }
+    g_free(ov->fake_id_opt);
+    g_free(ov);
 }
 
 
-OptsVisitor *
+Visitor *
 opts_visitor_new(const QemuOpts *opts)
 {
     OptsVisitor *ov;
@@ -553,23 +557,5 @@ opts_visitor_new(const QemuOpts *opts)
 
     ov->opts_root = opts;
 
-    return ov;
-}
-
-
-void
-opts_visitor_cleanup(OptsVisitor *ov)
-{
-    if (ov->unprocessed_opts != NULL) {
-        g_hash_table_destroy(ov->unprocessed_opts);
-    }
-    g_free(ov->fake_id_opt);
-    g_free(ov);
-}
-
-
-Visitor *
-opts_get_visitor(OptsVisitor *ov)
-{
     return &ov->visitor;
 }
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 926ec68..bf59846 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -156,15 +156,15 @@ out:
 
 Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
 {
-    OptsVisitor *ov;
+    Visitor *v;
     QDict *pdict;
     Object *obj = NULL;
 
-    ov = opts_visitor_new(opts);
+    v = opts_visitor_new(opts);
     pdict = qemu_opts_to_qdict(opts, NULL);
 
-    obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
-    opts_visitor_cleanup(ov);
+    obj = user_creatable_add(pdict, v, errp);
+    visit_free(v);
     QDECREF(pdict);
     return obj;
 }
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index d75b114..0a9e75f 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -37,16 +37,15 @@ setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
 {
     const char *opts_string = test_data;
     QemuOpts *opts;
-    OptsVisitor *ov;
+    Visitor *v;
 
     opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, false,
                            NULL);
     g_assert(opts != NULL);
 
-    ov = opts_visitor_new(opts);
-    visit_type_UserDefOptions(opts_get_visitor(ov), NULL, &f->userdef,
-                              &f->err);
-    opts_visitor_cleanup(ov);
+    v = opts_visitor_new(opts);
+    visit_type_UserDefOptions(v, NULL, &f->userdef, &f->err);
+    visit_free(v);
     qemu_opts_del(opts);
 }
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 06/15] string-input-visitor: Favor new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (4 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 05/15] opts-visitor: Favor " Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 07/15] qmp-input-visitor: " Markus Armbruster
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Now that we have a polymorphic visit_free(), we no longer need
string_input_visitor_cleanup(); which in turn means we no longer
need to return a subtype from string_input_visitor_new() nor a
public upcast function.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-7-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/string-input-visitor.h |  5 +----
 qapi/string-input-visitor.c         | 20 +++++---------------
 qom/object.c                        | 25 +++++++++++--------------
 tests/test-string-input-visitor.c   | 22 +++++++---------------
 tests/test-visitor-serialization.c  |  6 +++---
 5 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/include/qapi/string-input-visitor.h b/include/qapi/string-input-visitor.h
index 7b76c2b..3355134 100644
--- a/include/qapi/string-input-visitor.h
+++ b/include/qapi/string-input-visitor.h
@@ -22,9 +22,6 @@ typedef struct StringInputVisitor StringInputVisitor;
  * QAPI structs, alternates, null, or arbitrary QTypes.  It also
  * requires a non-null list argument to visit_start_list().
  */
-StringInputVisitor *string_input_visitor_new(const char *str);
-void string_input_visitor_cleanup(StringInputVisitor *v);
-
-Visitor *string_input_get_visitor(StringInputVisitor *v);
+Visitor *string_input_visitor_new(const char *str);
 
 #endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 6fb1229..8dfa561 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -326,26 +326,16 @@ static void parse_optional(Visitor *v, const char *name, bool *present)
     *present = true;
 }
 
-Visitor *string_input_get_visitor(StringInputVisitor *v)
-{
-    return &v->visitor;
-}
-
 static void string_input_free(Visitor *v)
 {
     StringInputVisitor *siv = to_siv(v);
 
-    string_input_visitor_cleanup(siv);
+    g_list_foreach(siv->ranges, free_range, NULL);
+    g_list_free(siv->ranges);
+    g_free(siv);
 }
 
-void string_input_visitor_cleanup(StringInputVisitor *v)
-{
-    g_list_foreach(v->ranges, free_range, NULL);
-    g_list_free(v->ranges);
-    g_free(v);
-}
-
-StringInputVisitor *string_input_visitor_new(const char *str)
+Visitor *string_input_visitor_new(const char *str)
 {
     StringInputVisitor *v;
 
@@ -365,5 +355,5 @@ StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.free = string_input_free;
 
     v->string = str;
-    return v;
+    return &v->visitor;
 }
diff --git a/qom/object.c b/qom/object.c
index 3d6a955..02c0a3a 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1222,7 +1222,7 @@ int object_property_get_enum(Object *obj, const char *name,
 {
     Error *err = NULL;
     StringOutputVisitor *sov;
-    StringInputVisitor *siv;
+    Visitor *v;
     char *str;
     int ret;
     ObjectProperty *prop = object_property_find(obj, name, errp);
@@ -1249,13 +1249,12 @@ int object_property_get_enum(Object *obj, const char *name,
         return 0;
     }
     str = string_output_get_string(sov);
-    siv = string_input_visitor_new(str);
     string_output_visitor_cleanup(sov);
-    visit_type_enum(string_input_get_visitor(siv), name, &ret,
-                    enumprop->strings, errp);
+    v = string_input_visitor_new(str);
+    visit_type_enum(v, name, &ret, enumprop->strings, errp);
 
     g_free(str);
-    string_input_visitor_cleanup(siv);
+    visit_free(v);
 
     return ret;
 }
@@ -1265,7 +1264,7 @@ void object_property_get_uint16List(Object *obj, const char *name,
 {
     Error *err = NULL;
     StringOutputVisitor *ov;
-    StringInputVisitor *iv;
+    Visitor *v;
     char *str;
 
     ov = string_output_visitor_new(false);
@@ -1276,11 +1275,11 @@ void object_property_get_uint16List(Object *obj, const char *name,
         goto out;
     }
     str = string_output_get_string(ov);
-    iv = string_input_visitor_new(str);
-    visit_type_uint16List(string_input_get_visitor(iv), NULL, list, errp);
+    v = string_input_visitor_new(str);
+    visit_type_uint16List(v, NULL, list, errp);
 
     g_free(str);
-    string_input_visitor_cleanup(iv);
+    visit_free(v);
 out:
     string_output_visitor_cleanup(ov);
 }
@@ -1288,11 +1287,9 @@ out:
 void object_property_parse(Object *obj, const char *string,
                            const char *name, Error **errp)
 {
-    StringInputVisitor *siv;
-    siv = string_input_visitor_new(string);
-    object_property_set(obj, string_input_get_visitor(siv), name, errp);
-
-    string_input_visitor_cleanup(siv);
+    Visitor *v = string_input_visitor_new(string);
+    object_property_set(obj, v, name, errp);
+    visit_free(v);
 }
 
 char *object_property_print(Object *obj, const char *name, bool human,
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 7fe7a9c..d837ebe 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -20,15 +20,15 @@
 #include "qapi/qmp/types.h"
 
 typedef struct TestInputVisitorData {
-    StringInputVisitor *siv;
+    Visitor *v;
 } TestInputVisitorData;
 
 static void visitor_input_teardown(TestInputVisitorData *data,
                                    const void *unused)
 {
-    if (data->siv) {
-        string_input_visitor_cleanup(data->siv);
-        data->siv = NULL;
+    if (data->v) {
+        visit_free(data->v);
+        data->v = NULL;
     }
 }
 
@@ -39,15 +39,9 @@ static
 Visitor *visitor_input_test_init(TestInputVisitorData *data,
                                  const char *string)
 {
-    Visitor *v;
-
-    data->siv = string_input_visitor_new(string);
-    g_assert(data->siv != NULL);
-
-    v = string_input_get_visitor(data->siv);
-    g_assert(v != NULL);
-
-    return v;
+    data->v = string_input_visitor_new(string);
+    g_assert(data->v);
+    return data->v;
 }
 
 static void test_visitor_in_int(TestInputVisitorData *data,
@@ -199,8 +193,6 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
 
         visitor_input_teardown(data, NULL);
     }
-
-    data->siv = NULL;
 }
 
 /* Try to crash the visitors */
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index bcbfd2a..c3c8634 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1056,7 +1056,7 @@ static void qmp_cleanup(void *datap)
 typedef struct StringSerializeData {
     char *string;
     StringOutputVisitor *sov;
-    StringInputVisitor *siv;
+    Visitor *siv;
 } StringSerializeData;
 
 static void string_serialize(void *native_in, void **datap,
@@ -1076,7 +1076,7 @@ static void string_deserialize(void **native_out, void *datap,
 
     d->string = string_output_get_string(d->sov);
     d->siv = string_input_visitor_new(d->string);
-    visit(string_input_get_visitor(d->siv), native_out, errp);
+    visit(d->siv, native_out, errp);
 }
 
 static void string_cleanup(void *datap)
@@ -1084,7 +1084,7 @@ static void string_cleanup(void *datap)
     StringSerializeData *d = datap;
 
     string_output_visitor_cleanup(d->sov);
-    string_input_visitor_cleanup(d->siv);
+    visit_free(d->siv);
     g_free(d->string);
     g_free(d);
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 07/15] qmp-input-visitor: Favor new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (5 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 06/15] string-input-visitor: " Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:13 ` [Qemu-devel] [PULL 08/15] string-output-visitor: " Markus Armbruster
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Now that we have a polymorphic visit_free(), we no longer need
qmp_input_visitor_cleanup(); which in turn means we no longer
need to return a subtype from qmp_input_visitor_new() nor a
public upcast function.

Generated code changes to qmp-marshal.c look like:

|@@ -52,11 +52,10 @@ void qmp_marshal_add_fd(QDict *args, QOb
| {
|     Error *err = NULL;
|     AddfdInfo *retval;
|-    QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
|     Visitor *v;
|     q_obj_add_fd_arg arg = {0};
|
|-    v = qmp_input_get_visitor(qiv);
|+    v = qmp_input_visitor_new(QOBJECT(args), true);
|     visit_start_struct(v, NULL, NULL, 0, &err);
|     if (err) {
|         goto out;

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-8-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt             |  3 +--
 include/qapi/qmp-input-visitor.h   |  6 +-----
 qapi/qmp-input-visitor.c           | 18 ++++--------------
 qmp.c                              |  9 ++++-----
 qom/qom-qobject.c                  |  9 ++++-----
 replay/replay-input.c              |  6 ++----
 scripts/qapi-commands.py           |  3 +--
 tests/check-qnull.c                |  8 ++++----
 tests/test-qmp-commands.c          |  8 ++++----
 tests/test-qmp-input-strict.c      | 12 +++---------
 tests/test-qmp-input-visitor.c     | 12 +++---------
 tests/test-visitor-serialization.c |  6 +++---
 util/qemu-sockets.c                |  6 ++----
 13 files changed, 36 insertions(+), 70 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index db9d5b6..7c30762 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -1002,11 +1002,10 @@ Example:
     {
         Error *err = NULL;
         UserDefOne *retval;
-        QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
         Visitor *v;
         UserDefOneList *arg1 = NULL;
 
-        v = qmp_input_get_visitor(qiv);
+        v = qmp_input_visitor_new(QOBJECT(args), true);
         visit_start_struct(v, NULL, NULL, 0, &err);
         if (err) {
             goto out;
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index b0624d8..f3ff5f3 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qmp-input-visitor.h
@@ -25,10 +25,6 @@ typedef struct QmpInputVisitor QmpInputVisitor;
  * Set @strict to reject a parse that doesn't consume all keys of a
  * dictionary; otherwise excess input is ignored.
  */
-QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict);
-
-void qmp_input_visitor_cleanup(QmpInputVisitor *v);
-
-Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
+Visitor *qmp_input_visitor_new(QObject *obj, bool strict);
 
 #endif
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 41b7f87..21edb39 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -373,25 +373,15 @@ static void qmp_input_optional(Visitor *v, const char *name, bool *present)
     *present = true;
 }
 
-Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
-{
-    return &v->visitor;
-}
-
 static void qmp_input_free(Visitor *v)
 {
     QmpInputVisitor *qiv = to_qiv(v);
 
-    qmp_input_visitor_cleanup(qiv);
+    qobject_decref(qiv->root);
+    g_free(qiv);
 }
 
-void qmp_input_visitor_cleanup(QmpInputVisitor *v)
-{
-    qobject_decref(v->root);
-    g_free(v);
-}
-
-QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
+Visitor *qmp_input_visitor_new(QObject *obj, bool strict)
 {
     QmpInputVisitor *v;
 
@@ -419,5 +409,5 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
     v->root = obj;
     qobject_incref(obj);
 
-    return v;
+    return &v->visitor;
 }
diff --git a/qmp.c b/qmp.c
index 7df6543..b6d531e 100644
--- a/qmp.c
+++ b/qmp.c
@@ -655,7 +655,7 @@ void qmp_object_add(const char *type, const char *id,
                     bool has_props, QObject *props, Error **errp)
 {
     const QDict *pdict = NULL;
-    QmpInputVisitor *qiv;
+    Visitor *v;
     Object *obj;
 
     if (props) {
@@ -666,10 +666,9 @@ void qmp_object_add(const char *type, const char *id,
         }
     }
 
-    qiv = qmp_input_visitor_new(props, true);
-    obj = user_creatable_add_type(type, id, pdict,
-                                  qmp_input_get_visitor(qiv), errp);
-    qmp_input_visitor_cleanup(qiv);
+    v = qmp_input_visitor_new(props, true);
+    obj = user_creatable_add_type(type, id, pdict, v, errp);
+    visit_free(v);
     if (obj) {
         object_unref(obj);
     }
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index b66088d..c3c9188 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -21,12 +21,11 @@
 void object_property_set_qobject(Object *obj, QObject *value,
                                  const char *name, Error **errp)
 {
-    QmpInputVisitor *qiv;
+    Visitor *v;
     /* TODO: Should we reject, rather than ignore, excess input? */
-    qiv = qmp_input_visitor_new(value, false);
-    object_property_set(obj, qmp_input_get_visitor(qiv), name, errp);
-
-    qmp_input_visitor_cleanup(qiv);
+    v = qmp_input_visitor_new(value, false);
+    object_property_set(obj, v, name, errp);
+    visit_free(v);
 }
 
 QObject *object_property_get_qobject(Object *obj, const char *name,
diff --git a/replay/replay-input.c b/replay/replay-input.c
index 03e99d5..7b6fa93 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -23,7 +23,6 @@
 static InputEvent *qapi_clone_InputEvent(InputEvent *src)
 {
     QmpOutputVisitor *qov;
-    QmpInputVisitor *qiv;
     Visitor *ov, *iv;
     QObject *obj;
     InputEvent *dst = NULL;
@@ -37,10 +36,9 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src)
         return NULL;
     }
 
-    qiv = qmp_input_visitor_new(obj, true);
-    iv = qmp_input_get_visitor(qiv);
+    iv = qmp_input_visitor_new(obj, true);
     visit_type_InputEvent(iv, NULL, &dst, &error_abort);
-    qmp_input_visitor_cleanup(qiv);
+    visit_free(iv);
     qobject_decref(obj);
 
     return dst;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 77ecd47..49d4ce2 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -113,11 +113,10 @@ def gen_marshal(name, arg_type, ret_type):
 
     if arg_type and arg_type.members:
         ret += mcgen('''
-    QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
     Visitor *v;
     %(c_name)s arg = {0};
 
-    v = qmp_input_get_visitor(qiv);
+    v = qmp_input_visitor_new(QOBJECT(args), true);
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
         goto out;
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index 05d562d..d0412e4 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -38,7 +38,7 @@ static void qnull_visit_test(void)
 {
     QObject *obj;
     QmpOutputVisitor *qov;
-    QmpInputVisitor *qiv;
+    Visitor *v;
 
     /*
      * Most tests of interactions between QObject and visitors are in
@@ -48,10 +48,10 @@ static void qnull_visit_test(void)
 
     g_assert(qnull_.refcnt == 1);
     obj = qnull();
-    qiv = qmp_input_visitor_new(obj, true);
+    v = qmp_input_visitor_new(obj, true);
     qobject_decref(obj);
-    visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort);
-    qmp_input_visitor_cleanup(qiv);
+    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);
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index e8f619d..8ffeb04 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -216,14 +216,14 @@ static void test_dealloc_partial(void)
     /* create partial object */
     {
         QDict *ud2_dict;
-        QmpInputVisitor *qiv;
+        Visitor *v;
 
         ud2_dict = qdict_new();
         qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text)));
 
-        qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true);
-        visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err);
-        qmp_input_visitor_cleanup(qiv);
+        v = qmp_input_visitor_new(QOBJECT(ud2_dict), true);
+        visit_type_UserDefTwo(v, NULL, &ud2, &err);
+        visit_free(v);
         QDECREF(ud2_dict);
     }
 
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 79739e9..814550a 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -26,7 +26,7 @@
 
 typedef struct TestInputVisitorData {
     QObject *obj;
-    QmpInputVisitor *qiv;
+    Visitor *qiv;
 } TestInputVisitorData;
 
 static void validate_teardown(TestInputVisitorData *data,
@@ -36,7 +36,7 @@ static void validate_teardown(TestInputVisitorData *data,
     data->obj = NULL;
 
     if (data->qiv) {
-        qmp_input_visitor_cleanup(data->qiv);
+        visit_free(data->qiv);
         data->qiv = NULL;
     }
 }
@@ -48,8 +48,6 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
                                             const char *json_string,
                                             va_list *ap)
 {
-    Visitor *v;
-
     validate_teardown(data, NULL);
 
     data->obj = qobject_from_jsonv(json_string, ap);
@@ -57,11 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
 
     data->qiv = qmp_input_visitor_new(data->obj, true);
     g_assert(data->qiv);
-
-    v = qmp_input_get_visitor(data->qiv);
-    g_assert(v);
-
-    return v;
+    return data->qiv;
 }
 
 static GCC_FMT_ATTR(2, 3)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b236183..f583dce 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -22,7 +22,7 @@
 
 typedef struct TestInputVisitorData {
     QObject *obj;
-    QmpInputVisitor *qiv;
+    Visitor *qiv;
 } TestInputVisitorData;
 
 static void visitor_input_teardown(TestInputVisitorData *data,
@@ -32,7 +32,7 @@ static void visitor_input_teardown(TestInputVisitorData *data,
     data->obj = NULL;
 
     if (data->qiv) {
-        qmp_input_visitor_cleanup(data->qiv);
+        visit_free(data->qiv);
         data->qiv = NULL;
     }
 }
@@ -44,8 +44,6 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
                                                  const char *json_string,
                                                  va_list *ap)
 {
-    Visitor *v;
-
     visitor_input_teardown(data, NULL);
 
     data->obj = qobject_from_jsonv(json_string, ap);
@@ -53,11 +51,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
 
     data->qiv = qmp_input_visitor_new(data->obj, false);
     g_assert(data->qiv);
-
-    v = qmp_input_get_visitor(data->qiv);
-    g_assert(v);
-
-    return v;
+    return data->qiv;
 }
 
 static GCC_FMT_ATTR(2, 3)
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index c3c8634..0766dcc 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1013,7 +1013,7 @@ static PrimitiveType pt_values[] = {
 
 typedef struct QmpSerializeData {
     QmpOutputVisitor *qov;
-    QmpInputVisitor *qiv;
+    Visitor *qiv;
 } QmpSerializeData;
 
 static void qmp_serialize(void *native_in, void **datap,
@@ -1041,14 +1041,14 @@ static void qmp_deserialize(void **native_out, void *datap,
     d->qiv = qmp_input_visitor_new(obj, true);
     qobject_decref(obj_orig);
     qobject_decref(obj);
-    visit(qmp_input_get_visitor(d->qiv), native_out, errp);
+    visit(d->qiv, native_out, errp);
 }
 
 static void qmp_cleanup(void *datap)
 {
     QmpSerializeData *d = datap;
     qmp_output_visitor_cleanup(d->qov);
-    qmp_input_visitor_cleanup(d->qiv);
+    visit_free(d->qiv);
 
     g_free(d);
 }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index cc2b043..3677cd2 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1148,7 +1148,6 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest,
                              SocketAddress *src)
 {
     QmpOutputVisitor *qov;
-    QmpInputVisitor *qiv;
     Visitor *ov, *iv;
     QObject *obj;
 
@@ -1163,10 +1162,9 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest,
         return;
     }
 
-    qiv = qmp_input_visitor_new(obj, true);
-    iv = qmp_input_get_visitor(qiv);
+    iv = qmp_input_visitor_new(obj, true);
     visit_type_SocketAddress(iv, NULL, p_dest, &error_abort);
-    qmp_input_visitor_cleanup(qiv);
+    visit_free(iv);
     qobject_decref(obj);
 }
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 08/15] string-output-visitor: Favor new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (6 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 07/15] qmp-input-visitor: " Markus Armbruster
@ 2016-07-06  9:13 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 09/15] qmp-output-visitor: " Markus Armbruster
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:13 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Now that we have a polymorphic visit_free(), we no longer need
string_output_visitor_cleanup(); however, we still need to
expose the subtype for string_output_get_string().

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-9-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hmp.c                                |  2 +-
 include/qapi/string-output-visitor.h |  1 -
 net/net.c                            |  2 +-
 qapi/string-output-visitor.c         |  5 -----
 qom/object.c                         | 11 ++++++-----
 tests/test-string-output-visitor.c   |  2 +-
 tests/test-visitor-serialization.c   |  4 ++--
 7 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/hmp.c b/hmp.c
index fd5daa7..559cad1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2006,7 +2006,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "  host nodes: %s\n", str);
 
         g_free(str);
-        string_output_visitor_cleanup(ov);
+        visit_free(string_output_get_visitor(ov));
         m = m->next;
         i++;
     }
diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
index e10522a..03e377e 100644
--- a/include/qapi/string-output-visitor.h
+++ b/include/qapi/string-output-visitor.h
@@ -23,7 +23,6 @@ typedef struct StringOutputVisitor StringOutputVisitor;
  * requires a non-null list argument to visit_start_list().
  */
 StringOutputVisitor *string_output_visitor_new(bool human);
-void string_output_visitor_cleanup(StringOutputVisitor *v);
 
 char *string_output_get_string(StringOutputVisitor *v);
 Visitor *string_output_get_visitor(StringOutputVisitor *v);
diff --git a/net/net.c b/net/net.c
index d25f802..336469f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1210,7 +1210,7 @@ static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
         object_property_get(OBJECT(nf), string_output_get_visitor(ov),
                             prop->name, NULL);
         str = string_output_get_string(ov);
-        string_output_visitor_cleanup(ov);
+        visit_free(string_output_get_visitor(ov));
         monitor_printf(mon, ",%s=%s", prop->name, str);
         g_free(str);
     }
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index ff9ddf0..78aab87 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -326,11 +326,6 @@ static void string_output_free(Visitor *v)
 {
     StringOutputVisitor *sov = to_sov(v);
 
-    string_output_visitor_cleanup(sov);
-}
-
-void string_output_visitor_cleanup(StringOutputVisitor *sov)
-{
     if (sov->string) {
         g_string_free(sov->string, true);
     }
diff --git a/qom/object.c b/qom/object.c
index 02c0a3a..2407b66 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1242,14 +1242,15 @@ int object_property_get_enum(Object *obj, const char *name,
     enumprop = prop->opaque;
 
     sov = string_output_visitor_new(false);
-    object_property_get(obj, string_output_get_visitor(sov), name, &err);
+    v = string_output_get_visitor(sov);
+    object_property_get(obj, v, name, &err);
     if (err) {
         error_propagate(errp, err);
-        string_output_visitor_cleanup(sov);
+        visit_free(v);
         return 0;
     }
     str = string_output_get_string(sov);
-    string_output_visitor_cleanup(sov);
+    visit_free(v);
     v = string_input_visitor_new(str);
     visit_type_enum(v, name, &ret, enumprop->strings, errp);
 
@@ -1281,7 +1282,7 @@ void object_property_get_uint16List(Object *obj, const char *name,
     g_free(str);
     visit_free(v);
 out:
-    string_output_visitor_cleanup(ov);
+    visit_free(string_output_get_visitor(ov));
 }
 
 void object_property_parse(Object *obj, const char *string,
@@ -1309,7 +1310,7 @@ char *object_property_print(Object *obj, const char *name, bool human,
     string = string_output_get_string(sov);
 
 out:
-    string_output_visitor_cleanup(sov);
+    visit_free(string_output_get_visitor(sov));
     return string;
 }
 
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index edff523..bfa3e8e 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -50,7 +50,7 @@ static void visitor_output_setup_human(TestOutputVisitorData *data,
 static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
-    string_output_visitor_cleanup(data->sov);
+    visit_free(data->ov);
     data->sov = NULL;
     data->ov = NULL;
 }
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 0766dcc..377ee3e 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1047,7 +1047,7 @@ static void qmp_deserialize(void **native_out, void *datap,
 static void qmp_cleanup(void *datap)
 {
     QmpSerializeData *d = datap;
-    qmp_output_visitor_cleanup(d->qov);
+    visit_free(qmp_output_get_visitor(d->qov));
     visit_free(d->qiv);
 
     g_free(d);
@@ -1083,7 +1083,7 @@ static void string_cleanup(void *datap)
 {
     StringSerializeData *d = datap;
 
-    string_output_visitor_cleanup(d->sov);
+    visit_free(string_output_get_visitor(d->sov));
     visit_free(d->siv);
     g_free(d->string);
     g_free(d);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 09/15] qmp-output-visitor: Favor new visit_free() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (7 preceding siblings ...)
  2016-07-06  9:13 ` [Qemu-devel] [PULL 08/15] string-output-visitor: " Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 10/15] tests: Clean up test-string-output-visitor Markus Armbruster
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Now that we have a polymorphic visit_free(), we no longer need
qmp_output_visitor_cleanup(); however, we still need to
expose the subtype for qmp_output_get_qobject().

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-10-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/qapi.c                      |  2 +-
 blockdev.c                        |  2 +-
 include/qapi/qmp-output-visitor.h |  1 -
 qapi/qmp-output-visitor.c         | 14 ++++----------
 qemu-img.c                        |  6 +++---
 qom/qom-qobject.c                 |  2 +-
 replay/replay-input.c             |  2 +-
 tests/check-qnull.c               |  2 +-
 tests/test-qmp-output-visitor.c   |  2 +-
 util/qemu-sockets.c               |  2 +-
 10 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 5594f74..41fe4f9 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -699,7 +699,7 @@ void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
     assert(qobject_type(obj) == QTYPE_QDICT);
     data = qdict_get(qobject_to_qdict(obj), "data");
     dump_qobject(func_fprintf, f, 1, data);
-    qmp_output_visitor_cleanup(ov);
+    visit_free(qmp_output_get_visitor(ov));
 }
 
 void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
diff --git a/blockdev.c b/blockdev.c
index 3a104a0..c05aaa6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4020,7 +4020,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
     }
 
 fail:
-    qmp_output_visitor_cleanup(ov);
+    visit_free(qmp_output_get_visitor(ov));
 }
 
 void qmp_x_blockdev_del(bool has_id, const char *id,
diff --git a/include/qapi/qmp-output-visitor.h b/include/qapi/qmp-output-visitor.h
index 2266770..29c9a2e 100644
--- a/include/qapi/qmp-output-visitor.h
+++ b/include/qapi/qmp-output-visitor.h
@@ -20,7 +20,6 @@
 typedef struct QmpOutputVisitor QmpOutputVisitor;
 
 QmpOutputVisitor *qmp_output_visitor_new(void);
-void qmp_output_visitor_cleanup(QmpOutputVisitor *v);
 
 QObject *qmp_output_get_qobject(QmpOutputVisitor *v);
 Visitor *qmp_output_get_visitor(QmpOutputVisitor *v);
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 5f0035c..3d12623 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -217,21 +217,15 @@ Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)
 static void qmp_output_free(Visitor *v)
 {
     QmpOutputVisitor *qov = to_qov(v);
-
-    qmp_output_visitor_cleanup(qov);
-}
-
-void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
-{
     QStackEntry *e, *tmp;
 
-    QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
-        QTAILQ_REMOVE(&v->stack, e, node);
+    QTAILQ_FOREACH_SAFE(e, &qov->stack, node, tmp) {
+        QTAILQ_REMOVE(&qov->stack, e, node);
         g_free(e);
     }
 
-    qobject_decref(v->root);
-    g_free(v);
+    qobject_decref(qov->root);
+    g_free(qov);
 }
 
 QmpOutputVisitor *qmp_output_visitor_new(void)
diff --git a/qemu-img.c b/qemu-img.c
index debd7f1..728f471 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -500,7 +500,7 @@ static void dump_json_image_check(ImageCheck *check, bool quiet)
     assert(str != NULL);
     qprintf(quiet, "%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
+    visit_free(qmp_output_get_visitor(ov));
     QDECREF(str);
 }
 
@@ -2190,7 +2190,7 @@ static void dump_json_image_info_list(ImageInfoList *list)
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
+    visit_free(qmp_output_get_visitor(ov));
     QDECREF(str);
 }
 
@@ -2206,7 +2206,7 @@ static void dump_json_image_info(ImageInfo *info)
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
+    visit_free(qmp_output_get_visitor(ov));
     QDECREF(str);
 }
 
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index c3c9188..6714565 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -41,6 +41,6 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
         ret = qmp_output_get_qobject(qov);
     }
     error_propagate(errp, local_err);
-    qmp_output_visitor_cleanup(qov);
+    visit_free(qmp_output_get_visitor(qov));
     return ret;
 }
diff --git a/replay/replay-input.c b/replay/replay-input.c
index 7b6fa93..9cbb4c1 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -31,7 +31,7 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src)
     ov = qmp_output_get_visitor(qov);
     visit_type_InputEvent(ov, NULL, &src, &error_abort);
     obj = qmp_output_get_qobject(qov);
-    qmp_output_visitor_cleanup(qov);
+    visit_free(ov);
     if (!obj) {
         return NULL;
     }
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index d0412e4..6310bf7 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -58,7 +58,7 @@ static void qnull_visit_test(void)
     obj = qmp_output_get_qobject(qov);
     g_assert(obj == &qnull_);
     qobject_decref(obj);
-    qmp_output_visitor_cleanup(qov);
+    visit_free(qmp_output_get_visitor(qov));
 
     g_assert(qnull_.refcnt == 1);
 }
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 75c8e1b..45d87fb 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -38,7 +38,7 @@ static void visitor_output_setup(TestOutputVisitorData *data,
 static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
-    qmp_output_visitor_cleanup(data->qov);
+    visit_free(data->ov);
     data->qov = NULL;
     data->ov = NULL;
 }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 3677cd2..01fc154 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1157,7 +1157,7 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest,
     ov = qmp_output_get_visitor(qov);
     visit_type_SocketAddress(ov, NULL, &src, &error_abort);
     obj = qmp_output_get_qobject(qov);
-    qmp_output_visitor_cleanup(qov);
+    visit_free(ov);
     if (!obj) {
         return;
     }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 10/15] tests: Clean up test-string-output-visitor
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (8 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 09/15] qmp-output-visitor: " Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 11/15] tests: Factor out common code in qapi output tests Markus Armbruster
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Use &error_abort and error_free_or_abort() in more places, use
the generated qapi_free_intList() instead of open-coding it,
reduce the scope of some variables, avoid code duplication
during test setup with visitor_output_setup_internal(), and
copy the visitor_reset() concept from the qmp-output test to
the string-output test.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-11-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-string-output-visitor.c | 61 ++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index bfa3e8e..d57a4d3 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -25,26 +25,26 @@ typedef struct TestOutputVisitorData {
     bool human;
 } TestOutputVisitorData;
 
+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);
+    g_assert(data->ov);
+}
+
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->human = false;
-    data->sov = string_output_visitor_new(data->human);
-    g_assert(data->sov != NULL);
-
-    data->ov = string_output_get_visitor(data->sov);
-    g_assert(data->ov != NULL);
+    return visitor_output_setup_internal(data, false);
 }
 
 static void visitor_output_setup_human(TestOutputVisitorData *data,
                                        const void *unused)
 {
-    data->human = true;
-    data->sov = string_output_visitor_new(data->human);
-    g_assert(data->sov != NULL);
-
-    data->ov = string_output_get_visitor(data->sov);
-    g_assert(data->ov != NULL);
+    return visitor_output_setup_internal(data, true);
 }
 
 static void visitor_output_teardown(TestOutputVisitorData *data,
@@ -55,6 +55,14 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
     data->ov = NULL;
 }
 
+static void visitor_reset(TestOutputVisitorData *data)
+{
+    bool human = data->human;
+
+    visitor_output_teardown(data, NULL);
+    visitor_output_setup_internal(data, human);
+}
+
 static void test_visitor_out_int(TestOutputVisitorData *data,
                                  const void *unused)
 {
@@ -106,12 +114,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
             "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
     }
     g_free(str);
-    while (list) {
-        intList *tmp2;
-        tmp2 = list->next;
-        g_free(list);
-        list = tmp2;
-    }
+    qapi_free_intList(list);
 }
 
 static void test_visitor_out_bool(TestOutputVisitorData *data,
@@ -171,12 +174,10 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
                                        const void *unused)
 {
     char *string = NULL;
-    Error *err = NULL;
     char *str;
 
     /* A null string should return "" */
-    visit_type_str(data->ov, NULL, &string, &err);
-    g_assert(!err);
+    visit_type_str(data->ov, NULL, &string, &error_abort);
 
     str = string_output_get_string(data->sov);
     g_assert(str != NULL);
@@ -191,27 +192,24 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
 static void test_visitor_out_enum(TestOutputVisitorData *data,
                                   const void *unused)
 {
-    Error *err = NULL;
     char *str;
     EnumOne i;
 
     for (i = 0; i < ENUM_ONE__MAX; i++) {
-        char *str_human;
-
-        visit_type_EnumOne(data->ov, "unused", &i, &err);
-        g_assert(!err);
-
-        str_human = g_strdup_printf("\"%s\"", EnumOne_lookup[i]);
+        visit_type_EnumOne(data->ov, "unused", &i, &error_abort);
 
         str = string_output_get_string(data->sov);
         g_assert(str != NULL);
         if (data->human) {
+            char *str_human = g_strdup_printf("\"%s\"", EnumOne_lookup[i]);
+
             g_assert_cmpstr(str, ==, str_human);
+            g_free(str_human);
         } else {
             g_assert_cmpstr(str, ==, EnumOne_lookup[i]);
         }
-        g_free(str_human);
-	g_free(str);
+        g_free(str);
+        visitor_reset(data);
     }
 }
 
@@ -224,8 +222,7 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
     for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
         err = NULL;
         visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
-        g_assert(err);
-        error_free(err);
+        error_free_or_abort(&err);
     }
 }
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 11/15] tests: Factor out common code in qapi output tests
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (9 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 10/15] tests: Clean up test-string-output-visitor Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 12/15] qapi: Add new visit_complete() function Markus Armbruster
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Create a new visitor_get() function to capture common
actions taken in collecting output from an output visitor,
to make it easier to refactor the output visitors in a
later patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-12-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-qmp-output-visitor.c    | 78 +++++++++++++-------------------------
 tests/test-string-output-visitor.c | 38 +++++++++----------
 2 files changed, 44 insertions(+), 72 deletions(-)

diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 45d87fb..af24b10 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -23,6 +23,7 @@
 typedef struct TestOutputVisitorData {
     QmpOutputVisitor *qov;
     Visitor *ov;
+    QObject *obj;
 } TestOutputVisitorData;
 
 static void visitor_output_setup(TestOutputVisitorData *data,
@@ -41,6 +42,15 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
     visit_free(data->ov);
     data->qov = NULL;
     data->ov = NULL;
+    qobject_decref(data->obj);
+    data->obj = NULL;
+}
+
+static QObject *visitor_get(TestOutputVisitorData *data)
+{
+    data->obj = qmp_output_get_qobject(data->qov);
+    g_assert(data->obj);
+    return data->obj;
 }
 
 static void visitor_reset(TestOutputVisitorData *data)
@@ -57,12 +67,9 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
 
     visit_type_int(data->ov, NULL, &value, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QINT);
     g_assert_cmpint(qint_get_int(qobject_to_qint(obj)), ==, value);
-
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_bool(TestOutputVisitorData *data,
@@ -73,12 +80,9 @@ static void test_visitor_out_bool(TestOutputVisitorData *data,
 
     visit_type_bool(data->ov, NULL, &value, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QBOOL);
     g_assert(qbool_get_bool(qobject_to_qbool(obj)) == value);
-
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_number(TestOutputVisitorData *data,
@@ -89,12 +93,9 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
 
     visit_type_number(data->ov, NULL, &value, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QFLOAT);
     g_assert(qfloat_get_double(qobject_to_qfloat(obj)) == value);
-
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_string(TestOutputVisitorData *data,
@@ -105,12 +106,9 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
 
     visit_type_str(data->ov, NULL, &string, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QSTRING);
     g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, string);
-
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_no_string(TestOutputVisitorData *data,
@@ -122,12 +120,9 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
     /* A null string should return "" */
     visit_type_str(data->ov, NULL, &string, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QSTRING);
     g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, "");
-
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_enum(TestOutputVisitorData *data,
@@ -139,12 +134,10 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
     for (i = 0; i < ENUM_ONE__MAX; i++) {
         visit_type_EnumOne(data->ov, "unused", &i, &error_abort);
 
-        obj = qmp_output_get_qobject(data->qov);
-        g_assert(obj != NULL);
+        obj = visitor_get(data);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
         g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==,
                         EnumOne_lookup[i]);
-        qobject_decref(obj);
         visitor_reset(data);
     }
 }
@@ -177,8 +170,7 @@ static void test_visitor_out_struct(TestOutputVisitorData *data,
 
     visit_type_TestStruct(data->ov, NULL, &p, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QDICT);
 
     qdict = qobject_to_qdict(obj);
@@ -186,8 +178,6 @@ static void test_visitor_out_struct(TestOutputVisitorData *data,
     g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
     g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
     g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "foo");
-
-    QDECREF(qdict);
 }
 
 static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
@@ -222,8 +212,7 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
 
     visit_type_UserDefTwo(data->ov, "unused", &ud2, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QDICT);
 
     qdict = qobject_to_qdict(obj);
@@ -250,7 +239,6 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
     g_assert_cmpint(qdict_get_int(userdef, "integer"), ==, value);
     g_assert_cmpstr(qdict_get_str(userdef, "string"), ==, string);
 
-    QDECREF(qdict);
     qapi_free_UserDefTwo(ud2);
 }
 
@@ -302,8 +290,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
 
     visit_type_TestStructList(data->ov, NULL, &head, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QLIST);
 
     qlist = qobject_to_qlist(obj);
@@ -324,7 +311,6 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
     }
     g_assert_cmpint(i, ==, max_items);
 
-    QDECREF(qlist);
     qapi_free_TestStructList(head);
 }
 
@@ -368,11 +354,9 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
 
     qobj = QOBJECT(qint_from_int(-42));
     visit_type_any(data->ov, NULL, &qobj, &error_abort);
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     g_assert(qobject_type(obj) == QTYPE_QINT);
     g_assert_cmpint(qint_get_int(qobject_to_qint(obj)), ==, -42);
-    qobject_decref(obj);
     qobject_decref(qobj);
 
     visitor_reset(data);
@@ -383,8 +367,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qobj = QOBJECT(qdict);
     visit_type_any(data->ov, NULL, &qobj, &error_abort);
     qobject_decref(qobj);
-    obj = qmp_output_get_qobject(data->qov);
-    g_assert(obj != NULL);
+    obj = visitor_get(data);
     qdict = qobject_to_qdict(obj);
     g_assert(qdict);
     qobj = qdict_get(qdict, "integer");
@@ -402,7 +385,6 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qstring = qobject_to_qstring(qobj);
     g_assert(qstring);
     g_assert_cmpstr(qstring_get_str(qstring), ==, "foo");
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_union_flat(TestOutputVisitorData *data,
@@ -418,7 +400,7 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     tmp->u.value1.boolean = true;
 
     visit_type_UserDefFlatUnion(data->ov, NULL, &tmp, &error_abort);
-    arg = qmp_output_get_qobject(data->qov);
+    arg = visitor_get(data);
 
     g_assert(qobject_type(arg) == QTYPE_QDICT);
     qdict = qobject_to_qdict(arg);
@@ -429,7 +411,6 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);
 
     qapi_free_UserDefFlatUnion(tmp);
-    QDECREF(qdict);
 }
 
 static void test_visitor_out_alternate(TestOutputVisitorData *data,
@@ -444,13 +425,12 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     tmp->u.i = 42;
 
     visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
-    arg = qmp_output_get_qobject(data->qov);
+    arg = visitor_get(data);
 
     g_assert(qobject_type(arg) == QTYPE_QINT);
     g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);
 
     qapi_free_UserDefAlternate(tmp);
-    qobject_decref(arg);
 
     visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
@@ -458,13 +438,12 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     tmp->u.s = g_strdup("hello");
 
     visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
-    arg = qmp_output_get_qobject(data->qov);
+    arg = visitor_get(data);
 
     g_assert(qobject_type(arg) == QTYPE_QSTRING);
     g_assert_cmpstr(qstring_get_str(qobject_to_qstring(arg)), ==, "hello");
 
     qapi_free_UserDefAlternate(tmp);
-    qobject_decref(arg);
 
     visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
@@ -475,7 +454,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     tmp->u.udfu.u.value1.boolean = true;
 
     visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
-    arg = qmp_output_get_qobject(data->qov);
+    arg = visitor_get(data);
 
     g_assert_cmpint(qobject_type(arg), ==, QTYPE_QDICT);
     qdict = qobject_to_qdict(arg);
@@ -486,7 +465,6 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);
 
     qapi_free_UserDefAlternate(tmp);
-    qobject_decref(arg);
 }
 
 static void test_visitor_out_null(TestOutputVisitorData *data,
@@ -500,14 +478,13 @@ static void test_visitor_out_null(TestOutputVisitorData *data,
     visit_type_null(data->ov, "a", &error_abort);
     visit_check_struct(data->ov, &error_abort);
     visit_end_struct(data->ov, NULL);
-    arg = qmp_output_get_qobject(data->qov);
+    arg = visitor_get(data);
     g_assert(qobject_type(arg) == QTYPE_QDICT);
     qdict = qobject_to_qdict(arg);
     g_assert_cmpint(qdict_size(qdict), ==, 1);
     nil = qdict_get(qdict, "a");
     g_assert(nil);
     g_assert(qobject_type(nil) == QTYPE_QNULL);
-    qobject_decref(arg);
 }
 
 static void init_native_list(UserDefNativeListUnion *cvalue)
@@ -738,10 +715,9 @@ static void test_native_list(TestOutputVisitorData *data,
 
     visit_type_UserDefNativeListUnion(data->ov, NULL, &cvalue, &error_abort);
 
-    obj = qmp_output_get_qobject(data->qov);
+    obj = visitor_get(data);
     check_native_list(obj, cvalue->type);
     qapi_free_UserDefNativeListUnion(cvalue);
-    qobject_decref(obj);
 }
 
 static void test_visitor_out_native_list_int(TestOutputVisitorData *data,
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index d57a4d3..e17035b 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -22,6 +22,7 @@
 typedef struct TestOutputVisitorData {
     StringOutputVisitor *sov;
     Visitor *ov;
+    char *str;
     bool human;
 } TestOutputVisitorData;
 
@@ -53,6 +54,15 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
     visit_free(data->ov);
     data->sov = NULL;
     data->ov = NULL;
+    g_free(data->str);
+    data->str = NULL;
+}
+
+static char *visitor_get(TestOutputVisitorData *data)
+{
+    data->str = string_output_get_string(data->sov);
+    g_assert(data->str);
+    return data->str;
 }
 
 static void visitor_reset(TestOutputVisitorData *data)
@@ -73,14 +83,12 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
     visit_type_int(data->ov, NULL, &value, &err);
     g_assert(!err);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     if (data->human) {
         g_assert_cmpstr(str, ==, "42 (0x2a)");
     } else {
         g_assert_cmpstr(str, ==, "42");
     }
-    g_free(str);
 }
 
 static void test_visitor_out_intList(TestOutputVisitorData *data,
@@ -102,8 +110,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
     visit_type_intList(data->ov, NULL, &list, &err);
     g_assert(err == NULL);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     if (data->human) {
         g_assert_cmpstr(str, ==,
             "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807 "
@@ -113,7 +120,6 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
         g_assert_cmpstr(str, ==,
             "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
     }
-    g_free(str);
     qapi_free_intList(list);
 }
 
@@ -127,10 +133,8 @@ static void test_visitor_out_bool(TestOutputVisitorData *data,
     visit_type_bool(data->ov, NULL, &value, &err);
     g_assert(!err);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     g_assert_cmpstr(str, ==, "true");
-    g_free(str);
 }
 
 static void test_visitor_out_number(TestOutputVisitorData *data,
@@ -143,10 +147,8 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
     visit_type_number(data->ov, NULL, &value, &err);
     g_assert(!err);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     g_assert_cmpstr(str, ==, "3.140000");
-    g_free(str);
 }
 
 static void test_visitor_out_string(TestOutputVisitorData *data,
@@ -160,14 +162,12 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
     visit_type_str(data->ov, NULL, &string, &err);
     g_assert(!err);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     if (data->human) {
         g_assert_cmpstr(str, ==, string_human);
     } else {
         g_assert_cmpstr(str, ==, string);
     }
-    g_free(str);
 }
 
 static void test_visitor_out_no_string(TestOutputVisitorData *data,
@@ -179,14 +179,12 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
     /* A null string should return "" */
     visit_type_str(data->ov, NULL, &string, &error_abort);
 
-    str = string_output_get_string(data->sov);
-    g_assert(str != NULL);
+    str = visitor_get(data);
     if (data->human) {
         g_assert_cmpstr(str, ==, "<null>");
     } else {
         g_assert_cmpstr(str, ==, "");
     }
-    g_free(str);
 }
 
 static void test_visitor_out_enum(TestOutputVisitorData *data,
@@ -198,8 +196,7 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
     for (i = 0; i < ENUM_ONE__MAX; i++) {
         visit_type_EnumOne(data->ov, "unused", &i, &error_abort);
 
-        str = string_output_get_string(data->sov);
-        g_assert(str != NULL);
+        str = visitor_get(data);
         if (data->human) {
             char *str_human = g_strdup_printf("\"%s\"", EnumOne_lookup[i]);
 
@@ -208,7 +205,6 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
         } else {
             g_assert_cmpstr(str, ==, EnumOne_lookup[i]);
         }
-        g_free(str);
         visitor_reset(data);
     }
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 12/15] qapi: Add new visit_complete() function
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (10 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 11/15] tests: Factor out common code in qapi output tests Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 13/15] qapi: Add new clone visitor Markus Armbruster
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

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 --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;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 13/15] qapi: Add new clone visitor
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (11 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 12/15] qapi: Add new visit_complete() function Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 14/15] sockets: Use new QAPI cloning Markus Armbruster
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

We have a couple places in the code base that want to deep-clone
one QAPI object into another, and they were resorting to serializing
the struct out to QObject then reparsing it.  A much more efficient
version can be done by adding a new clone visitor.

Since cloning is still relatively uncommon, expose the use of the
new visitor via a QAPI_CLONE() macro that takes care of type-punning
the underlying function pointer, rather than generating lots of
unused functions for types that won't be cloned.  And yes, we're
relying on the compiler treating all pointers equally, even though
a strict C program cannot portably do so - but we're not the first
one in the qemu code base to expect it to work (hello, glib!).

The choice of adding a fourth visitor type deserves some explanation.
On the surface, the clone visitor is mostly an input visitor (it
takes arbitrary input - in this case, another QAPI object - and
creates a new QAPI object during the course of the visit).  But
ever since commit da72ab0 consolidated enum visits based on the
visitor type, using VISITOR_INPUT would cause us to run
visit_type_str(), even though for cloning there is nothing to do
(we just copy the enum value across, without regards to its mapping
to strings).   Also, since our input happens to be a QAPI object,
we can also satisfy the internal checks for VISITOR_OUTPUT.  So in
the end, I settled with a new VISITOR_CLONE, and chose its value
such that many internal checks can use 'v->type & mask', sticking
to 'v->type == value' where the difference matters.

Note that we can only clone objects (including alternates) and lists,
not built-ins or enums.  The visitor core hides integer width from
the actual visitor (since commit 04e070d), and as long as that's the
case, we can't clone top-level integers.  Then again, those can
always be cloned by direct copy, since they are not objects with
deep pointers, so it's no real loss.  And restricting cloning to
just objects and lists is cleaner than restricting it to non-integers.
As such, I documented that the clone visitor is for direct use only
by code internal to QAPI, and should not be used on incomplete objects
(other than a hack to work around the fact that we allow NULL in place
of "" in visit_type_str() in other output visitors).  Note that as
written, the clone visitor will never fail on a complete object.

Scalars (including enums) not at the root of the clone copy just fine
with no additional effort while visiting the scalar, by virtue of a
g_memdup() each time we push another struct onto the stack.  Cloning
a string requires deduplication of a pointer, which means it can also
provide the guarantee of an input visitor of never producing NULL
even when still accepting NULL in place of "" the way the QMP output
visitor does.

Cloning an 'any' type could be possible by incrementing the QObject
refcnt, but it's not obvious whether that is better than implementing
a QObject deep clone.  So for now, we document it as unsupported,
and intentionally omit the .type_any() callback to let a developer
know their usage needs implementation.

Add testsuite coverage for several different clone situations, to
ensure that the code is working.  I also tested that valgrind was
happy with the test.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-14-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/clone-visitor.h |  37 ++++++++
 include/qapi/visitor-impl.h  |  14 +--
 include/qapi/visitor.h       |  66 ++++++++------
 qapi/Makefile.objs           |   2 +-
 qapi/qapi-clone-visitor.c    | 182 ++++++++++++++++++++++++++++++++++++++
 qapi/qapi-visit-core.c       |  28 ++++--
 tests/.gitignore             |   1 +
 tests/Makefile.include       |   4 +
 tests/test-clone-visitor.c   | 206 +++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 497 insertions(+), 43 deletions(-)
 create mode 100644 include/qapi/clone-visitor.h
 create mode 100644 qapi/qapi-clone-visitor.c
 create mode 100644 tests/test-clone-visitor.c

diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
new file mode 100644
index 0000000..16ceff5
--- /dev/null
+++ b/include/qapi/clone-visitor.h
@@ -0,0 +1,37 @@
+/*
+ * Clone Visitor
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QAPI_CLONE_VISITOR_H
+#define QAPI_CLONE_VISITOR_H
+
+#include "qapi/visitor.h"
+
+/*
+ * The clone visitor is for direct use only by the QAPI_CLONE() macro;
+ * it requires that the root visit occur on an object, list, or
+ * alternate, and is not usable directly on built-in QAPI types.
+ */
+typedef struct QapiCloneVisitor QapiCloneVisitor;
+
+void *qapi_clone(const void *src, void (*visit_type)(Visitor *, const char *,
+                                                     void **, Error **));
+
+/*
+ * Deep-clone QAPI object @src of the given @type, and return the result.
+ *
+ * Not usable on QAPI scalars (integers, strings, enums), nor on a
+ * QAPI object that references the 'any' type.  Safe when @src is NULL.
+ */
+#define QAPI_CLONE(type, src)                                           \
+    ((type *)qapi_clone(src,                                            \
+                        (void (*)(Visitor *, const char *, void**,      \
+                                  Error **))visit_type_ ## type))
+
+#endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 16e0b86..8bd47ee 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -27,14 +27,18 @@
  */
 
 /*
- * There are three classes of visitors; setting the class determines
+ * There are four classes of visitors; setting the class determines
  * how QAPI enums are visited, as well as what additional restrictions
- * can be asserted.
+ * can be asserted.  The values are intentionally chosen so as to
+ * permit some assertions based on whether a given bit is set (that
+ * is, some assertions apply to input and clone visitors, some
+ * assertions apply to output and clone visitors).
  */
 typedef enum VisitorType {
-    VISITOR_INPUT,
-    VISITOR_OUTPUT,
-    VISITOR_DEALLOC,
+    VISITOR_INPUT = 1,
+    VISITOR_OUTPUT = 2,
+    VISITOR_CLONE = 3,
+    VISITOR_DEALLOC = 4,
 } VisitorType;
 
 struct Visitor
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 00a60ea..fb8f4eb 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -24,15 +24,16 @@
  * for doing work at each node of a QAPI graph; it can also be used
  * for a virtual walk, where there is no actual QAPI C struct.
  *
- * There are three kinds of visitor classes: input visitors (QMP,
+ * There are four kinds of visitor classes: input visitors (QMP,
  * string, and QemuOpts) parse an external representation and build
  * the corresponding QAPI graph, output visitors (QMP and string) take
- * a completed QAPI graph and generate an external representation, and
- * the dealloc visitor can take a QAPI graph (possibly partially
- * constructed) and recursively free its resources.  While the dealloc
- * and QMP input/output visitors are general, the string and QemuOpts
- * visitors have some implementation limitations; see the
- * documentation for each visitor for more details on what it
+ * a completed QAPI graph and generate an external representation, the
+ * dealloc visitor can take a QAPI graph (possibly partially
+ * constructed) and recursively free its resources, and the clone
+ * visitor performs a deep clone of one QAPI object to another.  While
+ * the dealloc and QMP input/output visitors are general, the string,
+ * QemuOpts, and clone visitors have some implementation limitations;
+ * see the documentation for each visitor for more details on what it
  * supports.  Also, see visitor-impl.h for the callback contracts
  * implemented by each visitor, and docs/qapi-code-gen.txt for more
  * about the QAPI code generator.
@@ -80,9 +81,9 @@
  *
  * If an error is detected during visit_type_FOO() with an input
  * visitor, then *@obj will be NULL for pointer types, and left
- * unchanged for scalar types.  Using an output visitor with an
- * incomplete object has undefined behavior (other than a special case
- * for visit_type_str() treating NULL like ""), while the dealloc
+ * unchanged for scalar types.  Using an output or clone visitor with
+ * an incomplete object has undefined behavior (other than a special
+ * case for visit_type_str() treating NULL like ""), while the dealloc
  * visitor safely handles incomplete objects.  Since input visitors
  * never produce an incomplete object, such an object is possible only
  * by manual construction.
@@ -102,11 +103,19 @@
  *
  * void qapi_free_FOO(FOO *obj);
  *
- * which behaves like free() in that @obj may be NULL.  Because of
- * these functions, the dealloc visitor is seldom used directly
- * outside of generated code.  QAPI types can also inherit from a base
- * class; when this happens, a function is generated for easily going
- * from the derived type to the base type:
+ * where behaves like free() in that @obj may be NULL.  Such objects
+ * may also be used with the following macro, provided alongside the
+ * clone visitor:
+ *
+ * Type *QAPI_CLONE(Type, src);
+ *
+ * in order to perform a deep clone of @src.  Because of the generated
+ * qapi_free functions and the QAPI_CLONE() macro, the clone and
+ * dealloc visitor should not be used directly outside of QAPI code.
+ *
+ * QAPI types can also inherit from a base class; when this happens, a
+ * function is generated for easily going from the derived type to the
+ * base type:
  *
  * BASE *qapi_CHILD_base(CHILD *obj);
  *
@@ -272,9 +281,9 @@ void visit_free(Visitor *v);
  * container; see the general description of @name above.
  *
  * @obj must be non-NULL for a real walk, in which case @size
- * determines how much memory an input visitor will allocate into
- * *@obj.  @obj may also be NULL for a virtual walk, in which case
- * @size is ignored.
+ * determines how much memory an input or clone visitor will allocate
+ * into *@obj.  @obj may also be NULL for a virtual walk, in which
+ * case @size is ignored.
  *
  * @errp obeys typical error usage, and reports failures such as a
  * member @name is not present, or present but not an object.  On
@@ -327,9 +336,9 @@ void visit_end_struct(Visitor *v, void **obj);
  * container; see the general description of @name above.
  *
  * @list must be non-NULL for a real walk, in which case @size
- * determines how much memory an input visitor will allocate into
- * *@list (at least sizeof(GenericList)).  Some visitors also allow
- * @list to be NULL for a virtual walk, in which case @size is
+ * determines how much memory an input or clone visitor will allocate
+ * into *@list (at least sizeof(GenericList)).  Some visitors also
+ * allow @list to be NULL for a virtual walk, in which case @size is
  * ignored.
  *
  * @errp obeys typical error usage, and reports failures such as a
@@ -386,10 +395,10 @@ void visit_end_list(Visitor *v, void **list);
  * @name expresses the relationship of this alternate to its parent
  * container; see the general description of @name above.
  *
- * @obj must not be NULL. Input visitors use @size to determine how
- * much memory to allocate into *@obj, then determine the qtype of the
- * next thing to be visited, stored in (*@obj)->type.  Other visitors
- * will leave @obj unchanged.
+ * @obj must not be NULL. Input and clone visitors use @size to
+ * determine how much memory to allocate into *@obj, then determine
+ * the qtype of the next thing to be visited, stored in (*@obj)->type.
+ * Other visitors will leave @obj unchanged.
  *
  * If @promote_int, treat integers as QTYPE_FLOAT.
  *
@@ -554,9 +563,10 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp);
  * @name expresses the relationship of this string to its parent
  * container; see the general description of @name above.
  *
- * @obj must be non-NULL.  Input visitors set *@obj to the value
- * (never NULL).  Other visitors leave *@obj unchanged, and commonly
- * treat NULL like "".
+ * @obj must be non-NULL.  Input and clone visitors set *@obj to the
+ * value (always using "" rather than NULL for an empty string).
+ * Other visitors leave *@obj unchanged, and commonly treat NULL like
+ * "".
  *
  * It is safe to cast away const when preparing a (const char *) value
  * into @obj for use by an output visitor.
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 2278970..7ea4aeb 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,6 +1,6 @@
 util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
 util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
-util-obj-y += opts-visitor.o
+util-obj-y += opts-visitor.o qapi-clone-visitor.o
 util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
new file mode 100644
index 0000000..0bb8216
--- /dev/null
+++ b/qapi/qapi-clone-visitor.c
@@ -0,0 +1,182 @@
+/*
+ * Copy one QAPI object to another
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/visitor-impl.h"
+#include "qapi/error.h"
+
+struct QapiCloneVisitor {
+    Visitor visitor;
+    size_t depth;
+};
+
+static QapiCloneVisitor *to_qcv(Visitor *v)
+{
+    return container_of(v, QapiCloneVisitor, visitor);
+}
+
+static void qapi_clone_start_struct(Visitor *v, const char *name, void **obj,
+                                    size_t size, Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    if (!obj) {
+        assert(qcv->depth);
+        /* Only possible when visiting an alternate's object
+         * branch. Nothing further to do here, since the earlier
+         * visit_start_alternate() already copied memory. */
+        return;
+    }
+
+    *obj = g_memdup(*obj, size);
+    qcv->depth++;
+}
+
+static void qapi_clone_end(Visitor *v, void **obj)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    if (obj) {
+        qcv->depth--;
+    }
+}
+
+static void qapi_clone_start_list(Visitor *v, const char *name,
+                                  GenericList **listp, size_t size,
+                                  Error **errp)
+{
+    qapi_clone_start_struct(v, name, (void **)listp, size, errp);
+}
+
+static GenericList *qapi_clone_next_list(Visitor *v, GenericList *tail,
+                                         size_t size)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Unshare the tail of the list cloned by g_memdup() */
+    tail->next = g_memdup(tail->next, size);
+    return tail->next;
+}
+
+static void qapi_clone_start_alternate(Visitor *v, const char *name,
+                                       GenericAlternate **obj, size_t size,
+                                       bool promote_int, Error **errp)
+{
+    qapi_clone_start_struct(v, name, (void **)obj, size, errp);
+}
+
+static void qapi_clone_type_int64(Visitor *v, const char *name, int64_t *obj,
+                                   Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Value was already cloned by g_memdup() */
+}
+
+static void qapi_clone_type_uint64(Visitor *v, const char *name,
+                                    uint64_t *obj, Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Value was already cloned by g_memdup() */
+}
+
+static void qapi_clone_type_bool(Visitor *v, const char *name, bool *obj,
+                                  Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Value was already cloned by g_memdup() */
+}
+
+static void qapi_clone_type_str(Visitor *v, const char *name, char **obj,
+                                 Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /*
+     * Pointer was already cloned by g_memdup; create fresh copy.
+     * Note that as long as qmp-output-visitor accepts NULL instead of
+     * "", then we must do likewise. However, we want to obey the
+     * input visitor semantics of never producing NULL when the empty
+     * string is intended.
+     */
+    *obj = g_strdup(*obj ?: "");
+}
+
+static void qapi_clone_type_number(Visitor *v, const char *name, double *obj,
+                                    Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Value was already cloned by g_memdup() */
+}
+
+static void qapi_clone_type_null(Visitor *v, const char *name, Error **errp)
+{
+    QapiCloneVisitor *qcv = to_qcv(v);
+
+    assert(qcv->depth);
+    /* Nothing to do */
+}
+
+static void qapi_clone_free(Visitor *v)
+{
+    g_free(v);
+}
+
+static Visitor *qapi_clone_visitor_new(void)
+{
+    QapiCloneVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type = VISITOR_CLONE;
+    v->visitor.start_struct = qapi_clone_start_struct;
+    v->visitor.end_struct = qapi_clone_end;
+    v->visitor.start_list = qapi_clone_start_list;
+    v->visitor.next_list = qapi_clone_next_list;
+    v->visitor.end_list = qapi_clone_end;
+    v->visitor.start_alternate = qapi_clone_start_alternate;
+    v->visitor.end_alternate = qapi_clone_end;
+    v->visitor.type_int64 = qapi_clone_type_int64;
+    v->visitor.type_uint64 = qapi_clone_type_uint64;
+    v->visitor.type_bool = qapi_clone_type_bool;
+    v->visitor.type_str = qapi_clone_type_str;
+    v->visitor.type_number = qapi_clone_type_number;
+    v->visitor.type_null = qapi_clone_type_null;
+    v->visitor.free = qapi_clone_free;
+
+    return &v->visitor;
+}
+
+void *qapi_clone(const void *src, void (*visit_type)(Visitor *, const char *,
+                                                     void **, Error **))
+{
+    Visitor *v;
+    void *dst = (void *) src; /* Cast away const */
+
+    if (!src) {
+        return NULL;
+    }
+
+    v = qapi_clone_visitor_new();
+    visit_type(v, NULL, &dst, &error_abort);
+    visit_free(v);
+    return dst;
+}
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index eb7dd72..55f5876 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -42,10 +42,10 @@ void visit_start_struct(Visitor *v, const char *name, void **obj,
 
     if (obj) {
         assert(size);
-        assert(v->type != VISITOR_OUTPUT || *obj);
+        assert(!(v->type & VISITOR_OUTPUT) || *obj);
     }
     v->start_struct(v, name, obj, size, &err);
-    if (obj && v->type == VISITOR_INPUT) {
+    if (obj && (v->type & VISITOR_INPUT)) {
         assert(!err != !*obj);
     }
     error_propagate(errp, err);
@@ -70,7 +70,7 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list,
 
     assert(!list || size >= sizeof(GenericList));
     v->start_list(v, name, list, size, &err);
-    if (list && v->type == VISITOR_INPUT) {
+    if (list && (v->type & VISITOR_INPUT)) {
         assert(!(err && *list));
     }
     error_propagate(errp, err);
@@ -94,11 +94,11 @@ void visit_start_alternate(Visitor *v, const char *name,
     Error *err = NULL;
 
     assert(obj && size >= sizeof(GenericAlternate));
-    assert(v->type != VISITOR_OUTPUT || *obj);
+    assert(!(v->type & VISITOR_OUTPUT) || *obj);
     if (v->start_alternate) {
         v->start_alternate(v, name, obj, size, promote_int, &err);
     }
-    if (v->type == VISITOR_INPUT) {
+    if (v->type & VISITOR_INPUT) {
         assert(v->start_alternate && !err != !*obj);
     }
     error_propagate(errp, err);
@@ -250,10 +250,10 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
     assert(obj);
     /* TODO: Fix callers to not pass NULL when they mean "", so that we
      * can enable:
-    assert(v->type != VISITOR_OUTPUT || *obj);
+    assert(!(v->type & VISITOR_OUTPUT) || *obj);
      */
     v->type_str(v, name, obj, &err);
-    if (v->type == VISITOR_INPUT) {
+    if (v->type & VISITOR_INPUT) {
         assert(!err != !*obj);
     }
     error_propagate(errp, err);
@@ -335,9 +335,19 @@ void visit_type_enum(Visitor *v, const char *name, int *obj,
                      const char *const strings[], Error **errp)
 {
     assert(obj && strings);
-    if (v->type == VISITOR_INPUT) {
+    switch (v->type) {
+    case VISITOR_INPUT:
         input_type_enum(v, name, obj, strings, errp);
-    } else if (v->type == VISITOR_OUTPUT) {
+        break;
+    case VISITOR_OUTPUT:
         output_type_enum(v, name, obj, strings, errp);
+        break;
+    case VISITOR_CLONE:
+        /* nothing further to do, scalar value was already copied by
+         * g_memdup() during visit_start_*() */
+        break;
+    case VISITOR_DEALLOC:
+        /* nothing to deallocate for a scalar */
+        break;
     }
 }
diff --git a/tests/.gitignore b/tests/.gitignore
index 840ea39..dbb5263 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -13,6 +13,7 @@ test-aio
 test-base64
 test-bitops
 test-blockjob-txn
+test-clone-visitor
 test-coroutine
 test-crypto-afsplit
 test-crypto-block
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f8e3c6b..2010b11 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -22,6 +22,8 @@ check-unit-y += tests/check-qjson$(EXESUF)
 gcov-files-check-qjson-y = qobject/qjson.c
 check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
 gcov-files-test-qmp-output-visitor-y = qapi/qmp-output-visitor.c
+check-unit-y += tests/test-clone-visitor$(EXESUF)
+gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c
 check-unit-y += tests/test-qmp-input-visitor$(EXESUF)
 gcov-files-test-qmp-input-visitor-y = qapi/qmp-input-visitor.c
 check-unit-y += tests/test-qmp-input-strict$(EXESUF)
@@ -402,6 +404,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qjson.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
 	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
+	tests/test-clone-visitor.o \
 	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
 	tests/test-qmp-commands.o tests/test-visitor-serialization.o \
 	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
@@ -499,6 +502,7 @@ tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y)
 tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
 tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y)
+tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y)
 tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y)
 tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y)
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
diff --git a/tests/test-clone-visitor.c b/tests/test-clone-visitor.c
new file mode 100644
index 0000000..df0c045
--- /dev/null
+++ b/tests/test-clone-visitor.c
@@ -0,0 +1,206 @@
+/*
+ * QAPI Clone Visitor unit-tests.
+ *
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include <glib.h>
+
+#include "qemu-common.h"
+#include "qapi/clone-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qapi/qmp/types.h"
+
+static void test_clone_struct(void)
+{
+    UserDefOne *src, *dst;
+
+    src = g_new0(UserDefOne, 1);
+    src->integer = 42;
+    src->string = g_strdup("Hello");
+    src->has_enum1 = false;
+    src->enum1 = ENUM_ONE_VALUE2;
+
+    dst = QAPI_CLONE(UserDefOne, src);
+    g_assert(dst);
+    g_assert_cmpint(dst->integer, ==, 42);
+    g_assert(dst->string != src->string);
+    g_assert_cmpstr(dst->string, ==, "Hello");
+    g_assert_cmpint(dst->has_enum1, ==, false);
+    /* Our implementation does this, but it is not required:
+    g_assert_cmpint(dst->enum1, ==, ENUM_ONE_VALUE2);
+    */
+
+    qapi_free_UserDefOne(src);
+    qapi_free_UserDefOne(dst);
+}
+
+static void test_clone_alternate(void)
+{
+    AltStrBool *b_src, *s_src, *b_dst, *s_dst;
+
+    b_src = g_new0(AltStrBool, 1);
+    b_src->type = QTYPE_QBOOL;
+    b_src->u.b = true;
+    s_src = g_new0(AltStrBool, 1);
+    s_src->type = QTYPE_QSTRING;
+    s_src->u.s = g_strdup("World");
+
+    b_dst = QAPI_CLONE(AltStrBool, b_src);
+    g_assert(b_dst);
+    g_assert_cmpint(b_dst->type, ==, b_src->type);
+    g_assert_cmpint(b_dst->u.b, ==, b_src->u.b);
+    s_dst = QAPI_CLONE(AltStrBool, s_src);
+    g_assert(s_dst);
+    g_assert_cmpint(s_dst->type, ==, s_src->type);
+    g_assert_cmpstr(s_dst->u.s, ==, s_src->u.s);
+    g_assert(s_dst->u.s != s_src->u.s);
+
+    qapi_free_AltStrBool(b_src);
+    qapi_free_AltStrBool(s_src);
+    qapi_free_AltStrBool(b_dst);
+    qapi_free_AltStrBool(s_dst);
+}
+
+static void test_clone_native_list(void)
+{
+    uint8List *src, *dst;
+    uint8List *tmp = NULL;
+    int i;
+
+    /* Build list in reverse */
+    for (i = 10; i; i--) {
+        src = g_new0(uint8List, 1);
+        src->next = tmp;
+        src->value = i;
+        tmp = src;
+    }
+
+    dst = QAPI_CLONE(uint8List, src);
+    for (tmp = dst, i = 1; i <= 10; i++) {
+        g_assert(tmp);
+        g_assert_cmpint(tmp->value, ==, i);
+        tmp = tmp->next;
+    }
+    g_assert(!tmp);
+
+    qapi_free_uint8List(src);
+    qapi_free_uint8List(dst);
+}
+
+static void test_clone_empty(void)
+{
+    Empty2 *src, *dst;
+
+    src = g_new0(Empty2, 1);
+    dst = QAPI_CLONE(Empty2, src);
+    g_assert(dst);
+    qapi_free_Empty2(src);
+    qapi_free_Empty2(dst);
+}
+
+static void test_clone_complex1(void)
+{
+    UserDefNativeListUnion *src, *dst;
+
+    src = g_new0(UserDefNativeListUnion, 1);
+    src->type = USER_DEF_NATIVE_LIST_UNION_KIND_STRING;
+
+    dst = QAPI_CLONE(UserDefNativeListUnion, src);
+    g_assert(dst);
+    g_assert_cmpint(dst->type, ==, src->type);
+    g_assert(!dst->u.string.data);
+
+    qapi_free_UserDefNativeListUnion(src);
+    qapi_free_UserDefNativeListUnion(dst);
+}
+
+static void test_clone_complex2(void)
+{
+    WrapAlternate *src, *dst;
+
+    src = g_new0(WrapAlternate, 1);
+    src->alt = g_new(UserDefAlternate, 1);
+    src->alt->type = QTYPE_QDICT;
+    src->alt->u.udfu.integer = 42;
+    /* Clone intentionally converts NULL into "" for strings */
+    src->alt->u.udfu.string = NULL;
+    src->alt->u.udfu.enum1 = ENUM_ONE_VALUE3;
+    src->alt->u.udfu.u.value3.intb = 99;
+    src->alt->u.udfu.u.value3.has_a_b = true;
+    src->alt->u.udfu.u.value3.a_b = true;
+
+    dst = QAPI_CLONE(WrapAlternate, src);
+    g_assert(dst);
+    g_assert(dst->alt);
+    g_assert_cmpint(dst->alt->type, ==, QTYPE_QDICT);
+    g_assert_cmpint(dst->alt->u.udfu.integer, ==, 42);
+    g_assert_cmpstr(dst->alt->u.udfu.string, ==, "");
+    g_assert_cmpint(dst->alt->u.udfu.enum1, ==, ENUM_ONE_VALUE3);
+    g_assert_cmpint(dst->alt->u.udfu.u.value3.intb, ==, 99);
+    g_assert_cmpint(dst->alt->u.udfu.u.value3.has_a_b, ==, true);
+    g_assert_cmpint(dst->alt->u.udfu.u.value3.a_b, ==, true);
+
+    qapi_free_WrapAlternate(src);
+    qapi_free_WrapAlternate(dst);
+}
+
+static void test_clone_complex3(void)
+{
+    __org_qemu_x_Struct2 *src, *dst;
+    __org_qemu_x_Union1List *tmp;
+
+    src = g_new0(__org_qemu_x_Struct2, 1);
+    tmp = src->array = g_new0(__org_qemu_x_Union1List, 1);
+    tmp->value = g_new0(__org_qemu_x_Union1, 1);
+    tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
+    tmp->value->u.__org_qemu_x_branch.data = g_strdup("one");
+    tmp = tmp->next = g_new0(__org_qemu_x_Union1List, 1);
+    tmp->value = g_new0(__org_qemu_x_Union1, 1);
+    tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
+    tmp->value->u.__org_qemu_x_branch.data = g_strdup("two");
+    tmp = tmp->next = g_new0(__org_qemu_x_Union1List, 1);
+    tmp->value = g_new0(__org_qemu_x_Union1, 1);
+    tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
+    tmp->value->u.__org_qemu_x_branch.data = g_strdup("three");
+
+    dst = QAPI_CLONE(__org_qemu_x_Struct2, src);
+    g_assert(dst);
+    tmp = dst->array;
+    g_assert(tmp);
+    g_assert(tmp->value);
+    g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "one");
+    tmp = tmp->next;
+    g_assert(tmp);
+    g_assert(tmp->value);
+    g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "two");
+    tmp = tmp->next;
+    g_assert(tmp);
+    g_assert(tmp->value);
+    g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "three");
+    tmp = tmp->next;
+    g_assert(!tmp);
+
+    qapi_free___org_qemu_x_Struct2(src);
+    qapi_free___org_qemu_x_Struct2(dst);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/visitor/clone/struct", test_clone_struct);
+    g_test_add_func("/visitor/clone/alternate", test_clone_alternate);
+    g_test_add_func("/visitor/clone/native_list", test_clone_native_list);
+    g_test_add_func("/visitor/clone/empty", test_clone_empty);
+    g_test_add_func("/visitor/clone/complex1", test_clone_complex1);
+    g_test_add_func("/visitor/clone/complex2", test_clone_complex2);
+    g_test_add_func("/visitor/clone/complex3", test_clone_complex3);
+
+    return g_test_run();
+}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 14/15] sockets: Use new QAPI cloning
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (12 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 13/15] qapi: Add new clone visitor Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06  9:14 ` [Qemu-devel] [PULL 15/15] replay: " Markus Armbruster
  2016-07-06 11:46 ` [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Peter Maydell
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Rather than rolling our own clone via an expensive conversion
in and back out of QObject, use the new clone visitor.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-15-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/io/task.h            |  2 +-
 include/qapi/clone-visitor.h |  2 ++
 include/qemu/sockets.h       |  4 ----
 io/channel-socket.c          |  9 +++++----
 qemu-char.c                  |  5 ++---
 util/qemu-sockets.c          | 23 -----------------------
 6 files changed, 10 insertions(+), 35 deletions(-)

diff --git a/include/io/task.h b/include/io/task.h
index a993212..df9499a 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -159,7 +159,7 @@ typedef int (*QIOTaskWorker)(QIOTask *task,
  *      QIOTask *task;
  *      SocketAddress *addrCopy;
  *
- *      qapi_copy_SocketAddress(&addrCopy, addr);
+ *      addrCopy = QAPI_CLONE(SocketAddress, addr);
  *      task = qio_task_new(OBJECT(obj), func, opaque, notify);
  *
  *      qio_task_run_in_thread(task, myobject_listen_worker,
diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
index 16ceff5..b16177e 100644
--- a/include/qapi/clone-visitor.h
+++ b/include/qapi/clone-visitor.h
@@ -11,7 +11,9 @@
 #ifndef QAPI_CLONE_VISITOR_H
 #define QAPI_CLONE_VISITOR_H
 
+#include "qemu/typedefs.h"
 #include "qapi/visitor.h"
+#include "qapi-visit.h"
 
 /*
  * The clone visitor is for direct use only by the QAPI_CLONE() macro;
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 462033a..2f3763f 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -107,10 +107,6 @@ SocketAddress *socket_local_address(int fd, Error **errp);
  */
 SocketAddress *socket_remote_address(int fd, Error **errp);
 
-
-void qapi_copy_SocketAddress(SocketAddress **p_dest,
-                             SocketAddress *src);
-
 /**
  * socket_address_to_string:
  * @addr: the socket address struct
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 6ec87f8..196a4f1 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -23,6 +23,7 @@
 #include "io/channel-socket.h"
 #include "io/channel-watch.h"
 #include "trace.h"
+#include "qapi/clone-visitor.h"
 
 #define SOCKET_MAX_FDS 16
 
@@ -189,7 +190,7 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
         OBJECT(ioc), callback, opaque, destroy);
     SocketAddress *addrCopy;
 
-    qapi_copy_SocketAddress(&addrCopy, addr);
+    addrCopy = QAPI_CLONE(SocketAddress, addr);
 
     /* socket_connect() does a non-blocking connect(), but it
      * still blocks in DNS lookups, so we must use a thread */
@@ -251,7 +252,7 @@ void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
         OBJECT(ioc), callback, opaque, destroy);
     SocketAddress *addrCopy;
 
-    qapi_copy_SocketAddress(&addrCopy, addr);
+    addrCopy = QAPI_CLONE(SocketAddress, addr);
 
     /* socket_listen() blocks in DNS lookups, so we must use a thread */
     trace_qio_channel_socket_listen_async(ioc, addr);
@@ -331,8 +332,8 @@ void qio_channel_socket_dgram_async(QIOChannelSocket *ioc,
     struct QIOChannelSocketDGramWorkerData *data = g_new0(
         struct QIOChannelSocketDGramWorkerData, 1);
 
-    qapi_copy_SocketAddress(&data->localAddr, localAddr);
-    qapi_copy_SocketAddress(&data->remoteAddr, remoteAddr);
+    data->localAddr = QAPI_CLONE(SocketAddress, localAddr);
+    data->remoteAddr = QAPI_CLONE(SocketAddress, remoteAddr);
 
     trace_qio_channel_socket_dgram_async(ioc, localAddr, remoteAddr);
     qio_task_run_in_thread(task,
diff --git a/qemu-char.c b/qemu-char.c
index b73969d..0698b98 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,8 +32,7 @@
 #include "sysemu/char.h"
 #include "hw/usb.h"
 #include "qmp-commands.h"
-#include "qapi/qmp-input-visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/clone-visitor.h"
 #include "qapi-visit.h"
 #include "qemu/base64.h"
 #include "io/channel-socket.h"
@@ -4389,7 +4388,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
         }
     }
 
-    qapi_copy_SocketAddress(&s->addr, sock->addr);
+    s->addr = QAPI_CLONE(SocketAddress, sock->addr);
 
     chr->opaque = s;
     chr->chr_write = tcp_chr_write;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index a0ca6d4..fb83d48 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1143,29 +1143,6 @@ SocketAddress *socket_remote_address(int fd, Error **errp)
     return socket_sockaddr_to_address(&ss, sslen, errp);
 }
 
-
-void qapi_copy_SocketAddress(SocketAddress **p_dest,
-                             SocketAddress *src)
-{
-    Visitor *ov, *iv;
-    QObject *obj;
-
-    *p_dest = NULL;
-
-    ov = qmp_output_visitor_new(&obj);
-    visit_type_SocketAddress(ov, NULL, &src, &error_abort);
-    visit_complete(ov, &obj);
-    visit_free(ov);
-    if (!obj) {
-        return;
-    }
-
-    iv = qmp_input_visitor_new(obj, true);
-    visit_type_SocketAddress(iv, NULL, p_dest, &error_abort);
-    visit_free(iv);
-    qobject_decref(obj);
-}
-
 char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
 {
     char *buf;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PULL 15/15] replay: Use new QAPI cloning
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (13 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 14/15] sockets: Use new QAPI cloning Markus Armbruster
@ 2016-07-06  9:14 ` Markus Armbruster
  2016-07-06 11:46 ` [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Peter Maydell
  15 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2016-07-06  9:14 UTC (permalink / raw)
  To: qemu-devel

From: Eric Blake <eblake@redhat.com>

Rather than rolling our own clone via an expensive conversion
in and back out of QObject, use the new clone visitor.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-16-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 replay/replay-input.c | 30 +++---------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/replay/replay-input.c b/replay/replay-input.c
index 296399c..bd93554 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -16,31 +16,7 @@
 #include "replay-internal.h"
 #include "qemu/notify.h"
 #include "ui/input.h"
-#include "qapi/qmp-output-visitor.h"
-#include "qapi/qmp-input-visitor.h"
-#include "qapi-visit.h"
-
-static InputEvent *qapi_clone_InputEvent(InputEvent *src)
-{
-    Visitor *ov, *iv;
-    QObject *obj;
-    InputEvent *dst = NULL;
-
-    ov = qmp_output_visitor_new(&obj);
-    visit_type_InputEvent(ov, NULL, &src, &error_abort);
-    visit_complete(ov, &obj);
-    visit_free(ov);
-    if (!obj) {
-        return NULL;
-    }
-
-    iv = qmp_input_visitor_new(obj, true);
-    visit_type_InputEvent(iv, NULL, &dst, &error_abort);
-    visit_free(iv);
-    qobject_decref(obj);
-
-    return dst;
-}
+#include "qapi/clone-visitor.h"
 
 void replay_save_input_event(InputEvent *evt)
 {
@@ -139,7 +115,7 @@ InputEvent *replay_read_input_event(void)
         break;
     }
 
-    return qapi_clone_InputEvent(&evt);
+    return QAPI_CLONE(InputEvent, &evt);
 }
 
 void replay_input_event(QemuConsole *src, InputEvent *evt)
@@ -147,7 +123,7 @@ void replay_input_event(QemuConsole *src, InputEvent *evt)
     if (replay_mode == REPLAY_MODE_PLAY) {
         /* Nothing */
     } else if (replay_mode == REPLAY_MODE_RECORD) {
-        replay_add_input_event(qapi_clone_InputEvent(evt));
+        replay_add_input_event(QAPI_CLONE(InputEvent, evt));
     } else {
         qemu_input_event_send_impl(src, evt);
     }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06
  2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
                   ` (14 preceding siblings ...)
  2016-07-06  9:14 ` [Qemu-devel] [PULL 15/15] replay: " Markus Armbruster
@ 2016-07-06 11:46 ` Peter Maydell
  15 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-07-06 11:46 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers

On 6 July 2016 at 10:13, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit 791b7d2340cfafcac9af7864343cf23504d57804:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2016-07-05 16:48:24 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2016-07-06
>
> for you to fetch changes up to b6954712abea03afd686b724060f9873e2c61f2b:
>
>   replay: Use new QAPI cloning (2016-07-06 10:52:04 +0200)
>
> ----------------------------------------------------------------
> QAPI patches for 2016-07-06
>
> ----------------------------------------------------------------
> Eric Blake (15):
>       qapi: Improve use of qmp/types.h
>       qemu-img: Don't leak errors when outputting JSON
>       qapi: Add parameter to visit_end_*
>       qapi: Add new visit_free() function
>       opts-visitor: Favor new visit_free() function
>       string-input-visitor: Favor new visit_free() function
>       qmp-input-visitor: Favor new visit_free() function
>       string-output-visitor: Favor new visit_free() function
>       qmp-output-visitor: Favor new visit_free() function
>       tests: Clean up test-string-output-visitor
>       tests: Factor out common code in qapi output tests
>       qapi: Add new visit_complete() function
>       qapi: Add new clone visitor
>       sockets: Use new QAPI cloning
>       replay: Use new QAPI cloning

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-07-06 11:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-06  9:13 [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 01/15] qapi: Improve use of qmp/types.h Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 02/15] qemu-img: Don't leak errors when outputting JSON Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 03/15] qapi: Add parameter to visit_end_* Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 04/15] qapi: Add new visit_free() function Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 05/15] opts-visitor: Favor " Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 06/15] string-input-visitor: " Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 07/15] qmp-input-visitor: " Markus Armbruster
2016-07-06  9:13 ` [Qemu-devel] [PULL 08/15] string-output-visitor: " Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 09/15] qmp-output-visitor: " Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 10/15] tests: Clean up test-string-output-visitor Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 11/15] tests: Factor out common code in qapi output tests Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 12/15] qapi: Add new visit_complete() function Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 13/15] qapi: Add new clone visitor Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 14/15] sockets: Use new QAPI cloning Markus Armbruster
2016-07-06  9:14 ` [Qemu-devel] [PULL 15/15] replay: " Markus Armbruster
2016-07-06 11:46 ` [Qemu-devel] [PULL 00/15] QAPI patches for 2016-07-06 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).