All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor
@ 2015-12-10 23:53 Eric Blake
  2015-12-10 23:53   ` [Qemu-devel] " Eric Blake
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel

I wrote this series for several reasons:
1. I've been doing a lot of churn in the qapi visitor interfaces
lately; adding a new visitor is good proof whether the changes
still make sense
2. Alexander ended up writing his own simple JSON generator as
qjson.c in his work for vmstate self-description, rather than
reusing existing code, because the QObject JSON generator does
not have an easy entry point
3. Fam commented while trying to enhance 'qemu-img map' that we
are rather wasteful in that there is no way to go directly from
a qapi type to JSON without an intermediate QObject creation

RFC because it conflicts with other pending qemu-img patches:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01756.html
and because I still have testsuite failures in qemu-iotests that
must be addressed before the series can be applied (some output
reordering occurs, but it also flushed out some clients that are
abusing qapi visits by expecting NULL to turn into "").  Posting
now to at least get feedback on whether the ideas make sense.

Pending prerequisites:
+ Markus' qapi-not-next branch (including my qapi subset D patches):
http://repo.or.cz/qemu/armbru.git/shortlog/refs/heads/qapi-not-next
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00463.html
+ v8 or later of my qapi subset E patches (at the time of this mail,
only v7 has been posted):
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01181.html

Also available as a tag at this location (with prerequisites applied):
git fetch git://repo.or.cz/qemu/ericb.git qapi-jsonv1

Eric Blake (11):
  qapi: Rename qjson.h to qobject-json.h
  qapi: Improve use of qmp/types.h
  qapi: Factor out JSON string escaping
  qapi: Factor out JSON number formatting
  qapi: Use qstring_append_chr() where appropriate
  qapi: Add qstring_append_format()
  qapi: add json output visitor
  qjson: Simplify by using json-output-visitor
  qapi: Add qobject_to_json_pretty_prefix()
  qapi: Support pretty printing in JSON output visitor
  RFC: qemu-img: Use new JSON output formatter

 MAINTAINERS                                   |   2 +-
 balloon.c                                     |   2 +-
 block.c                                       |   2 +-
 block/archipelago.c                           |   2 +-
 block/nbd.c                                   |   2 +-
 block/quorum.c                                |   2 +-
 blockjob.c                                    |   2 +-
 hw/core/qdev.c                                |   2 +-
 hw/misc/pvpanic.c                             |   2 +-
 hw/net/virtio-net.c                           |   2 +-
 hw/pci/pcie_aer.c                             |   1 +
 include/qapi/json-output-visitor.h            |  25 ++
 include/qapi/qmp/{qjson.h => qobject-json.h}  |   1 +
 include/qapi/qmp/qstring.h                    |  12 +-
 include/qapi/qmp/types.h                      |   1 -
 monitor.c                                     |   8 +-
 qapi/Makefile.objs                            |   2 +-
 qapi/json-output-visitor.c                    | 221 ++++++++++++
 qapi/qmp-dispatch.c                           |   1 +
 qapi/qmp-event.c                              |   2 +-
 qemu-img.c                                    |  70 ++--
 qga/main.c                                    |   2 +-
 qjson.c                                       |  64 ++--
 qobject/Makefile.objs                         |   3 +-
 qobject/json-parser.c                         |  14 +-
 qobject/{qjson.c => qobject-json.c}           | 147 ++------
 qobject/qobject.c                             |   7 +-
 qobject/qstring.c                             | 119 ++++++-
 target-s390x/kvm.c                            |   2 +-
 tests/.gitignore                              |   3 +-
 tests/Makefile                                |  12 +-
 tests/{check-qjson.c => check-qobject-json.c} |  86 ++++-
 tests/libqtest.c                              |   2 +-
 tests/test-json-output-visitor.c              | 474 ++++++++++++++++++++++++++
 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 +
 ui/spice-core.c                               |   2 +-
 vl.c                                          |   2 +-
 40 files changed, 1046 insertions(+), 261 deletions(-)
 create mode 100644 include/qapi/json-output-visitor.h
 rename include/qapi/qmp/{qjson.h => qobject-json.h} (90%)
 create mode 100644 qapi/json-output-visitor.c
 rename qobject/{qjson.c => qobject-json.c} (50%)
 rename tests/{check-qjson.c => check-qobject-json.c} (95%)
 create mode 100644 tests/test-json-output-visitor.c

-- 
2.4.3

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

* [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
@ 2015-12-10 23:53   ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 02/11] qapi: Improve use of qmp/types.h Eric Blake
                     ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Chrysostomos Nanakos, Jeff Cody, Paolo Bonzini,
	Alberto Garcia, Michael S. Tsirkin, Jason Wang, Luiz Capitulino,
	Markus Armbruster, Michael Roth, Christian Borntraeger,
	Cornelia Huck, Alexander Graf, Richard Henderson, Gerd Hoffmann,
	open list:Block layer core, open list:Overall

We have two different JSON visitors in the tree; and having both
named 'qjson.h' can cause include confusion.  Rename the qapi
version.

Kill trailing whitespace in the renamed tests/check-qobject-json.c
to keep checkpatch.pl happy.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 MAINTAINERS                                   |  2 +-
 balloon.c                                     |  2 +-
 block.c                                       |  2 +-
 block/archipelago.c                           |  2 +-
 block/nbd.c                                   |  2 +-
 block/quorum.c                                |  2 +-
 blockjob.c                                    |  2 +-
 hw/core/qdev.c                                |  2 +-
 hw/misc/pvpanic.c                             |  2 +-
 hw/net/virtio-net.c                           |  2 +-
 include/qapi/qmp/{qjson.h => qobject-json.h}  |  0
 include/qapi/qmp/types.h                      |  2 +-
 monitor.c                                     |  2 +-
 qapi/qmp-event.c                              |  2 +-
 qemu-img.c                                    |  2 +-
 qga/main.c                                    |  2 +-
 qobject/Makefile.objs                         |  3 ++-
 qobject/{qjson.c => qobject-json.c}           |  2 +-
 target-s390x/kvm.c                            |  2 +-
 tests/.gitignore                              |  2 +-
 tests/Makefile                                |  8 ++++----
 tests/{check-qjson.c => check-qobject-json.c} | 14 +++++++-------
 tests/libqtest.c                              |  2 +-
 ui/spice-core.c                               |  2 +-
 vl.c                                          |  2 +-
 25 files changed, 34 insertions(+), 33 deletions(-)
 rename include/qapi/qmp/{qjson.h => qobject-json.h} (100%)
 rename qobject/{qjson.c => qobject-json.c} (99%)
 rename tests/{check-qjson.c => check-qobject-json.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index e8cee1e..c943ff4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1155,7 +1155,7 @@ X: include/qapi/qmp/dispatch.h
 F: tests/check-qdict.c
 F: tests/check-qfloat.c
 F: tests/check-qint.c
-F: tests/check-qjson.c
+F: tests/check-qobject-json.c
 F: tests/check-qlist.c
 F: tests/check-qstring.c
 T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
diff --git a/balloon.c b/balloon.c
index 0f45d1b..5983b4f 100644
--- a/balloon.c
+++ b/balloon.c
@@ -31,7 +31,7 @@
 #include "trace.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
diff --git a/block.c b/block.c
index 9971976..e611002 100644
--- a/block.c
+++ b/block.c
@@ -29,7 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"
 #include "qemu/notify.h"
diff --git a/block/archipelago.c b/block/archipelago.c
index 855655c..80a1bb5 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -56,7 +56,7 @@
 #include "qemu/thread.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/atomic.h"

 #include <inttypes.h>
diff --git a/block/nbd.c b/block/nbd.c
index cd6a587..9009f4f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -32,7 +32,7 @@
 #include "qemu/module.h"
 #include "qemu/sockets.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"

diff --git a/block/quorum.c b/block/quorum.c
index d162459..1e3a278 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qint.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi-event.h"
diff --git a/blockjob.c b/blockjob.c
index 80adb9d..84361f7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -31,7 +31,7 @@
 #include "block/block_int.h"
 #include "sysemu/block-backend.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/coroutine.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b3ad467..a98304d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -31,7 +31,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/visitor.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/error-report.h"
 #include "hw/hotplug.h"
 #include "hw/boards.h"
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 3709488..2571483 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -13,7 +13,7 @@
  */

 #include "qapi/qmp/qobject.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "sysemu/sysemu.h"
 #include "qemu/log.h"

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a877614..5988bd1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -21,7 +21,7 @@
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
 #include "hw/virtio/virtio-bus.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi-event.h"
 #include "hw/virtio/virtio-access.h"

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qobject-json.h
similarity index 100%
rename from include/qapi/qmp/qjson.h
rename to include/qapi/qmp/qobject-json.h
diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
index 7782ec5..9109eda 100644
--- a/include/qapi/qmp/types.h
+++ b/include/qapi/qmp/types.h
@@ -20,6 +20,6 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #endif /* QEMU_OBJECTS_H */
diff --git a/monitor.c b/monitor.c
index e7e7ae2..1dfd359 100644
--- a/monitor.c
+++ b/monitor.c
@@ -55,7 +55,7 @@
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include <qom/object_interfaces.h>
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index c0e435f..800b1f3 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -16,7 +16,7 @@
 #include "qemu-common.h"
 #include "qapi/qmp-event.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 static QMPEventFuncEmit qmp_emit;

diff --git a/qemu-img.c b/qemu-img.c
index 033011c..cb961e6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,7 +24,7 @@
 #include "qapi-visit.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
diff --git a/qga/main.c b/qga/main.c
index f83a97d..3cc0901 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -24,7 +24,7 @@
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qint.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qga/guest-agent-core.h"
 #include "qemu/module.h"
 #include "signal.h"
diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs
index bed5508..16a48ec 100644
--- a/qobject/Makefile.objs
+++ b/qobject/Makefile.objs
@@ -1,2 +1,3 @@
 util-obj-y = qnull.o qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
-util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
+util-obj-y += qobject-json.o qobject.o
+util-obj-y += json-lexer.o json-streamer.o json-parser.o
diff --git a/qobject/qjson.c b/qobject/qobject-json.c
similarity index 99%
rename from qobject/qjson.c
rename to qobject/qobject-json.c
index 41d9d65..8fc65a4 100644
--- a/qobject/qjson.c
+++ b/qobject/qobject-json.c
@@ -14,7 +14,7 @@
 #include "qapi/qmp/json-lexer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qbool.h"
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 75a0e5d..40dd729 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -36,7 +36,7 @@
 #include "hw/hw.h"
 #include "cpu.h"
 #include "sysemu/device_tree.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "exec/gdbstub.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
diff --git a/tests/.gitignore b/tests/.gitignore
index 1e55722..6d3416d 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,8 +1,8 @@
 check-qdict
 check-qfloat
 check-qint
-check-qjson
 check-qlist
+check-qobject-json
 check-qstring
 check-qom-interface
 check-qom-proplist
diff --git a/tests/Makefile b/tests/Makefile
index 053c1ae..17d74e2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -16,8 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
 gcov-files-check-qstring-y = qobject/qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
 gcov-files-check-qlist-y = qobject/qlist.c
-check-unit-y += tests/check-qjson$(EXESUF)
-gcov-files-check-qjson-y = qobject/qjson.c
+check-unit-y += tests/check-qobject-json$(EXESUF)
+gcov-files-check-qobject-json-y = qobject/qobject-json.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-qmp-input-visitor$(EXESUF)
@@ -360,7 +360,7 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
 	tests/test-qmp-introspect.h

 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
-	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
+	tests/check-qlist.o tests/check-qfloat.o tests/check-qobject-json.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-qmp-input-visitor.o tests/test-qmp-input-strict.o \
@@ -387,7 +387,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
 tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
 tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
-tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
+tests/check-qobject-json$(EXESUF): tests/check-qobject-json.o $(test-util-obj-y)
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
 tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
 tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
diff --git a/tests/check-qjson.c b/tests/check-qobject-json.c
similarity index 99%
rename from tests/check-qjson.c
rename to tests/check-qobject-json.c
index 61e9bfb..9c4e53a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qobject-json.c
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qfloat.h"
 #include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #include "qemu-common.h"

@@ -63,7 +63,7 @@ static void escaped_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].decoded);

@@ -98,7 +98,7 @@ static void simple_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -106,7 +106,7 @@ static void simple_string(void)
         g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);

         qobject_decref(obj);
-        
+
         QDECREF(str);
     }
 }
@@ -132,7 +132,7 @@ static void single_quote_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -880,7 +880,7 @@ static void vararg_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -1144,7 +1144,7 @@ static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
         helper.index = 0;
         helper.objs = lhs->value.qlist;
         helper.result = 1;
-        
+
         qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);

         return helper.result;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index fa314e1..149eb4c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -32,7 +32,7 @@
 #include "qemu/osdep.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 5
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6a62d71..ceb18ae 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -32,7 +32,7 @@
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/notify.h"
 #include "migration/migration.h"
 #include "hw/hw.h"
diff --git a/vl.c b/vl.c
index ed1203e..eaf1d0c 100644
--- a/vl.c
+++ b/vl.c
@@ -92,7 +92,7 @@ int main(int argc, char **argv)
 #include "audio/audio.h"
 #include "migration/migration.h"
 #include "sysemu/kvm.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qemu-options.h"
-- 
2.4.3


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

* [Qemu-devel] [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h
@ 2015-12-10 23:53   ` Eric Blake
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Alexander Graf, Alberto Garcia, Michael Roth,
	Chrysostomos Nanakos, Michael S. Tsirkin, Jeff Cody,
	Markus Armbruster, Luiz Capitulino, Christian Borntraeger,
	Gerd Hoffmann, open list:Overall, Cornelia Huck, Paolo Bonzini,
	open list:Block layer core, Jason Wang, Richard Henderson

We have two different JSON visitors in the tree; and having both
named 'qjson.h' can cause include confusion.  Rename the qapi
version.

Kill trailing whitespace in the renamed tests/check-qobject-json.c
to keep checkpatch.pl happy.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 MAINTAINERS                                   |  2 +-
 balloon.c                                     |  2 +-
 block.c                                       |  2 +-
 block/archipelago.c                           |  2 +-
 block/nbd.c                                   |  2 +-
 block/quorum.c                                |  2 +-
 blockjob.c                                    |  2 +-
 hw/core/qdev.c                                |  2 +-
 hw/misc/pvpanic.c                             |  2 +-
 hw/net/virtio-net.c                           |  2 +-
 include/qapi/qmp/{qjson.h => qobject-json.h}  |  0
 include/qapi/qmp/types.h                      |  2 +-
 monitor.c                                     |  2 +-
 qapi/qmp-event.c                              |  2 +-
 qemu-img.c                                    |  2 +-
 qga/main.c                                    |  2 +-
 qobject/Makefile.objs                         |  3 ++-
 qobject/{qjson.c => qobject-json.c}           |  2 +-
 target-s390x/kvm.c                            |  2 +-
 tests/.gitignore                              |  2 +-
 tests/Makefile                                |  8 ++++----
 tests/{check-qjson.c => check-qobject-json.c} | 14 +++++++-------
 tests/libqtest.c                              |  2 +-
 ui/spice-core.c                               |  2 +-
 vl.c                                          |  2 +-
 25 files changed, 34 insertions(+), 33 deletions(-)
 rename include/qapi/qmp/{qjson.h => qobject-json.h} (100%)
 rename qobject/{qjson.c => qobject-json.c} (99%)
 rename tests/{check-qjson.c => check-qobject-json.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index e8cee1e..c943ff4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1155,7 +1155,7 @@ X: include/qapi/qmp/dispatch.h
 F: tests/check-qdict.c
 F: tests/check-qfloat.c
 F: tests/check-qint.c
-F: tests/check-qjson.c
+F: tests/check-qobject-json.c
 F: tests/check-qlist.c
 F: tests/check-qstring.c
 T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
diff --git a/balloon.c b/balloon.c
index 0f45d1b..5983b4f 100644
--- a/balloon.c
+++ b/balloon.c
@@ -31,7 +31,7 @@
 #include "trace.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
diff --git a/block.c b/block.c
index 9971976..e611002 100644
--- a/block.c
+++ b/block.c
@@ -29,7 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"
 #include "qemu/notify.h"
diff --git a/block/archipelago.c b/block/archipelago.c
index 855655c..80a1bb5 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -56,7 +56,7 @@
 #include "qemu/thread.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/atomic.h"

 #include <inttypes.h>
diff --git a/block/nbd.c b/block/nbd.c
index cd6a587..9009f4f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -32,7 +32,7 @@
 #include "qemu/module.h"
 #include "qemu/sockets.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"

diff --git a/block/quorum.c b/block/quorum.c
index d162459..1e3a278 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qint.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi-event.h"
diff --git a/blockjob.c b/blockjob.c
index 80adb9d..84361f7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -31,7 +31,7 @@
 #include "block/block_int.h"
 #include "sysemu/block-backend.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/coroutine.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b3ad467..a98304d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -31,7 +31,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/visitor.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/error-report.h"
 #include "hw/hotplug.h"
 #include "hw/boards.h"
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 3709488..2571483 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -13,7 +13,7 @@
  */

 #include "qapi/qmp/qobject.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "sysemu/sysemu.h"
 #include "qemu/log.h"

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a877614..5988bd1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -21,7 +21,7 @@
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
 #include "hw/virtio/virtio-bus.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi-event.h"
 #include "hw/virtio/virtio-access.h"

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qobject-json.h
similarity index 100%
rename from include/qapi/qmp/qjson.h
rename to include/qapi/qmp/qobject-json.h
diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
index 7782ec5..9109eda 100644
--- a/include/qapi/qmp/types.h
+++ b/include/qapi/qmp/types.h
@@ -20,6 +20,6 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #endif /* QEMU_OBJECTS_H */
diff --git a/monitor.c b/monitor.c
index e7e7ae2..1dfd359 100644
--- a/monitor.c
+++ b/monitor.c
@@ -55,7 +55,7 @@
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include <qom/object_interfaces.h>
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index c0e435f..800b1f3 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -16,7 +16,7 @@
 #include "qemu-common.h"
 #include "qapi/qmp-event.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 static QMPEventFuncEmit qmp_emit;

diff --git a/qemu-img.c b/qemu-img.c
index 033011c..cb961e6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,7 +24,7 @@
 #include "qapi-visit.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
diff --git a/qga/main.c b/qga/main.c
index f83a97d..3cc0901 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -24,7 +24,7 @@
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qint.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qga/guest-agent-core.h"
 #include "qemu/module.h"
 #include "signal.h"
diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs
index bed5508..16a48ec 100644
--- a/qobject/Makefile.objs
+++ b/qobject/Makefile.objs
@@ -1,2 +1,3 @@
 util-obj-y = qnull.o qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
-util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
+util-obj-y += qobject-json.o qobject.o
+util-obj-y += json-lexer.o json-streamer.o json-parser.o
diff --git a/qobject/qjson.c b/qobject/qobject-json.c
similarity index 99%
rename from qobject/qjson.c
rename to qobject/qobject-json.c
index 41d9d65..8fc65a4 100644
--- a/qobject/qjson.c
+++ b/qobject/qobject-json.c
@@ -14,7 +14,7 @@
 #include "qapi/qmp/json-lexer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qbool.h"
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 75a0e5d..40dd729 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -36,7 +36,7 @@
 #include "hw/hw.h"
 #include "cpu.h"
 #include "sysemu/device_tree.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "exec/gdbstub.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
diff --git a/tests/.gitignore b/tests/.gitignore
index 1e55722..6d3416d 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,8 +1,8 @@
 check-qdict
 check-qfloat
 check-qint
-check-qjson
 check-qlist
+check-qobject-json
 check-qstring
 check-qom-interface
 check-qom-proplist
diff --git a/tests/Makefile b/tests/Makefile
index 053c1ae..17d74e2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -16,8 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
 gcov-files-check-qstring-y = qobject/qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
 gcov-files-check-qlist-y = qobject/qlist.c
-check-unit-y += tests/check-qjson$(EXESUF)
-gcov-files-check-qjson-y = qobject/qjson.c
+check-unit-y += tests/check-qobject-json$(EXESUF)
+gcov-files-check-qobject-json-y = qobject/qobject-json.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-qmp-input-visitor$(EXESUF)
@@ -360,7 +360,7 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
 	tests/test-qmp-introspect.h

 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
-	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
+	tests/check-qlist.o tests/check-qfloat.o tests/check-qobject-json.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-qmp-input-visitor.o tests/test-qmp-input-strict.o \
@@ -387,7 +387,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
 tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
 tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
-tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
+tests/check-qobject-json$(EXESUF): tests/check-qobject-json.o $(test-util-obj-y)
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
 tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
 tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
diff --git a/tests/check-qjson.c b/tests/check-qobject-json.c
similarity index 99%
rename from tests/check-qjson.c
rename to tests/check-qobject-json.c
index 61e9bfb..9c4e53a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qobject-json.c
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qfloat.h"
 #include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #include "qemu-common.h"

@@ -63,7 +63,7 @@ static void escaped_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].decoded);

@@ -98,7 +98,7 @@ static void simple_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -106,7 +106,7 @@ static void simple_string(void)
         g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);

         qobject_decref(obj);
-        
+
         QDECREF(str);
     }
 }
@@ -132,7 +132,7 @@ static void single_quote_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -880,7 +880,7 @@ static void vararg_string(void)

         g_assert(obj != NULL);
         g_assert(qobject_type(obj) == QTYPE_QSTRING);
-        
+
         str = qobject_to_qstring(obj);
         g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);

@@ -1144,7 +1144,7 @@ static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
         helper.index = 0;
         helper.objs = lhs->value.qlist;
         helper.result = 1;
-        
+
         qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);

         return helper.result;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index fa314e1..149eb4c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -32,7 +32,7 @@
 #include "qemu/osdep.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"

 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 5
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6a62d71..ceb18ae 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -32,7 +32,7 @@
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/notify.h"
 #include "migration/migration.h"
 #include "hw/hw.h"
diff --git a/vl.c b/vl.c
index ed1203e..eaf1d0c 100644
--- a/vl.c
+++ b/vl.c
@@ -92,7 +92,7 @@ int main(int argc, char **argv)
 #include "audio/audio.h"
 #include "migration/migration.h"
 #include "sysemu/kvm.h"
-#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qemu-options.h"
-- 
2.4.3

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

* [Qemu-devel] [PATCH 02/11] qapi: Improve use of qmp/types.h
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
  2015-12-10 23:53   ` [Qemu-devel] " Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 03/11] qapi: Factor out JSON string escaping Eric Blake
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Roth, Luiz Capitulino, Markus Armbruster, Michael S. Tsirkin

'qobject-json.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>
---
 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/qobject-json.c             | 6 +-----
 qobject/qobject.c                  | 7 +------
 tests/check-qobject-json.c         | 7 +------
 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(+), 29 deletions(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 98d2c18..dd5588c 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -20,6 +20,7 @@

 #include "sysemu/sysemu.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qobject-json.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 9109eda..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/qobject-json.h"

 #endif /* QEMU_OBJECTS_H */
diff --git a/monitor.c b/monitor.c
index 1dfd359..ed6a548 100644
--- a/monitor.c
+++ b/monitor.c
@@ -50,12 +50,8 @@
 #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/qobject-json.h"
+#include "qapi/qmp/types.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
 #include <qom/object_interfaces.h>
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index f36933d..4b79bf4 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -14,6 +14,7 @@
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/dispatch.h"
 #include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi-types.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 6ab98a7..441c6e9 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -14,12 +14,7 @@
 #include <stdarg.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/qobject-json.c b/qobject/qobject-json.c
index 8fc65a4..cc96ff6 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -15,11 +15,7 @@
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/qobject-json.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"

 typedef struct JSONParsingState
 {
diff --git a/qobject/qobject.c b/qobject/qobject.c
index a3ef14e..0c468d8 100644
--- a/qobject/qobject.c
+++ b/qobject/qobject.c
@@ -8,12 +8,7 @@
  */

 #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-qobject-json.c b/tests/check-qobject-json.c
index 9c4e53a..46d4edf 100644
--- a/tests/check-qobject-json.c
+++ b/tests/check-qobject-json.c
@@ -12,12 +12,7 @@
  */
 #include <glib.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/qobject-json.h"

 #include "qemu-common.h"
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 9792277..dabeaf0 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/qobject-json.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 45d06f3..60deaa0 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/qobject-json.h"

 typedef struct TestInputVisitorData {
     QObject *obj;
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index cb64500..691d906 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -17,6 +17,7 @@
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qobject-json.h"

 typedef struct TestOutputVisitorData {
     QmpOutputVisitor *qov;
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 9f67f9e..8ea1f43 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -20,6 +20,7 @@
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "qapi/qmp/qobject-json.h"
 #include "qapi/qmp-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/string-input-visitor.h"
-- 
2.4.3

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

* [Qemu-devel] [PATCH 03/11] qapi: Factor out JSON string escaping
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
  2015-12-10 23:53   ` [Qemu-devel] " Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 02/11] qapi: Improve use of qmp/types.h Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 04/11] qapi: Factor out JSON number formatting Eric Blake
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luiz Capitulino

Pull out a new qstring_append_json_string() helper, so that all
JSON output producers can use the same output escaping rules.

While it appears that vmstate's use of the simpler qjson.c
formatter is not currently encountering any string that needs
escapes to be valid JSON, it is better to be safe than sorry.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 +
 qjson.c                    |  9 +++----
 qobject/qobject-json.c     | 58 ++-------------------------------------------
 qobject/qstring.c          | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 62 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index df7df55..1a938f6 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -31,6 +31,7 @@ const char *qstring_get_str(const QString *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
+void qstring_append_json_string(QString *qstring, const char *raw);
 QString *qobject_to_qstring(const QObject *obj);
 void qstring_destroy_obj(QObject *obj);

diff --git a/qjson.c b/qjson.c
index e478802..25afaff 100644
--- a/qjson.c
+++ b/qjson.c
@@ -36,9 +36,8 @@ static void json_emit_element(QJSON *json, const char *name)
     }

     if (name) {
-        qstring_append(json->str, "\"");
-        qstring_append(json->str, name);
-        qstring_append(json->str, "\" : ");
+        qstring_append_json_string(json->str, name);
+        qstring_append(json->str, " : ");
     }
 }

@@ -77,9 +76,7 @@ void json_prop_int(QJSON *json, const char *name, int64_t val)
 void json_prop_str(QJSON *json, const char *name, const char *str)
 {
     json_emit_element(json, name);
-    qstring_append_chr(json->str, '"');
-    qstring_append(json->str, str);
-    qstring_append_chr(json->str, '"');
+    qstring_append_json_string(json->str, str);
 }

 const char *qjson_get_str(QJSON *json)
diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
index cc96ff6..1557ec6 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -79,7 +79,6 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent);
 static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 {
     ToJsonIterState *s = opaque;
-    QString *qkey;
     int j;

     if (s->count) {
@@ -92,9 +91,7 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
             qstring_append(s->str, "    ");
     }

-    qkey = qstring_from_str(key);
-    to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
-    QDECREF(qkey);
+    qstring_append_json_string(s->str, key);

     qstring_append(s->str, ": ");
     to_json(obj, s->str, s->pretty, s->indent);
@@ -136,58 +133,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
     }
     case QTYPE_QSTRING: {
         QString *val = qobject_to_qstring(obj);
-        const char *ptr;
-        int cp;
-        char buf[16];
-        char *end;
-
-        ptr = qstring_get_str(val);
-        qstring_append(str, "\"");
-
-        for (; *ptr; ptr = end) {
-            cp = mod_utf8_codepoint(ptr, 6, &end);
-            switch (cp) {
-            case '\"':
-                qstring_append(str, "\\\"");
-                break;
-            case '\\':
-                qstring_append(str, "\\\\");
-                break;
-            case '\b':
-                qstring_append(str, "\\b");
-                break;
-            case '\f':
-                qstring_append(str, "\\f");
-                break;
-            case '\n':
-                qstring_append(str, "\\n");
-                break;
-            case '\r':
-                qstring_append(str, "\\r");
-                break;
-            case '\t':
-                qstring_append(str, "\\t");
-                break;
-            default:
-                if (cp < 0) {
-                    cp = 0xFFFD; /* replacement character */
-                }
-                if (cp > 0xFFFF) {
-                    /* beyond BMP; need a surrogate pair */
-                    snprintf(buf, sizeof(buf), "\\u%04X\\u%04X",
-                             0xD800 + ((cp - 0x10000) >> 10),
-                             0xDC00 + ((cp - 0x10000) & 0x3FF));
-                } else if (cp < 0x20 || cp >= 0x7F) {
-                    snprintf(buf, sizeof(buf), "\\u%04X", cp);
-                } else {
-                    buf[0] = cp;
-                    buf[1] = 0;
-                }
-                qstring_append(str, buf);
-            }
-        };
-
-        qstring_append(str, "\"");
+        qstring_append_json_string(str, qstring_get_str(val));
         break;
     }
     case QTYPE_QDICT: {
diff --git a/qobject/qstring.c b/qobject/qstring.c
index f44c5c4..2882c47 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -106,6 +106,65 @@ void qstring_append_chr(QString *qstring, int c)
 }

 /**
+ * qstring_append_json_string(): Append a raw string to a QString, while
+ * adding outer "" and JSON escape sequences.
+ */
+void qstring_append_json_string(QString *qstring, const char *raw)
+{
+    const char *ptr = raw;
+    int cp;
+    char buf[16];
+    char *end;
+
+    qstring_append(qstring, "\"");
+
+    for (; *ptr; ptr = end) {
+        cp = mod_utf8_codepoint(ptr, 6, &end);
+        switch (cp) {
+        case '\"':
+            qstring_append(qstring, "\\\"");
+            break;
+        case '\\':
+            qstring_append(qstring, "\\\\");
+            break;
+        case '\b':
+            qstring_append(qstring, "\\b");
+            break;
+        case '\f':
+            qstring_append(qstring, "\\f");
+            break;
+        case '\n':
+            qstring_append(qstring, "\\n");
+            break;
+        case '\r':
+            qstring_append(qstring, "\\r");
+            break;
+        case '\t':
+            qstring_append(qstring, "\\t");
+            break;
+        default:
+            if (cp < 0) {
+                cp = 0xFFFD; /* replacement character */
+            }
+            if (cp > 0xFFFF) {
+                /* beyond BMP; need a surrogate pair */
+                snprintf(buf, sizeof(buf), "\\u%04X\\u%04X",
+                         0xD800 + ((cp - 0x10000) >> 10),
+                         0xDC00 + ((cp - 0x10000) & 0x3FF));
+            } else if (cp < 0x20 || cp >= 0x7F) {
+                snprintf(buf, sizeof(buf), "\\u%04X", cp);
+            } else {
+                buf[0] = cp;
+                buf[1] = 0;
+            }
+            qstring_append(qstring, buf);
+        }
+    };
+
+    qstring_append(qstring, "\"");
+}
+
+/**
  * qobject_to_qstring(): Convert a QObject to a QString
  */
 QString *qobject_to_qstring(const QObject *obj)
-- 
2.4.3

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

* [Qemu-devel] [PATCH 04/11] qapi: Factor out JSON number formatting
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (2 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 03/11] qapi: Factor out JSON string escaping Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 05/11] qapi: Use qstring_append_chr() where appropriate Eric Blake
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luiz Capitulino

Pull out a new qstring_append_json_number() helper, so that all
JSON output producers can use a consistent style for printing
floating point without duplicating code (since we are doing more
data massaging than a simple printf format).

Address one FIXME by adding an Error parameter and warning the
caller if the requested number cannot be represented in JSON;
but add another FIXME in its place because we have no way to
report the problem higher up the stack.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/qmp/qstring.h |  4 +++-
 qobject/qobject-json.c     | 24 +++---------------------
 qobject/qstring.c          | 37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 1a938f6..cab1faf 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -1,7 +1,7 @@
 /*
  * QString Module
  *
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009, 2015 Red Hat Inc.
  *
  * Authors:
  *  Luiz Capitulino <lcapitulino@redhat.com>
@@ -15,6 +15,7 @@

 #include <stdint.h>
 #include "qapi/qmp/qobject.h"
+#include "qapi/error.h"

 typedef struct QString {
     QObject base;
@@ -32,6 +33,7 @@ void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
 void qstring_append_json_string(QString *qstring, const char *raw);
+void qstring_append_json_number(QString *qstring, double number, Error **errp);
 QString *qobject_to_qstring(const QObject *obj);
 void qstring_destroy_obj(QObject *obj);

diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
index 1557ec6..dbe0ab8 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -176,27 +176,9 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
     }
     case QTYPE_QFLOAT: {
         QFloat *val = qobject_to_qfloat(obj);
-        char buffer[1024];
-        int len;
-
-        /* FIXME: snprintf() is locale dependent; but JSON requires
-         * numbers to be formatted as if in the C locale. */
-        /* FIXME: This risks printing Inf or NaN, which are not valid
-         * JSON values. */
-        /* FIXME: the default precision of %f may be insufficient to
-         * tell this number apart from others. */
-        len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
-        while (len > 0 && buffer[len - 1] == '0') {
-            len--;
-        }
-
-        if (len && buffer[len - 1] == '.') {
-            buffer[len - 1] = 0;
-        } else {
-            buffer[len] = 0;
-        }
-
-        qstring_append(str, buffer);
+        /* FIXME: no way to report invalid JSON to caller, so for now
+         * we just ignore it */
+        qstring_append_json_number(str, qfloat_get_double(val), NULL);
         break;
     }
     case QTYPE_QBOOL: {
diff --git a/qobject/qstring.c b/qobject/qstring.c
index 2882c47..c83e63c 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -1,7 +1,7 @@
 /*
  * QString Module
  *
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009, 2015 Red Hat Inc.
  *
  * Authors:
  *  Luiz Capitulino <lcapitulino@redhat.com>
@@ -13,6 +13,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu-common.h"
+#include <math.h>

 /**
  * qstring_new(): Create a new empty QString
@@ -165,6 +166,40 @@ void qstring_append_json_string(QString *qstring, const char *raw)
 }

 /**
+ * qstring_append_json_number(): Append a JSON number to a QString.
+ * Set @errp if the number is not representable in JSON, but append the
+ * output anyway (callers can then choose to ignore the warning).
+ */
+void qstring_append_json_number(QString *qstring, double number, Error **errp)
+{
+    char buffer[1024];
+    int len;
+
+    /* JSON does not allow Inf or NaN; we append it anyways but set errp */
+    if (!isfinite(number)) {
+        error_setg(errp, "Non-finite number %f is not valid JSON", number);
+    }
+
+    /* FIXME: snprintf() is locale dependent; but JSON requires
+     * numbers to be formatted as if in the C locale. */
+    /* FIXME: the default precision of %f may be insufficient to
+     * tell this number apart from others. */
+    len = snprintf(buffer, sizeof(buffer), "%f", number);
+    assert(len > 0 && len < sizeof(buffer));
+    while (len > 0 && buffer[len - 1] == '0') {
+        len--;
+    }
+
+    if (len && buffer[len - 1] == '.') {
+        buffer[len - 1] = 0;
+    } else {
+        buffer[len] = 0;
+    }
+
+    qstring_append(qstring, buffer);
+}
+
+/**
  * qobject_to_qstring(): Convert a QObject to a QString
  */
 QString *qobject_to_qstring(const QObject *obj)
-- 
2.4.3

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

* [Qemu-devel] [PATCH 05/11] qapi: Use qstring_append_chr() where appropriate
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (3 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 04/11] qapi: Factor out JSON number formatting Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 06/11] qapi: Add qstring_append_format() Eric Blake
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luiz Capitulino

No need to create a temporary buffer, when we already have a
function available for our needs.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qobject/json-parser.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 441c6e9..fc5510e 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -204,12 +204,7 @@ static QString *qstring_from_escaped_str(JSONParserContext *ctxt,
                 goto out;
             }
         } else {
-            char dummy[2];
-
-            dummy[0] = *ptr++;
-            dummy[1] = 0;
-
-            qstring_append(str, dummy);
+            qstring_append_chr(str, *ptr++);
         }
     }

-- 
2.4.3

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

* [Qemu-devel] [PATCH 06/11] qapi: Add qstring_append_format()
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (4 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 05/11] qapi: Use qstring_append_chr() where appropriate Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 07/11] qapi: add json output visitor Eric Blake
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luiz Capitulino

Back in commit 764c1ca (Nov 2009), we added qstring_append_int().
However, it did not see any use until commit 190c882 (Jan 2015).
Furthermore, it has a rather limited use case - to print anything
else, callers still have to format into a temporary buffer, unless
we want to introduce an explosion of new qstring_append_* methods
for each useful type to print.

A much better approach is to add a wrapper that merges printf
behavior onto qstring_append, via the new qstring_append_format()
(and its vararg counterpart).  In fact, with that in place, we
no longer need qstring_append_int().

Other immediate uses for the new function include simplifying
an existing client of qstring_append() on a just-formatted
buffer, and the fact that we can take advantage of printf width
manipulations for more efficient indentation.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/qmp/qstring.h |  7 ++++++-
 qjson.c                    |  2 +-
 qobject/qobject-json.c     | 25 +++++--------------------
 qobject/qstring.c          | 23 +++++++++++++++++++----
 4 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index cab1faf..1b1e130 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -14,6 +14,8 @@
 #define QSTRING_H

 #include <stdint.h>
+#include <stdarg.h>
+#include "qemu/compiler.h"
 #include "qapi/qmp/qobject.h"
 #include "qapi/error.h"

@@ -29,9 +31,12 @@ QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, int start, int end);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
-void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
+void qstring_append_format(QString *qstring, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
+void qstring_append_vformat(QString *qstring, const char *fmt, va_list ap)
+    GCC_FMT_ATTR(2, 0);
 void qstring_append_json_string(QString *qstring, const char *raw);
 void qstring_append_json_number(QString *qstring, double number, Error **errp);
 QString *qobject_to_qstring(const QObject *obj);
diff --git a/qjson.c b/qjson.c
index 25afaff..8c93c1b 100644
--- a/qjson.c
+++ b/qjson.c
@@ -70,7 +70,7 @@ void json_end_array(QJSON *json)
 void json_prop_int(QJSON *json, const char *name, int64_t val)
 {
     json_emit_element(json, name);
-    qstring_append_int(json->str, val);
+    qstring_append_format(json->str, "%" PRId64, val);
 }

 void json_prop_str(QJSON *json, const char *name, const char *str)
diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
index dbe0ab8..3a7b26b 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -79,16 +79,13 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent);
 static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 {
     ToJsonIterState *s = opaque;
-    int j;

     if (s->count) {
         qstring_append(s->str, s->pretty ? "," : ", ");
     }

     if (s->pretty) {
-        qstring_append(s->str, "\n");
-        for (j = 0 ; j < s->indent ; j++)
-            qstring_append(s->str, "    ");
+        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
     }

     qstring_append_json_string(s->str, key);
@@ -101,16 +98,13 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 static void to_json_list_iter(QObject *obj, void *opaque)
 {
     ToJsonIterState *s = opaque;
-    int j;

     if (s->count) {
         qstring_append(s->str, s->pretty ? "," : ", ");
     }

     if (s->pretty) {
-        qstring_append(s->str, "\n");
-        for (j = 0 ; j < s->indent ; j++)
-            qstring_append(s->str, "    ");
+        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
     }

     to_json(obj, s->str, s->pretty, s->indent);
@@ -125,10 +119,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         break;
     case QTYPE_QINT: {
         QInt *val = qobject_to_qint(obj);
-        char buffer[1024];
-
-        snprintf(buffer, sizeof(buffer), "%" PRId64, qint_get_int(val));
-        qstring_append(str, buffer);
+        qstring_append_format(str, "%" PRId64, qint_get_int(val));
         break;
     }
     case QTYPE_QSTRING: {
@@ -147,10 +138,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         qstring_append(str, "{");
         qdict_iter(val, to_json_dict_iter, &s);
         if (pretty) {
-            int j;
-            qstring_append(str, "\n");
-            for (j = 0 ; j < indent ; j++)
-                qstring_append(str, "    ");
+            qstring_append_format(str, "\n%*s", 4 * indent, "");
         }
         qstring_append(str, "}");
         break;
@@ -166,10 +154,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         qstring_append(str, "[");
         qlist_iter(val, (void *)to_json_list_iter, &s);
         if (pretty) {
-            int j;
-            qstring_append(str, "\n");
-            for (j = 0 ; j < indent ; j++)
-                qstring_append(str, "    ");
+            qstring_append_format(str, "\n%*s", 4 * indent, "");
         }
         qstring_append(str, "]");
         break;
diff --git a/qobject/qstring.c b/qobject/qstring.c
index c83e63c..a348ae3 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -88,12 +88,27 @@ void qstring_append(QString *qstring, const char *str)
     qstring->string[qstring->length] = 0;
 }

-void qstring_append_int(QString *qstring, int64_t value)
+void qstring_append_format(QString *qstring, const char *fmt, ...)
 {
-    char num[32];
+    va_list ap;

-    snprintf(num, sizeof(num), "%" PRId64, value);
-    qstring_append(qstring, num);
+    va_start(ap, fmt);
+    qstring_append_vformat(qstring, fmt, ap);
+    va_end(ap);
+}
+
+void qstring_append_vformat(QString *qstring, const char *fmt, va_list ap)
+{
+    va_list ap2;
+    size_t len;
+
+    va_copy(ap2, ap);
+    len = vsnprintf(qstring->string + qstring->length, 0, fmt, ap2);
+    va_end(ap2);
+
+    capacity_increase(qstring, len);
+    vsnprintf(qstring->string + qstring->length, len + 1, fmt, ap);
+    qstring->length += len;
 }

 /**
-- 
2.4.3

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

* [Qemu-devel] [PATCH 07/11] qapi: add json output visitor
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (5 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 06/11] qapi: Add qstring_append_format() Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-11  0:24   ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor Eric Blake
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Michael Roth

We have several places that want to go from qapi to JSON; right now,
they have to create an intermediate QObject to do the work.  That
also has the drawback that the JSON formatting of a QDict will
rearrange keys (according to a deterministic, but unpredictable,
hash), when humans have an easier time if dicts are produced in
the same order as the qapi type.

For these reasons, it is time to add a new JSON output visitor.
This patch just adds the basic visitor and tests that it works;
later patches will add pretty-printing, and convert clients to
use the visitor.

Design choices: Unlike the QMP output visitor, the JSON visitor
refuses to visit a required string with a NULL value (abort), as
well as a non-finite number (raises an error message).  Reusing
QString to grow the contents means that we easily share code with
both qobject-json.c and qjson.c; although it might be nice to
enhance things to take an optional output callback function so
that the output can truly be streamed instead of collected in
memory.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/json-output-visitor.h |  25 +++
 qapi/Makefile.objs                 |   2 +-
 qapi/json-output-visitor.c         | 202 ++++++++++++++++++
 tests/.gitignore                   |   1 +
 tests/Makefile                     |   4 +
 tests/test-json-output-visitor.c   | 410 +++++++++++++++++++++++++++++++++++++
 6 files changed, 643 insertions(+), 1 deletion(-)
 create mode 100644 include/qapi/json-output-visitor.h
 create mode 100644 qapi/json-output-visitor.c
 create mode 100644 tests/test-json-output-visitor.c

diff --git a/include/qapi/json-output-visitor.h b/include/qapi/json-output-visitor.h
new file mode 100644
index 0000000..5be5a13
--- /dev/null
+++ b/include/qapi/json-output-visitor.h
@@ -0,0 +1,25 @@
+/*
+ * JSON Output Visitor
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef JSON_OUTPUT_VISITOR_H
+#define JSON_OUTPUT_VISITOR_H
+
+#include "qapi/visitor.h"
+
+typedef struct JsonOutputVisitor JsonOutputVisitor;
+
+JsonOutputVisitor *json_output_visitor_new(void);
+void json_output_visitor_cleanup(JsonOutputVisitor *v);
+void json_output_visitor_reset(JsonOutputVisitor *v);
+
+char *json_output_get_string(JsonOutputVisitor *v);
+Visitor *json_output_get_visitor(JsonOutputVisitor *v);
+
+#endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 2278970..b60e11b 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 json-output-visitor.o
 util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
new file mode 100644
index 0000000..0d759c4
--- /dev/null
+++ b/qapi/json-output-visitor.c
@@ -0,0 +1,202 @@
+/*
+ * Convert QAPI to JSON
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qapi/json-output-visitor.h"
+#include "qapi/visitor-impl.h"
+#include "qemu/queue.h"
+#include "qemu-common.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qobject-json.h"
+
+struct JsonOutputVisitor {
+    Visitor visitor;
+    QString *str;
+    bool comma;
+    int depth;
+};
+
+static JsonOutputVisitor *to_jov(Visitor *v)
+{
+    return container_of(v, JsonOutputVisitor, visitor);
+}
+
+static void json_output_name(JsonOutputVisitor *jov, const char *name)
+{
+    if (!jov->str) {
+        jov->str = qstring_new();
+    }
+    if (jov->comma) {
+        qstring_append(jov->str, ", ");
+    } else {
+        jov->comma = true;
+    }
+    if (name && jov->depth) {
+        qstring_append_json_string(jov->str, name);
+        qstring_append(jov->str, ": ");
+    }
+}
+
+static bool json_output_start_struct(Visitor *v, void **obj, size_t unused,
+                                     const char *name, Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append(jov->str, "{");
+    jov->comma = false;
+    jov->depth++;
+    return false;
+}
+
+static void json_output_end_struct(Visitor *v)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    assert(jov->depth);
+    jov->depth--;
+    qstring_append(jov->str, "}");
+    jov->comma = true;
+}
+
+static bool json_output_start_list(Visitor *v, GenericList **listp,
+                                   size_t size, const char *name, Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append(jov->str, "[");
+    jov->comma = false;
+    jov->depth++;
+    return false;
+}
+
+static GenericList *json_output_next_list(Visitor *v, GenericList *list,
+                                          size_t size, Error **errp)
+{
+    return list->next;
+}
+
+static void json_output_end_list(Visitor *v)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    assert(jov->depth);
+    jov->depth--;
+    qstring_append(jov->str, "]");
+    jov->comma = true;
+}
+
+static void json_output_type_int64(Visitor *v, int64_t *obj, const char *name,
+                                   Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append_format(jov->str, "%" PRId64, *obj);
+}
+
+static void json_output_type_uint64(Visitor *v, uint64_t *obj,
+                                    const char *name, Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append_format(jov->str, "%" PRIu64, *obj);
+}
+
+static void json_output_type_bool(Visitor *v, bool *obj, const char *name,
+                                  Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append(jov->str, *obj ? "true" : "false");
+}
+
+static void json_output_type_str(Visitor *v, char **obj, const char *name,
+                                 Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    assert(*obj);
+    json_output_name(jov, name);
+    qstring_append_json_string(jov->str, *obj);
+}
+
+static void json_output_type_number(Visitor *v, double *obj, const char *name,
+                                    Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append_json_number(jov->str, *obj, errp);
+}
+
+static void json_output_type_any(Visitor *v, QObject **obj, const char *name,
+                                 Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    QString *str = qobject_to_json(*obj);
+    assert(str);
+    json_output_name(jov, name);
+    qstring_append(jov->str, qstring_get_str(str));
+    QDECREF(str);
+}
+
+static void json_output_type_null(Visitor *v, const char *name, Error **errp)
+{
+    JsonOutputVisitor *jov = to_jov(v);
+    json_output_name(jov, name);
+    qstring_append(jov->str, "null");
+}
+
+/* Finish building, and return the resulting string. Will not be NULL. */
+char *json_output_get_string(JsonOutputVisitor *jov)
+{
+    char *result;
+
+    assert(!jov->depth);
+    result = g_strdup(qstring_get_str(jov->str));
+    json_output_visitor_reset(jov);
+    return result;
+}
+
+Visitor *json_output_get_visitor(JsonOutputVisitor *v)
+{
+    return &v->visitor;
+}
+
+void json_output_visitor_reset(JsonOutputVisitor *v)
+{
+    QDECREF(v->str);
+    v->str = NULL;
+    v->depth = 0;
+    v->comma = false;
+}
+
+void json_output_visitor_cleanup(JsonOutputVisitor *v)
+{
+    json_output_visitor_reset(v);
+    g_free(v);
+}
+
+JsonOutputVisitor *json_output_visitor_new(void)
+{
+    JsonOutputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.start_struct = json_output_start_struct;
+    v->visitor.end_struct = json_output_end_struct;
+    v->visitor.start_list = json_output_start_list;
+    v->visitor.next_list = json_output_next_list;
+    v->visitor.end_list = json_output_end_list;
+    v->visitor.type_enum = output_type_enum;
+    v->visitor.type_int64 = json_output_type_int64;
+    v->visitor.type_uint64 = json_output_type_uint64;
+    v->visitor.type_bool = json_output_type_bool;
+    v->visitor.type_str = json_output_type_str;
+    v->visitor.type_number = json_output_type_number;
+    v->visitor.type_any = json_output_type_any;
+    v->visitor.type_null = json_output_type_null;
+
+    return v;
+}
diff --git a/tests/.gitignore b/tests/.gitignore
index 6d3416d..b4f371c 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -24,6 +24,7 @@ test-cutils
 test-hbitmap
 test-int128
 test-iov
+test-json-output-visitor
 test-mul64
 test-opts-visitor
 test-qapi-event.[ch]
diff --git a/tests/Makefile b/tests/Makefile
index 17d74e2..f617e2d 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -20,6 +20,8 @@ check-unit-y += tests/check-qobject-json$(EXESUF)
 gcov-files-check-qobject-json-y = qobject/qobject-json.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-json-output-visitor$(EXESUF)
+gcov-files-test-json-output-visitor-y = qapi/json-output-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)
@@ -363,6 +365,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qfloat.o tests/check-qobject-json.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-json-output-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 \
@@ -447,6 +450,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-json-output-visitor$(EXESUF): tests/test-json-output-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-json-output-visitor.c b/tests/test-json-output-visitor.c
new file mode 100644
index 0000000..e47571a
--- /dev/null
+++ b/tests/test-json-output-visitor.c
@@ -0,0 +1,410 @@
+/*
+ * JSON Output Visitor unit-tests.
+ *
+ * Copyright (C) 2011, 2015 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 <glib.h>
+
+#include "qemu-common.h"
+#include "qapi/json-output-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qapi/qmp/types.h"
+
+typedef struct TestOutputVisitorData {
+    JsonOutputVisitor *jov;
+    Visitor *ov;
+} TestOutputVisitorData;
+
+static void visitor_output_setup(TestOutputVisitorData *data,
+                                 const void *unused)
+{
+    data->jov = json_output_visitor_new();
+    g_assert(data->jov);
+
+    data->ov = json_output_get_visitor(data->jov);
+    g_assert(data->ov);
+}
+
+static void visitor_output_teardown(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    json_output_visitor_cleanup(data->jov);
+    data->jov = NULL;
+    data->ov = NULL;
+}
+
+static void test_visitor_out_int(TestOutputVisitorData *data,
+                                 const void *unused)
+{
+    int64_t value = -42;
+    char *out;
+
+    visit_type_int(data->ov, &value, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "-42");
+    g_free(out);
+}
+
+static void test_visitor_out_bool(TestOutputVisitorData *data,
+                                  const void *unused)
+{
+    bool value = true;
+    char *out;
+
+    visit_type_bool(data->ov, &value, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "true");
+    g_free(out);
+}
+
+static void test_visitor_out_number(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    double value = 3.14;
+    char *out;
+
+    visit_type_number(data->ov, &value, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "3.14");
+    g_free(out);
+}
+
+static void test_visitor_out_string(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    char *string = (char *) "Q E M U";
+    char *out;
+
+    visit_type_str(data->ov, &string, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "\"Q E M U\"");
+    g_free(out);
+}
+
+static void test_visitor_out_enum(TestOutputVisitorData *data,
+                                  const void *unused)
+{
+    char *out;
+    char *tmp;
+    EnumOne i;
+    size_t len;
+
+    for (i = 0; i < ENUM_ONE__MAX; i++) {
+        visit_type_EnumOne(data->ov, &i, "unused", &error_abort);
+
+        out = json_output_get_string(data->jov);
+        g_assert(*out == '"');
+        len = strlen(out);
+        g_assert(out[len - 1] == '"');
+        tmp = out + 1;
+        out[len - 1] = 0;
+        g_assert_cmpstr(tmp, ==, EnumOne_lookup[i]);
+        g_free(out);
+    }
+}
+
+static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
+                                         const void *unused)
+{
+    EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
+    Error *err;
+
+    for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
+        err = NULL;
+        visit_type_EnumOne(data->ov, &bad_values[i], "unused", &err);
+        g_assert(err);
+        error_free(err);
+    }
+}
+
+
+static void test_visitor_out_struct(TestOutputVisitorData *data,
+                                    const void *unused)
+{
+    TestStruct test_struct = { .integer = 42,
+                               .boolean = false,
+                               .string = (char *) "foo"};
+    TestStruct *p = &test_struct;
+    char *out;
+
+    visit_type_TestStruct(data->ov, &p, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==,
+                    "{"
+                     "\"integer\": 42, "
+                     "\"boolean\": false, "
+                     "\"string\": \"foo\""
+                    "}");
+    g_free(out);
+}
+
+static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
+                                           const void *unused)
+{
+    int64_t value = 42;
+    UserDefTwo *ud2;
+    const char *string = "user def string";
+    const char *strings[] = { "forty two", "forty three", "forty four",
+                              "forty five" };
+    char *out;
+
+    ud2 = g_malloc0(sizeof(*ud2));
+    ud2->string0 = g_strdup(strings[0]);
+
+    ud2->dict1 = g_malloc0(sizeof(*ud2->dict1));
+    ud2->dict1->string1 = g_strdup(strings[1]);
+
+    ud2->dict1->dict2 = g_malloc0(sizeof(*ud2->dict1->dict2));
+    ud2->dict1->dict2->userdef = g_new0(UserDefOne, 1);
+    ud2->dict1->dict2->userdef->string = g_strdup(string);
+    ud2->dict1->dict2->userdef->integer = value;
+    ud2->dict1->dict2->string = g_strdup(strings[2]);
+
+    ud2->dict1->dict3 = g_malloc0(sizeof(*ud2->dict1->dict3));
+    ud2->dict1->has_dict3 = true;
+    ud2->dict1->dict3->userdef = g_new0(UserDefOne, 1);
+    ud2->dict1->dict3->userdef->string = g_strdup(string);
+    ud2->dict1->dict3->userdef->integer = value;
+    ud2->dict1->dict3->string = g_strdup(strings[3]);
+
+    visit_type_UserDefTwo(data->ov, &ud2, "unused", &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==,
+                    "{"
+                     "\"string0\": \"forty two\", "
+                     "\"dict1\": {"
+                      "\"string1\": \"forty three\", "
+                      "\"dict2\": {"
+                       "\"userdef\": {"
+                        "\"integer\": 42, "
+                        "\"string\": \"user def string\""
+                        "}, "
+                       "\"string\": \"forty four\""
+                       "}, "
+                      "\"dict3\": {"
+                       "\"userdef\": {"
+                        "\"integer\": 42, "
+                        "\"string\": \"user def string\""
+                        "}, "
+                      "\"string\": \"forty five\""
+                      "}"
+                     "}"
+                    "}");
+    qapi_free_UserDefTwo(ud2);
+    g_free(out);
+}
+
+static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
+                                           const void *unused)
+{
+    EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
+    UserDefOne u = { .string = (char *)"" };
+    UserDefOne *pu = &u;
+    Error *err;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
+        err = NULL;
+        u.has_enum1 = true;
+        u.enum1 = bad_values[i];
+        visit_type_UserDefOne(data->ov, &pu, "unused", &err);
+        g_assert(err);
+        error_free(err);
+        json_output_visitor_reset(data->jov);
+    }
+}
+
+
+static void test_visitor_out_list(TestOutputVisitorData *data,
+                                  const void *unused)
+{
+    const char *value_str = "list value";
+    TestStructList *p, *head = NULL;
+    const int max_items = 10;
+    bool value_bool = true;
+    int value_int = 10;
+    int i;
+    char *out;
+
+    for (i = 0; i < max_items; i++) {
+        p = g_malloc0(sizeof(*p));
+        p->value = g_malloc0(sizeof(*p->value));
+        p->value->integer = value_int + (max_items - i - 1);
+        p->value->boolean = value_bool;
+        p->value->string = g_strdup(value_str);
+
+        p->next = head;
+        head = p;
+    }
+
+    visit_type_TestStructList(data->ov, &head, NULL, &error_abort);
+
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==,
+                    "["
+                     "{\"integer\": 10, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 11, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 12, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 13, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 14, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 15, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 16, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 17, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 18, \"boolean\": true, "
+                      "\"string\": \"list value\"}, "
+                     "{\"integer\": 19, \"boolean\": true, "
+                      "\"string\": \"list value\"}"
+                    "]");
+    qapi_free_TestStructList(head);
+    g_free(out);
+}
+
+static void test_visitor_out_any(TestOutputVisitorData *data,
+                                 const void *unused)
+{
+    QObject *qobj;
+    QDict *qdict;
+    char *out;
+
+    qobj = QOBJECT(qint_from_int(-42));
+    visit_type_any(data->ov, &qobj, NULL, &error_abort);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "-42");
+    qobject_decref(qobj);
+    g_free(out);
+
+    qdict = qdict_new();
+    qdict_put(qdict, "integer", qint_from_int(-42));
+    qdict_put(qdict, "boolean", qbool_from_bool(true));
+    qdict_put(qdict, "string", qstring_from_str("foo"));
+    qobj = QOBJECT(qdict);
+    visit_type_any(data->ov, &qobj, NULL, &error_abort);
+    qobject_decref(qobj);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==,
+                    "{"
+                     "\"integer\": -42, "
+                     "\"boolean\": true, "
+                     "\"string\": \"foo\""
+                    "}");
+    g_free(out);
+}
+
+static void test_visitor_out_union_flat(TestOutputVisitorData *data,
+                                        const void *unused)
+{
+    char *out;
+    UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
+
+    tmp->enum1 = ENUM_ONE_VALUE1;
+    tmp->string = g_strdup("str");
+    tmp->u.value1 = g_malloc0(sizeof(UserDefA));
+    tmp->integer = 41;
+    tmp->u.value1->boolean = true;
+
+    visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &error_abort);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==,
+                    "{"
+                     "\"integer\": 41, "
+                     "\"string\": \"str\", "
+                     "\"enum1\": \"value1\", "
+                     "\"boolean\": true"
+                    "}");
+    g_free(out);
+    qapi_free_UserDefFlatUnion(tmp);
+}
+
+static void test_visitor_out_alternate(TestOutputVisitorData *data,
+                                       const void *unused)
+{
+    UserDefAlternate *tmp;
+    char *out;
+
+    tmp = g_new0(UserDefAlternate, 1);
+    tmp->type = QTYPE_QINT;
+    tmp->u.i = 42;
+
+    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "42");
+    g_free(out);
+    qapi_free_UserDefAlternate(tmp);
+
+    tmp = g_new0(UserDefAlternate, 1);
+    tmp->type = QTYPE_QSTRING;
+    tmp->u.s = g_strdup("hello");
+
+    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "\"hello\"");
+    g_free(out);
+    qapi_free_UserDefAlternate(tmp);
+}
+
+static void test_visitor_out_empty(TestOutputVisitorData *data,
+                                   const void *unused)
+{
+    char *out;
+
+    visit_type_null(data->ov, NULL, &error_abort);
+    out = json_output_get_string(data->jov);
+    g_assert_cmpstr(out, ==, "null");
+    g_free(out);
+}
+
+static void output_visitor_test_add(const char *testpath,
+                                    void (*test_func)(TestOutputVisitorData *,
+                                                      const void *))
+{
+    g_test_add(testpath, TestOutputVisitorData, NULL, visitor_output_setup,
+               test_func, visitor_output_teardown);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    output_visitor_test_add("/visitor/json/int", test_visitor_out_int);
+    output_visitor_test_add("/visitor/json/bool", test_visitor_out_bool);
+    output_visitor_test_add("/visitor/json/number", test_visitor_out_number);
+    output_visitor_test_add("/visitor/json/string", test_visitor_out_string);
+    output_visitor_test_add("/visitor/json/enum", test_visitor_out_enum);
+    output_visitor_test_add("/visitor/json/enum-errors",
+                            test_visitor_out_enum_errors);
+    output_visitor_test_add("/visitor/json/struct", test_visitor_out_struct);
+    output_visitor_test_add("/visitor/json/struct-nested",
+                            test_visitor_out_struct_nested);
+    output_visitor_test_add("/visitor/json/struct-errors",
+                            test_visitor_out_struct_errors);
+    output_visitor_test_add("/visitor/json/list", test_visitor_out_list);
+    output_visitor_test_add("/visitor/json/any", test_visitor_out_any);
+    output_visitor_test_add("/visitor/json/union-flat",
+                            test_visitor_out_union_flat);
+    output_visitor_test_add("/visitor/json/alternate",
+                            test_visitor_out_alternate);
+    output_visitor_test_add("/visitor/json/empty", test_visitor_out_empty);
+
+    g_test_run();
+
+    return 0;
+}
-- 
2.4.3

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

* [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (6 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 07/11] qapi: add json output visitor Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-11 11:10   ` Paolo Bonzini
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 09/11] qapi: Add qobject_to_json_pretty_prefix() Eric Blake
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel

Instead of rolling our own limited JSON outputter, we can just
wrap the more full-featured JSON output Visitor.

This slightly changes the output (different spacing), but the
result is still equivalent JSON contents.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qjson.c | 61 ++++++++++++++++++++++---------------------------------------
 1 file changed, 22 insertions(+), 39 deletions(-)

diff --git a/qjson.c b/qjson.c
index 8c93c1b..6aac46d 100644
--- a/qjson.c
+++ b/qjson.c
@@ -11,103 +11,86 @@
  *
  */

-#include <qapi/qmp/qstring.h>
 #include <stdbool.h>
-#include <glib.h>
+#include <stdint.h>
 #include <qjson.h>
 #include <qemu/module.h>
 #include <qom/object.h>
+#include "qapi/json-output-visitor.h"

 struct QJSON {
     Object obj;
-    QString *str;
-    bool omit_comma;
+    JsonOutputVisitor *jov;
+    char *str;
 };

 #define QJSON(obj) OBJECT_CHECK(QJSON, (obj), TYPE_QJSON)

-static void json_emit_element(QJSON *json, const char *name)
-{
-    /* Check whether we need to print a , before an element */
-    if (json->omit_comma) {
-        json->omit_comma = false;
-    } else {
-        qstring_append(json->str, ", ");
-    }
-
-    if (name) {
-        qstring_append_json_string(json->str, name);
-        qstring_append(json->str, " : ");
-    }
-}
-
 void json_start_object(QJSON *json, const char *name)
 {
-    json_emit_element(json, name);
-    qstring_append(json->str, "{ ");
-    json->omit_comma = true;
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_start_struct(v, NULL, 0, name, &error_abort);
 }

 void json_end_object(QJSON *json)
 {
-    qstring_append(json->str, " }");
-    json->omit_comma = false;
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_check_struct(v, &error_abort);
+    visit_end_struct(v);
 }

 void json_start_array(QJSON *json, const char *name)
 {
-    json_emit_element(json, name);
-    qstring_append(json->str, "[ ");
-    json->omit_comma = true;
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_start_list(v, NULL, 0, name, &error_abort);
 }

 void json_end_array(QJSON *json)
 {
-    qstring_append(json->str, " ]");
-    json->omit_comma = false;
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_end_list(v);
 }

 void json_prop_int(QJSON *json, const char *name, int64_t val)
 {
-    json_emit_element(json, name);
-    qstring_append_format(json->str, "%" PRId64, val);
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_type_int(v, &val, name, &error_abort);
 }

 void json_prop_str(QJSON *json, const char *name, const char *str)
 {
-    json_emit_element(json, name);
-    qstring_append_json_string(json->str, str);
+    Visitor *v = json_output_get_visitor(json->jov);
+    visit_type_str(v, (char **)&str, name, &error_abort);
 }

 const char *qjson_get_str(QJSON *json)
 {
-    return qstring_get_str(json->str);
+    return json->str;
 }

 QJSON *qjson_new(void)
 {
     QJSON *json = QJSON(object_new(TYPE_QJSON));
+    json_start_object(json, NULL);
     return json;
 }

 void qjson_finish(QJSON *json)
 {
     json_end_object(json);
+    json->str = json_output_get_string(json->jov);
 }

 static void qjson_initfn(Object *obj)
 {
     QJSON *json = QJSON(obj);
-
-    json->str = qstring_from_str("{ ");
-    json->omit_comma = true;
+    json->jov = json_output_visitor_new();
 }

 static void qjson_finalizefn(Object *obj)
 {
     QJSON *json = QJSON(obj);
-
-    qobject_decref(QOBJECT(json->str));
+    g_free(json->str);
 }

 static const TypeInfo qjson_type_info = {
-- 
2.4.3

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

* [Qemu-devel] [PATCH 09/11] qapi: Add qobject_to_json_pretty_prefix()
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (7 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 10/11] qapi: Support pretty printing in JSON output visitor Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter Eric Blake
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luiz Capitulino

The next patch will add pretty indentation to the JSON visitor.
But in order to support pretty output in the type_any() callback,
we need to prefix every line of the QObject visitor by the current
indentation in the JSON visitor.  Hence, a new function
qobject_to_json_pretty_indent(), and the old function becomes a
thin wrapper to the expanded behavior.

While at it, change 'pretty' to be a bool, to match its usage.

Note that the new simple_pretty() test is a bit sensitive to our
current notion of prettiness, as well as to the hash ordering in
QDict (most of the tests in check-qobject-json intentionally do
not compare the original string to the round-trip string, because
we liberally accept more input forms than the canonical form that
we output).

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/qmp/qobject-json.h |  1 +
 qobject/qobject-json.c          | 40 ++++++++++++++++++-------
 tests/check-qobject-json.c      | 65 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 11 deletions(-)

diff --git a/include/qapi/qmp/qobject-json.h b/include/qapi/qmp/qobject-json.h
index ee4d31a..120266b 100644
--- a/include/qapi/qmp/qobject-json.h
+++ b/include/qapi/qmp/qobject-json.h
@@ -25,5 +25,6 @@ QObject *qobject_from_jsonv(const char *string, va_list *ap) GCC_FMT_ATTR(1, 0);

 QString *qobject_to_json(const QObject *obj);
 QString *qobject_to_json_pretty(const QObject *obj);
+QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix);

 #endif /* QJSON_H */
diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
index 3a7b26b..6d0030e 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -69,12 +69,14 @@ QObject *qobject_from_jsonf(const char *string, ...)
 typedef struct ToJsonIterState
 {
     int indent;
-    int pretty;
+    bool pretty;
     int count;
     QString *str;
+    const char *prefix;
 } ToJsonIterState;

-static void to_json(const QObject *obj, QString *str, int pretty, int indent);
+static void to_json(const QObject *obj, QString *str, bool pretty, int indent,
+                    const char *prefix);

 static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 {
@@ -85,13 +87,13 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
     }

     if (s->pretty) {
-        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
+        qstring_append_format(s->str, "\n%s%*s", s->prefix, 4 * s->indent, "");
     }

     qstring_append_json_string(s->str, key);

     qstring_append(s->str, ": ");
-    to_json(obj, s->str, s->pretty, s->indent);
+    to_json(obj, s->str, s->pretty, s->indent, s->prefix);
     s->count++;
 }

@@ -104,14 +106,15 @@ static void to_json_list_iter(QObject *obj, void *opaque)
     }

     if (s->pretty) {
-        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
+        qstring_append_format(s->str, "\n%s%*s", s->prefix, 4 * s->indent, "");
     }

-    to_json(obj, s->str, s->pretty, s->indent);
+    to_json(obj, s->str, s->pretty, s->indent, s->prefix);
     s->count++;
 }

-static void to_json(const QObject *obj, QString *str, int pretty, int indent)
+static void to_json(const QObject *obj, QString *str, bool pretty, int indent,
+                    const char *prefix)
 {
     switch (qobject_type(obj)) {
     case QTYPE_QNULL:
@@ -135,10 +138,11 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.str = str;
         s.indent = indent + 1;
         s.pretty = pretty;
+        s.prefix = prefix;
         qstring_append(str, "{");
         qdict_iter(val, to_json_dict_iter, &s);
         if (pretty) {
-            qstring_append_format(str, "\n%*s", 4 * indent, "");
+            qstring_append_format(str, "\n%s%*s", prefix, 4 * indent, "");
         }
         qstring_append(str, "}");
         break;
@@ -151,10 +155,11 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.str = str;
         s.indent = indent + 1;
         s.pretty = pretty;
+        s.prefix = prefix;
         qstring_append(str, "[");
         qlist_iter(val, (void *)to_json_list_iter, &s);
         if (pretty) {
-            qstring_append_format(str, "\n%*s", 4 * indent, "");
+            qstring_append_format(str, "\n%s%*s", prefix, 4 * indent, "");
         }
         qstring_append(str, "]");
         break;
@@ -185,7 +190,7 @@ QString *qobject_to_json(const QObject *obj)
 {
     QString *str = qstring_new();

-    to_json(obj, str, 0, 0);
+    to_json(obj, str, false, 0, "");

     return str;
 }
@@ -194,7 +199,20 @@ QString *qobject_to_json_pretty(const QObject *obj)
 {
     QString *str = qstring_new();

-    to_json(obj, str, 1, 0);
+    to_json(obj, str, true, 0, "");
+
+    return str;
+}
+
+/*
+ * Pretty-print JSON representing obj, inserting prefix after every newline.
+ * Note that prefix is NOT applied before the first character.
+ */
+QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix)
+{
+    QString *str = qstring_new();
+
+    to_json(obj, str, true, 0, prefix);

     return str;
 }
diff --git a/tests/check-qobject-json.c b/tests/check-qobject-json.c
index 46d4edf..b07ac4d 100644
--- a/tests/check-qobject-json.c
+++ b/tests/check-qobject-json.c
@@ -1387,6 +1387,70 @@ static void simple_whitespace(void)
     }
 }

+static void simple_pretty(void)
+{
+    int i;
+    struct {
+        const char *encoded;
+        LiteralQObject decoded;
+    } test_cases[] = {
+        {
+            /* The first line is intentionally not prefixed. */
+            .encoded =
+            "[\n"
+            "        43,\n"
+            "        42\n"
+            "    ]",
+            .decoded = QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QINT(43),
+                        QLIT_QINT(42),
+                        { }
+                    })),
+        },
+        {
+            .encoded =
+            "[\n"
+            "        43,\n"
+            "        {\n"
+            "            \"a\": 32,\n"
+            "            \"h\": \"b\"\n"
+            "        },\n"
+            "        [\n"
+            "        ],\n"
+            "        42\n"
+            "    ]",
+            .decoded = QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QINT(43),
+                        QLIT_QDICT(((LiteralQDictEntry[]){
+                                    { "a", QLIT_QINT(32) },
+                                    { "h", QLIT_QSTR("b") },
+                                    { } })),
+                        QLIT_QLIST(((LiteralQObject[]){
+                                    { } })),
+                        QLIT_QINT(42),
+                        { }
+                    })),
+        },
+        { }
+    };
+
+    for (i = 0; test_cases[i].encoded; i++) {
+        QObject *obj;
+        QString *str;
+
+        obj = qobject_from_json(test_cases[i].encoded);
+        g_assert(obj != NULL);
+        g_assert(qobject_type(obj) == QTYPE_QLIST);
+
+        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+
+        str = qobject_to_json_pretty_prefix(obj, "    ");
+        qobject_decref(obj);
+        g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].encoded);
+        QDECREF(str);
+    }
+}
+
 static void simple_varargs(void)
 {
     QObject *embedded_obj;
@@ -1524,6 +1588,7 @@ int main(int argc, char **argv)
     g_test_add_func("/lists/simple_list", simple_list);

     g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
+    g_test_add_func("/whitespace/pretty", simple_pretty);

     g_test_add_func("/varargs/simple_varargs", simple_varargs);

-- 
2.4.3

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

* [Qemu-devel] [PATCH 10/11] qapi: Support pretty printing in JSON output visitor
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (8 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 09/11] qapi: Add qobject_to_json_pretty_prefix() Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter Eric Blake
  10 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Michael Roth

Similar to pretty printing in the QObject visitor.  Trickiest
parts are the fact that during type_any(), we have to coordinate
with QObject to also print pretty; and the fact that the testsuite
now has to honor parameterization on whether pretty printing is
enabled.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/json-output-visitor.h |   2 +-
 qapi/json-output-visitor.c         |  25 +++++-
 qjson.c                            |   2 +-
 tests/test-json-output-visitor.c   | 160 ++++++++++++++++++++++++++-----------
 4 files changed, 136 insertions(+), 53 deletions(-)

diff --git a/include/qapi/json-output-visitor.h b/include/qapi/json-output-visitor.h
index 5be5a13..e5e16e6 100644
--- a/include/qapi/json-output-visitor.h
+++ b/include/qapi/json-output-visitor.h
@@ -15,7 +15,7 @@

 typedef struct JsonOutputVisitor JsonOutputVisitor;

-JsonOutputVisitor *json_output_visitor_new(void);
+JsonOutputVisitor *json_output_visitor_new(bool pretty);
 void json_output_visitor_cleanup(JsonOutputVisitor *v);
 void json_output_visitor_reset(JsonOutputVisitor *v);

diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
index 0d759c4..a939af1 100644
--- a/qapi/json-output-visitor.c
+++ b/qapi/json-output-visitor.c
@@ -18,6 +18,7 @@
 struct JsonOutputVisitor {
     Visitor visitor;
     QString *str;
+    bool pretty;
     bool comma;
     int depth;
 };
@@ -33,10 +34,13 @@ static void json_output_name(JsonOutputVisitor *jov, const char *name)
         jov->str = qstring_new();
     }
     if (jov->comma) {
-        qstring_append(jov->str, ", ");
+        qstring_append(jov->str, jov->pretty ? "," : ", ");
     } else {
         jov->comma = true;
     }
+    if (jov->pretty && jov->depth) {
+        qstring_append_format(jov->str, "\n%*s", 4 * jov->depth, "");
+    }
     if (name && jov->depth) {
         qstring_append_json_string(jov->str, name);
         qstring_append(jov->str, ": ");
@@ -59,6 +63,9 @@ static void json_output_end_struct(Visitor *v)
     JsonOutputVisitor *jov = to_jov(v);
     assert(jov->depth);
     jov->depth--;
+    if (jov->pretty) {
+        qstring_append_format(jov->str, "\n%*s", 4 * jov->depth, "");
+    }
     qstring_append(jov->str, "}");
     jov->comma = true;
 }
@@ -85,6 +92,9 @@ static void json_output_end_list(Visitor *v)
     JsonOutputVisitor *jov = to_jov(v);
     assert(jov->depth);
     jov->depth--;
+    if (jov->pretty) {
+        qstring_append_format(jov->str, "\n%*s", 4 * jov->depth, "");
+    }
     qstring_append(jov->str, "]");
     jov->comma = true;
 }
@@ -134,7 +144,15 @@ static void json_output_type_any(Visitor *v, QObject **obj, const char *name,
                                  Error **errp)
 {
     JsonOutputVisitor *jov = to_jov(v);
-    QString *str = qobject_to_json(*obj);
+    QString *str;
+
+    if (jov->pretty) {
+        char *prefix = g_strdup_printf("%*s", 4 * jov->depth, "");
+        str = qobject_to_json_pretty_prefix(*obj, prefix);
+        g_free(prefix);
+    } else {
+        str = qobject_to_json(*obj);
+    }
     assert(str);
     json_output_name(jov, name);
     qstring_append(jov->str, qstring_get_str(str));
@@ -178,11 +196,12 @@ void json_output_visitor_cleanup(JsonOutputVisitor *v)
     g_free(v);
 }

-JsonOutputVisitor *json_output_visitor_new(void)
+JsonOutputVisitor *json_output_visitor_new(bool pretty)
 {
     JsonOutputVisitor *v;

     v = g_malloc0(sizeof(*v));
+    v->pretty = pretty;

     v->visitor.start_struct = json_output_start_struct;
     v->visitor.end_struct = json_output_end_struct;
diff --git a/qjson.c b/qjson.c
index 6aac46d..9debf1d 100644
--- a/qjson.c
+++ b/qjson.c
@@ -84,7 +84,7 @@ void qjson_finish(QJSON *json)
 static void qjson_initfn(Object *obj)
 {
     QJSON *json = QJSON(obj);
-    json->jov = json_output_visitor_new();
+    json->jov = json_output_visitor_new(false);
 }

 static void qjson_finalizefn(Object *obj)
diff --git a/tests/test-json-output-visitor.c b/tests/test-json-output-visitor.c
index e47571a..bfa21f7 100644
--- a/tests/test-json-output-visitor.c
+++ b/tests/test-json-output-visitor.c
@@ -21,9 +21,10 @@ typedef struct TestOutputVisitorData {
 } TestOutputVisitorData;

 static void visitor_output_setup(TestOutputVisitorData *data,
-                                 const void *unused)
+                                 const void *arg)
 {
-    data->jov = json_output_visitor_new();
+    const bool *pretty = arg;
+    data->jov = json_output_visitor_new(*pretty);
     g_assert(data->jov);

     data->ov = json_output_get_visitor(data->jov);
@@ -149,8 +150,9 @@ static void test_visitor_out_struct(TestOutputVisitorData *data,
 }

 static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
-                                           const void *unused)
+                                           const void *arg)
 {
+    const bool *pretty = arg;
     int64_t value = 42;
     UserDefTwo *ud2;
     const char *string = "user def string";
@@ -180,27 +182,51 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
     visit_type_UserDefTwo(data->ov, &ud2, "unused", &error_abort);

     out = json_output_get_string(data->jov);
-    g_assert_cmpstr(out, ==,
-                    "{"
-                     "\"string0\": \"forty two\", "
-                     "\"dict1\": {"
-                      "\"string1\": \"forty three\", "
-                      "\"dict2\": {"
-                       "\"userdef\": {"
-                        "\"integer\": 42, "
-                        "\"string\": \"user def string\""
-                        "}, "
-                       "\"string\": \"forty four\""
-                       "}, "
-                      "\"dict3\": {"
-                       "\"userdef\": {"
-                        "\"integer\": 42, "
-                        "\"string\": \"user def string\""
-                        "}, "
-                      "\"string\": \"forty five\""
-                      "}"
-                     "}"
-                    "}");
+    if (*pretty) {
+        g_assert_cmpstr(out, ==,
+                        "{\n"
+                        "    \"string0\": \"forty two\",\n"
+                        "    \"dict1\": {\n"
+                        "        \"string1\": \"forty three\",\n"
+                        "        \"dict2\": {\n"
+                        "            \"userdef\": {\n"
+                        "                \"integer\": 42,\n"
+                        "                \"string\": \"user def string\"\n"
+                        "            },\n"
+                        "            \"string\": \"forty four\"\n"
+                        "        },\n"
+                        "        \"dict3\": {\n"
+                        "            \"userdef\": {\n"
+                        "                \"integer\": 42,\n"
+                        "                \"string\": \"user def string\"\n"
+                        "            },\n"
+                        "            \"string\": \"forty five\"\n"
+                        "        }\n"
+                        "    }\n"
+                        "}");
+    } else {
+        g_assert_cmpstr(out, ==,
+                        "{"
+                         "\"string0\": \"forty two\", "
+                         "\"dict1\": {"
+                          "\"string1\": \"forty three\", "
+                          "\"dict2\": {"
+                           "\"userdef\": {"
+                            "\"integer\": 42, "
+                            "\"string\": \"user def string\""
+                            "}, "
+                           "\"string\": \"forty four\""
+                           "}, "
+                          "\"dict3\": {"
+                           "\"userdef\": {"
+                            "\"integer\": 42, "
+                            "\"string\": \"user def string\""
+                            "}, "
+                          "\"string\": \"forty five\""
+                          "}"
+                         "}"
+                        "}");
+    }
     qapi_free_UserDefTwo(ud2);
     g_free(out);
 }
@@ -279,16 +305,23 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
 }

 static void test_visitor_out_any(TestOutputVisitorData *data,
-                                 const void *unused)
+                                 const void *arg)
 {
+    const bool *pretty = arg;
     QObject *qobj;
     QDict *qdict;
     char *out;

     qobj = QOBJECT(qint_from_int(-42));
+    visit_start_list(data->ov, NULL, NULL, 0, &error_abort);
     visit_type_any(data->ov, &qobj, NULL, &error_abort);
+    visit_end_list(data->ov);
     out = json_output_get_string(data->jov);
-    g_assert_cmpstr(out, ==, "-42");
+    if (*pretty) {
+        g_assert_cmpstr(out, ==, "[\n    -42\n]");
+    } else {
+        g_assert_cmpstr(out, ==, "[-42]");
+    }
     qobject_decref(qobj);
     g_free(out);

@@ -297,15 +330,30 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qdict_put(qdict, "boolean", qbool_from_bool(true));
     qdict_put(qdict, "string", qstring_from_str("foo"));
     qobj = QOBJECT(qdict);
+    visit_start_list(data->ov, NULL, NULL, 0, &error_abort);
     visit_type_any(data->ov, &qobj, NULL, &error_abort);
+    visit_end_list(data->ov);
     qobject_decref(qobj);
     out = json_output_get_string(data->jov);
-    g_assert_cmpstr(out, ==,
-                    "{"
-                     "\"integer\": -42, "
-                     "\"boolean\": true, "
-                     "\"string\": \"foo\""
-                    "}");
+    if (*pretty) {
+        g_assert_cmpstr(out, ==,
+                        "[\n"
+                        "    {\n"
+                        "        \"integer\": -42,\n"
+                        "        \"boolean\": true,\n"
+                        "        \"string\": \"foo\"\n"
+                        "    }\n"
+                        "]");
+    } else {
+        g_assert_cmpstr(out, ==,
+                        "["
+                         "{"
+                          "\"integer\": -42, "
+                          "\"boolean\": true, "
+                          "\"string\": \"foo\""
+                         "}"
+                        "]");
+    }
     g_free(out);
 }

@@ -372,37 +420,53 @@ static void test_visitor_out_empty(TestOutputVisitorData *data,
     g_free(out);
 }

-static void output_visitor_test_add(const char *testpath,
+static void output_visitor_test_add(const char *testpath, bool *data,
                                     void (*test_func)(TestOutputVisitorData *,
                                                       const void *))
 {
-    g_test_add(testpath, TestOutputVisitorData, NULL, visitor_output_setup,
+    g_test_add(testpath, TestOutputVisitorData, data, visitor_output_setup,
                test_func, visitor_output_teardown);
 }

 int main(int argc, char **argv)
 {
+    bool plain = false;
+    bool pretty = true;
+
     g_test_init(&argc, &argv, NULL);

-    output_visitor_test_add("/visitor/json/int", test_visitor_out_int);
-    output_visitor_test_add("/visitor/json/bool", test_visitor_out_bool);
-    output_visitor_test_add("/visitor/json/number", test_visitor_out_number);
-    output_visitor_test_add("/visitor/json/string", test_visitor_out_string);
-    output_visitor_test_add("/visitor/json/enum", test_visitor_out_enum);
-    output_visitor_test_add("/visitor/json/enum-errors",
+    output_visitor_test_add("/visitor/json/int", &plain,
+                            test_visitor_out_int);
+    output_visitor_test_add("/visitor/json/bool", &plain,
+                            test_visitor_out_bool);
+    output_visitor_test_add("/visitor/json/number", &plain,
+                            test_visitor_out_number);
+    output_visitor_test_add("/visitor/json/string", &plain,
+                            test_visitor_out_string);
+    output_visitor_test_add("/visitor/json/enum", &plain,
+                            test_visitor_out_enum);
+    output_visitor_test_add("/visitor/json/enum-errors", &plain,
                             test_visitor_out_enum_errors);
-    output_visitor_test_add("/visitor/json/struct", test_visitor_out_struct);
-    output_visitor_test_add("/visitor/json/struct-nested",
+    output_visitor_test_add("/visitor/json/struct", &plain,
+                            test_visitor_out_struct);
+    output_visitor_test_add("/visitor/json/struct-nested", &plain,
                             test_visitor_out_struct_nested);
-    output_visitor_test_add("/visitor/json/struct-errors",
+    output_visitor_test_add("/visitor/json-pretty/struct-nested", &pretty,
+                            test_visitor_out_struct_nested);
+    output_visitor_test_add("/visitor/json/struct-errors", &plain,
                             test_visitor_out_struct_errors);
-    output_visitor_test_add("/visitor/json/list", test_visitor_out_list);
-    output_visitor_test_add("/visitor/json/any", test_visitor_out_any);
-    output_visitor_test_add("/visitor/json/union-flat",
+    output_visitor_test_add("/visitor/json/list", &plain,
+                            test_visitor_out_list);
+    output_visitor_test_add("/visitor/json/any", &plain,
+                            test_visitor_out_any);
+    output_visitor_test_add("/visitor/json-pretty/any", &pretty,
+                            test_visitor_out_any);
+    output_visitor_test_add("/visitor/json/union-flat", &plain,
                             test_visitor_out_union_flat);
-    output_visitor_test_add("/visitor/json/alternate",
+    output_visitor_test_add("/visitor/json/alternate", &plain,
                             test_visitor_out_alternate);
-    output_visitor_test_add("/visitor/json/empty", test_visitor_out_empty);
+    output_visitor_test_add("/visitor/json/empty", &plain,
+                            test_visitor_out_empty);

     g_test_run();

-- 
2.4.3

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

* [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter
  2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
                   ` (9 preceding siblings ...)
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 10/11] qapi: Support pretty printing in JSON output visitor Eric Blake
@ 2015-12-10 23:53 ` Eric Blake
  2015-12-11  1:36   ` Fam Zheng
  10 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2015-12-10 23:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, open list:Block layer core

Now that we can pretty-print straight to JSON from a visitor,
we can eliminate the temporary conversion into QObject inside
qemu-img.

RFC because at least qemu-iotests 043 has changed output, not
included in this version of the patch.  Conflicts with Fam's
qemu-img edits, so one of the two of us will get to rebase on
the other:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01756.html

Signed-off-by: Eric Blake <eblake@redhat.com>
Cc: Fam Zheng <famz@redhat.com>
---
 qemu-img.c | 70 ++++++++++++++++++++++++++------------------------------------
 1 file changed, 29 insertions(+), 41 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index cb961e6..08dca98 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -22,9 +22,9 @@
  * THE SOFTWARE.
  */
 #include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/json-output-visitor.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp/qobject-json.h"
+#include "qapi/qmp/qstring.h"
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
@@ -375,19 +375,15 @@ 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),
-                          &check, NULL, &local_err);
-    obj = qmp_output_get_qobject(ov);
-    str = qobject_to_json_pretty(obj);
-    assert(str != NULL);
-    qprintf(quiet, "%s\n", qstring_get_str(str));
-    qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
-    QDECREF(str);
+    char *str;
+    JsonOutputVisitor *ov = json_output_visitor_new(true);
+    visit_type_ImageCheck(json_output_get_visitor(ov),
+                          &check, NULL, &error_abort);
+    str = json_output_get_string(ov);
+    assert(str);
+    qprintf(quiet, "%s\n", str);
+    g_free(str);
+    json_output_visitor_cleanup(ov);
 }

 static void dump_human_image_check(ImageCheck *check, bool quiet)
@@ -1926,36 +1922,28 @@ 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),
-                             &list, NULL, &local_err);
-    obj = qmp_output_get_qobject(ov);
-    str = qobject_to_json_pretty(obj);
-    assert(str != NULL);
-    printf("%s\n", qstring_get_str(str));
-    qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
-    QDECREF(str);
+    char *str;
+    JsonOutputVisitor *ov = json_output_visitor_new(true);
+    visit_type_ImageInfoList(json_output_get_visitor(ov),
+                             &list, NULL, &error_abort);
+    str = json_output_get_string(ov);
+    assert(str);
+    printf("%s\n", str);
+    json_output_visitor_cleanup(ov);
+    g_free(str);
 }

 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),
-                         &info, NULL, &local_err);
-    obj = qmp_output_get_qobject(ov);
-    str = qobject_to_json_pretty(obj);
-    assert(str != NULL);
-    printf("%s\n", qstring_get_str(str));
-    qobject_decref(obj);
-    qmp_output_visitor_cleanup(ov);
-    QDECREF(str);
+    char *str;
+    JsonOutputVisitor *ov = json_output_visitor_new(true);
+    visit_type_ImageInfo(json_output_get_visitor(ov),
+                         &info, NULL, &error_abort);
+    str = json_output_get_string(ov);
+    assert(str);
+    printf("%s\n", str);
+    json_output_visitor_cleanup(ov);
+    g_free(str);
 }

 static void dump_human_image_info_list(ImageInfoList *list)
-- 
2.4.3

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

* Re: [Qemu-devel] [PATCH 07/11] qapi: add json output visitor
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 07/11] qapi: add json output visitor Eric Blake
@ 2015-12-11  0:24   ` Eric Blake
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-11  0:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, Michael Roth

[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]

On 12/10/2015 04:53 PM, Eric Blake wrote:
> We have several places that want to go from qapi to JSON; right now,
> they have to create an intermediate QObject to do the work.  That
> also has the drawback that the JSON formatting of a QDict will
> rearrange keys (according to a deterministic, but unpredictable,
> hash), when humans have an easier time if dicts are produced in
> the same order as the qapi type.
> 
> For these reasons, it is time to add a new JSON output visitor.
> This patch just adds the basic visitor and tests that it works;
> later patches will add pretty-printing, and convert clients to
> use the visitor.
> 
> Design choices: Unlike the QMP output visitor, the JSON visitor
> refuses to visit a required string with a NULL value (abort), as
> well as a non-finite number (raises an error message).  Reusing
> QString to grow the contents means that we easily share code with
> both qobject-json.c and qjson.c; although it might be nice to
> enhance things to take an optional output callback function so
> that the output can truly be streamed instead of collected in
> memory.
> 

> +static void json_output_type_number(Visitor *v, double *obj, const char *name,
> +                                    Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append_json_number(jov->str, *obj, errp);
> +}

Hmm. The generated qapi callers always pass a non-NULL local errp, and
fail the overall iteration even if the upper-level caller passed NULL.
My intent here was to allow the choice between diagnosing NaN, or
ignoring the problem and producing output anyways; but for that to work,
I may need to add a configuration parameter 'bool strict' at the
creation of the visitor, that determines whether I pass errp or NULL on
to qstring_append_json_number() here.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter Eric Blake
@ 2015-12-11  1:36   ` Fam Zheng
  0 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2015-12-11  1:36 UTC (permalink / raw)
  To: Eric Blake; +Cc: Kevin Wolf, qemu-devel, open list:Block layer core

On Thu, 12/10 16:53, Eric Blake wrote:
> Now that we can pretty-print straight to JSON from a visitor,
> we can eliminate the temporary conversion into QObject inside
> qemu-img.
> 
> RFC because at least qemu-iotests 043 has changed output, not
> included in this version of the patch.  Conflicts with Fam's
> qemu-img edits, so one of the two of us will get to rebase on
> the other:
> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01756.html

I touched qemu-img map's output, this patch updates info and check, so the
context conflict should be okay, but it would be better to get this patch in
before mine so I can update my patch to make use of the new formatter, without
code churn.

Thanks!

Fam

> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Cc: Fam Zheng <famz@redhat.com>
> ---
>  qemu-img.c | 70 ++++++++++++++++++++++++++------------------------------------
>  1 file changed, 29 insertions(+), 41 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index cb961e6..08dca98 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -22,9 +22,9 @@
>   * THE SOFTWARE.
>   */
>  #include "qapi-visit.h"
> -#include "qapi/qmp-output-visitor.h"
> +#include "qapi/json-output-visitor.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qobject-json.h"
> +#include "qapi/qmp/qstring.h"
>  #include "qemu-common.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> @@ -375,19 +375,15 @@ 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),
> -                          &check, NULL, &local_err);
> -    obj = qmp_output_get_qobject(ov);
> -    str = qobject_to_json_pretty(obj);
> -    assert(str != NULL);
> -    qprintf(quiet, "%s\n", qstring_get_str(str));
> -    qobject_decref(obj);
> -    qmp_output_visitor_cleanup(ov);
> -    QDECREF(str);
> +    char *str;
> +    JsonOutputVisitor *ov = json_output_visitor_new(true);
> +    visit_type_ImageCheck(json_output_get_visitor(ov),
> +                          &check, NULL, &error_abort);
> +    str = json_output_get_string(ov);
> +    assert(str);
> +    qprintf(quiet, "%s\n", str);
> +    g_free(str);
> +    json_output_visitor_cleanup(ov);
>  }
> 
>  static void dump_human_image_check(ImageCheck *check, bool quiet)
> @@ -1926,36 +1922,28 @@ 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),
> -                             &list, NULL, &local_err);
> -    obj = qmp_output_get_qobject(ov);
> -    str = qobject_to_json_pretty(obj);
> -    assert(str != NULL);
> -    printf("%s\n", qstring_get_str(str));
> -    qobject_decref(obj);
> -    qmp_output_visitor_cleanup(ov);
> -    QDECREF(str);
> +    char *str;
> +    JsonOutputVisitor *ov = json_output_visitor_new(true);
> +    visit_type_ImageInfoList(json_output_get_visitor(ov),
> +                             &list, NULL, &error_abort);
> +    str = json_output_get_string(ov);
> +    assert(str);
> +    printf("%s\n", str);
> +    json_output_visitor_cleanup(ov);
> +    g_free(str);
>  }
> 
>  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),
> -                         &info, NULL, &local_err);
> -    obj = qmp_output_get_qobject(ov);
> -    str = qobject_to_json_pretty(obj);
> -    assert(str != NULL);
> -    printf("%s\n", qstring_get_str(str));
> -    qobject_decref(obj);
> -    qmp_output_visitor_cleanup(ov);
> -    QDECREF(str);
> +    char *str;
> +    JsonOutputVisitor *ov = json_output_visitor_new(true);
> +    visit_type_ImageInfo(json_output_get_visitor(ov),
> +                         &info, NULL, &error_abort);
> +    str = json_output_get_string(ov);
> +    assert(str);
> +    printf("%s\n", str);
> +    json_output_visitor_cleanup(ov);
> +    g_free(str);
>  }
> 
>  static void dump_human_image_info_list(ImageInfoList *list)
> -- 
> 2.4.3
> 

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

* Re: [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h
  2015-12-10 23:53   ` [Qemu-devel] " Eric Blake
@ 2015-12-11 11:00     ` Paolo Bonzini
  -1 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-12-11 11:00 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, Chrysostomos Nanakos, Jeff Cody, Alberto Garcia,
	Michael S. Tsirkin, Jason Wang, Luiz Capitulino,
	Markus Armbruster, Michael Roth, Christian Borntraeger,
	Cornelia Huck, Alexander Graf, Richard Henderson, Gerd Hoffmann,
	open list:Block layer core, open list:Overall



On 11/12/2015 00:53, Eric Blake wrote:
> We have two different JSON visitors in the tree; and having both
> named 'qjson.h' can cause include confusion.  Rename the qapi
> version.
> 
> Kill trailing whitespace in the renamed tests/check-qobject-json.c
> to keep checkpatch.pl happy.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  MAINTAINERS                                   |  2 +-
>  balloon.c                                     |  2 +-
>  block.c                                       |  2 +-
>  block/archipelago.c                           |  2 +-
>  block/nbd.c                                   |  2 +-
>  block/quorum.c                                |  2 +-
>  blockjob.c                                    |  2 +-
>  hw/core/qdev.c                                |  2 +-
>  hw/misc/pvpanic.c                             |  2 +-
>  hw/net/virtio-net.c                           |  2 +-
>  include/qapi/qmp/{qjson.h => qobject-json.h}  |  0
>  include/qapi/qmp/types.h                      |  2 +-
>  monitor.c                                     |  2 +-
>  qapi/qmp-event.c                              |  2 +-
>  qemu-img.c                                    |  2 +-
>  qga/main.c                                    |  2 +-
>  qobject/Makefile.objs                         |  3 ++-
>  qobject/{qjson.c => qobject-json.c}           |  2 +-
>  target-s390x/kvm.c                            |  2 +-
>  tests/.gitignore                              |  2 +-
>  tests/Makefile                                |  8 ++++----
>  tests/{check-qjson.c => check-qobject-json.c} | 14 +++++++-------
>  tests/libqtest.c                              |  2 +-
>  ui/spice-core.c                               |  2 +-
>  vl.c                                          |  2 +-
>  25 files changed, 34 insertions(+), 33 deletions(-)
>  rename include/qapi/qmp/{qjson.h => qobject-json.h} (100%)
>  rename qobject/{qjson.c => qobject-json.c} (99%)
>  rename tests/{check-qjson.c => check-qobject-json.c} (99%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e8cee1e..c943ff4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1155,7 +1155,7 @@ X: include/qapi/qmp/dispatch.h
>  F: tests/check-qdict.c
>  F: tests/check-qfloat.c
>  F: tests/check-qint.c
> -F: tests/check-qjson.c
> +F: tests/check-qobject-json.c
>  F: tests/check-qlist.c
>  F: tests/check-qstring.c
>  T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
> diff --git a/balloon.c b/balloon.c
> index 0f45d1b..5983b4f 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -31,7 +31,7 @@
>  #include "trace.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
> diff --git a/block.c b/block.c
> index 9971976..e611002 100644
> --- a/block.c
> +++ b/block.c
> @@ -29,7 +29,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/notify.h"
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 855655c..80a1bb5 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -56,7 +56,7 @@
>  #include "qemu/thread.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/atomic.h"
> 
>  #include <inttypes.h>
> diff --git a/block/nbd.c b/block/nbd.c
> index cd6a587..9009f4f 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -32,7 +32,7 @@
>  #include "qemu/module.h"
>  #include "qemu/sockets.h"
>  #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index d162459..1e3a278 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -18,7 +18,7 @@
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qstring.h"
>  #include "qapi-event.h"
> diff --git a/blockjob.c b/blockjob.c
> index 80adb9d..84361f7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -31,7 +31,7 @@
>  #include "block/block_int.h"
>  #include "sysemu/block-backend.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/coroutine.h"
>  #include "qmp-commands.h"
>  #include "qemu/timer.h"
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b3ad467..a98304d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -31,7 +31,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/visitor.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/error-report.h"
>  #include "hw/hotplug.h"
>  #include "hw/boards.h"
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 3709488..2571483 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -13,7 +13,7 @@
>   */
> 
>  #include "qapi/qmp/qobject.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/log.h"
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a877614..5988bd1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -21,7 +21,7 @@
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
>  #include "hw/virtio/virtio-bus.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi-event.h"
>  #include "hw/virtio/virtio-access.h"
> 
> diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qobject-json.h
> similarity index 100%
> rename from include/qapi/qmp/qjson.h
> rename to include/qapi/qmp/qobject-json.h
> diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
> index 7782ec5..9109eda 100644
> --- a/include/qapi/qmp/types.h
> +++ b/include/qapi/qmp/types.h
> @@ -20,6 +20,6 @@
>  #include "qapi/qmp/qstring.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qlist.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #endif /* QEMU_OBJECTS_H */
> diff --git a/monitor.c b/monitor.c
> index e7e7ae2..1dfd359 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -55,7 +55,7 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include <qom/object_interfaces.h>
> diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
> index c0e435f..800b1f3 100644
> --- a/qapi/qmp-event.c
> +++ b/qapi/qmp-event.c
> @@ -16,7 +16,7 @@
>  #include "qemu-common.h"
>  #include "qapi/qmp-event.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  static QMPEventFuncEmit qmp_emit;
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 033011c..cb961e6 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -24,7 +24,7 @@
>  #include "qapi-visit.h"
>  #include "qapi/qmp-output-visitor.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu-common.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> diff --git a/qga/main.c b/qga/main.c
> index f83a97d..3cc0901 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -24,7 +24,7 @@
>  #include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qga/guest-agent-core.h"
>  #include "qemu/module.h"
>  #include "signal.h"
> diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs
> index bed5508..16a48ec 100644
> --- a/qobject/Makefile.objs
> +++ b/qobject/Makefile.objs
> @@ -1,2 +1,3 @@
>  util-obj-y = qnull.o qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
> -util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
> +util-obj-y += qobject-json.o qobject.o
> +util-obj-y += json-lexer.o json-streamer.o json-parser.o
> diff --git a/qobject/qjson.c b/qobject/qobject-json.c
> similarity index 99%
> rename from qobject/qjson.c
> rename to qobject/qobject-json.c
> index 41d9d65..8fc65a4 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qobject-json.c
> @@ -14,7 +14,7 @@
>  #include "qapi/qmp/json-lexer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/json-streamer.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qbool.h"
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 75a0e5d..40dd729 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -36,7 +36,7 @@
>  #include "hw/hw.h"
>  #include "cpu.h"
>  #include "sysemu/device_tree.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "exec/gdbstub.h"
>  #include "exec/address-spaces.h"
>  #include "trace.h"
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 1e55722..6d3416d 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,8 +1,8 @@
>  check-qdict
>  check-qfloat
>  check-qint
> -check-qjson
>  check-qlist
> +check-qobject-json
>  check-qstring
>  check-qom-interface
>  check-qom-proplist
> diff --git a/tests/Makefile b/tests/Makefile
> index 053c1ae..17d74e2 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -16,8 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
>  gcov-files-check-qstring-y = qobject/qstring.c
>  check-unit-y += tests/check-qlist$(EXESUF)
>  gcov-files-check-qlist-y = qobject/qlist.c
> -check-unit-y += tests/check-qjson$(EXESUF)
> -gcov-files-check-qjson-y = qobject/qjson.c
> +check-unit-y += tests/check-qobject-json$(EXESUF)
> +gcov-files-check-qobject-json-y = qobject/qobject-json.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-qmp-input-visitor$(EXESUF)
> @@ -360,7 +360,7 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
>  	tests/test-qmp-introspect.h
> 
>  test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
> -	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
> +	tests/check-qlist.o tests/check-qfloat.o tests/check-qobject-json.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-qmp-input-visitor.o tests/test-qmp-input-strict.o \
> @@ -387,7 +387,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
>  tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
>  tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
>  tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
> -tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> +tests/check-qobject-json$(EXESUF): tests/check-qobject-json.o $(test-util-obj-y)
>  tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
>  tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
>  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
> diff --git a/tests/check-qjson.c b/tests/check-qobject-json.c
> similarity index 99%
> rename from tests/check-qjson.c
> rename to tests/check-qobject-json.c
> index 61e9bfb..9c4e53a 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qobject-json.c
> @@ -18,7 +18,7 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qfloat.h"
>  #include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #include "qemu-common.h"
> 
> @@ -63,7 +63,7 @@ static void escaped_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].decoded);
> 
> @@ -98,7 +98,7 @@ static void simple_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -106,7 +106,7 @@ static void simple_string(void)
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);
> 
>          qobject_decref(obj);
> -        
> +
>          QDECREF(str);
>      }
>  }
> @@ -132,7 +132,7 @@ static void single_quote_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -880,7 +880,7 @@ static void vararg_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -1144,7 +1144,7 @@ static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
>          helper.index = 0;
>          helper.objs = lhs->value.qlist;
>          helper.result = 1;
> -        
> +
>          qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
> 
>          return helper.result;
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index fa314e1..149eb4c 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -32,7 +32,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/json-streamer.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 5
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 6a62d71..ceb18ae 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -32,7 +32,7 @@
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/notify.h"
>  #include "migration/migration.h"
>  #include "hw/hw.h"
> diff --git a/vl.c b/vl.c
> index ed1203e..eaf1d0c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -92,7 +92,7 @@ int main(int argc, char **argv)
>  #include "audio/audio.h"
>  #include "migration/migration.h"
>  #include "sysemu/kvm.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/option.h"
>  #include "qemu/config-file.h"
>  #include "qemu-options.h"
> 

I'd rather get rid completely of the other qjson, but the rename makes
sense anyway.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

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

* Re: [Qemu-devel] [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h
@ 2015-12-11 11:00     ` Paolo Bonzini
  0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-12-11 11:00 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, Alexander Graf, Alberto Garcia, Michael Roth,
	Chrysostomos Nanakos, Michael S. Tsirkin, Jeff Cody,
	Markus Armbruster, Luiz Capitulino, Christian Borntraeger,
	Gerd Hoffmann, open list:Overall, Cornelia Huck,
	open list:Block layer core, Jason Wang, Richard Henderson



On 11/12/2015 00:53, Eric Blake wrote:
> We have two different JSON visitors in the tree; and having both
> named 'qjson.h' can cause include confusion.  Rename the qapi
> version.
> 
> Kill trailing whitespace in the renamed tests/check-qobject-json.c
> to keep checkpatch.pl happy.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  MAINTAINERS                                   |  2 +-
>  balloon.c                                     |  2 +-
>  block.c                                       |  2 +-
>  block/archipelago.c                           |  2 +-
>  block/nbd.c                                   |  2 +-
>  block/quorum.c                                |  2 +-
>  blockjob.c                                    |  2 +-
>  hw/core/qdev.c                                |  2 +-
>  hw/misc/pvpanic.c                             |  2 +-
>  hw/net/virtio-net.c                           |  2 +-
>  include/qapi/qmp/{qjson.h => qobject-json.h}  |  0
>  include/qapi/qmp/types.h                      |  2 +-
>  monitor.c                                     |  2 +-
>  qapi/qmp-event.c                              |  2 +-
>  qemu-img.c                                    |  2 +-
>  qga/main.c                                    |  2 +-
>  qobject/Makefile.objs                         |  3 ++-
>  qobject/{qjson.c => qobject-json.c}           |  2 +-
>  target-s390x/kvm.c                            |  2 +-
>  tests/.gitignore                              |  2 +-
>  tests/Makefile                                |  8 ++++----
>  tests/{check-qjson.c => check-qobject-json.c} | 14 +++++++-------
>  tests/libqtest.c                              |  2 +-
>  ui/spice-core.c                               |  2 +-
>  vl.c                                          |  2 +-
>  25 files changed, 34 insertions(+), 33 deletions(-)
>  rename include/qapi/qmp/{qjson.h => qobject-json.h} (100%)
>  rename qobject/{qjson.c => qobject-json.c} (99%)
>  rename tests/{check-qjson.c => check-qobject-json.c} (99%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e8cee1e..c943ff4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1155,7 +1155,7 @@ X: include/qapi/qmp/dispatch.h
>  F: tests/check-qdict.c
>  F: tests/check-qfloat.c
>  F: tests/check-qint.c
> -F: tests/check-qjson.c
> +F: tests/check-qobject-json.c
>  F: tests/check-qlist.c
>  F: tests/check-qstring.c
>  T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
> diff --git a/balloon.c b/balloon.c
> index 0f45d1b..5983b4f 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -31,7 +31,7 @@
>  #include "trace.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
> diff --git a/block.c b/block.c
> index 9971976..e611002 100644
> --- a/block.c
> +++ b/block.c
> @@ -29,7 +29,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/notify.h"
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 855655c..80a1bb5 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -56,7 +56,7 @@
>  #include "qemu/thread.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/atomic.h"
> 
>  #include <inttypes.h>
> diff --git a/block/nbd.c b/block/nbd.c
> index cd6a587..9009f4f 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -32,7 +32,7 @@
>  #include "qemu/module.h"
>  #include "qemu/sockets.h"
>  #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index d162459..1e3a278 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -18,7 +18,7 @@
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qstring.h"
>  #include "qapi-event.h"
> diff --git a/blockjob.c b/blockjob.c
> index 80adb9d..84361f7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -31,7 +31,7 @@
>  #include "block/block_int.h"
>  #include "sysemu/block-backend.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/coroutine.h"
>  #include "qmp-commands.h"
>  #include "qemu/timer.h"
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b3ad467..a98304d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -31,7 +31,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/visitor.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/error-report.h"
>  #include "hw/hotplug.h"
>  #include "hw/boards.h"
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 3709488..2571483 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -13,7 +13,7 @@
>   */
> 
>  #include "qapi/qmp/qobject.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/log.h"
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a877614..5988bd1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -21,7 +21,7 @@
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
>  #include "hw/virtio/virtio-bus.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi-event.h"
>  #include "hw/virtio/virtio-access.h"
> 
> diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qobject-json.h
> similarity index 100%
> rename from include/qapi/qmp/qjson.h
> rename to include/qapi/qmp/qobject-json.h
> diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
> index 7782ec5..9109eda 100644
> --- a/include/qapi/qmp/types.h
> +++ b/include/qapi/qmp/types.h
> @@ -20,6 +20,6 @@
>  #include "qapi/qmp/qstring.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qlist.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #endif /* QEMU_OBJECTS_H */
> diff --git a/monitor.c b/monitor.c
> index e7e7ae2..1dfd359 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -55,7 +55,7 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include <qom/object_interfaces.h>
> diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
> index c0e435f..800b1f3 100644
> --- a/qapi/qmp-event.c
> +++ b/qapi/qmp-event.c
> @@ -16,7 +16,7 @@
>  #include "qemu-common.h"
>  #include "qapi/qmp-event.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  static QMPEventFuncEmit qmp_emit;
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 033011c..cb961e6 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -24,7 +24,7 @@
>  #include "qapi-visit.h"
>  #include "qapi/qmp-output-visitor.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu-common.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> diff --git a/qga/main.c b/qga/main.c
> index f83a97d..3cc0901 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -24,7 +24,7 @@
>  #include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qga/guest-agent-core.h"
>  #include "qemu/module.h"
>  #include "signal.h"
> diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs
> index bed5508..16a48ec 100644
> --- a/qobject/Makefile.objs
> +++ b/qobject/Makefile.objs
> @@ -1,2 +1,3 @@
>  util-obj-y = qnull.o qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
> -util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
> +util-obj-y += qobject-json.o qobject.o
> +util-obj-y += json-lexer.o json-streamer.o json-parser.o
> diff --git a/qobject/qjson.c b/qobject/qobject-json.c
> similarity index 99%
> rename from qobject/qjson.c
> rename to qobject/qobject-json.c
> index 41d9d65..8fc65a4 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qobject-json.c
> @@ -14,7 +14,7 @@
>  #include "qapi/qmp/json-lexer.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/json-streamer.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qbool.h"
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 75a0e5d..40dd729 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -36,7 +36,7 @@
>  #include "hw/hw.h"
>  #include "cpu.h"
>  #include "sysemu/device_tree.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "exec/gdbstub.h"
>  #include "exec/address-spaces.h"
>  #include "trace.h"
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 1e55722..6d3416d 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,8 +1,8 @@
>  check-qdict
>  check-qfloat
>  check-qint
> -check-qjson
>  check-qlist
> +check-qobject-json
>  check-qstring
>  check-qom-interface
>  check-qom-proplist
> diff --git a/tests/Makefile b/tests/Makefile
> index 053c1ae..17d74e2 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -16,8 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
>  gcov-files-check-qstring-y = qobject/qstring.c
>  check-unit-y += tests/check-qlist$(EXESUF)
>  gcov-files-check-qlist-y = qobject/qlist.c
> -check-unit-y += tests/check-qjson$(EXESUF)
> -gcov-files-check-qjson-y = qobject/qjson.c
> +check-unit-y += tests/check-qobject-json$(EXESUF)
> +gcov-files-check-qobject-json-y = qobject/qobject-json.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-qmp-input-visitor$(EXESUF)
> @@ -360,7 +360,7 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
>  	tests/test-qmp-introspect.h
> 
>  test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
> -	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
> +	tests/check-qlist.o tests/check-qfloat.o tests/check-qobject-json.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-qmp-input-visitor.o tests/test-qmp-input-strict.o \
> @@ -387,7 +387,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
>  tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
>  tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
>  tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
> -tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> +tests/check-qobject-json$(EXESUF): tests/check-qobject-json.o $(test-util-obj-y)
>  tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
>  tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
>  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
> diff --git a/tests/check-qjson.c b/tests/check-qobject-json.c
> similarity index 99%
> rename from tests/check-qjson.c
> rename to tests/check-qobject-json.c
> index 61e9bfb..9c4e53a 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qobject-json.c
> @@ -18,7 +18,7 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qfloat.h"
>  #include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #include "qemu-common.h"
> 
> @@ -63,7 +63,7 @@ static void escaped_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].decoded);
> 
> @@ -98,7 +98,7 @@ static void simple_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -106,7 +106,7 @@ static void simple_string(void)
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);
> 
>          qobject_decref(obj);
> -        
> +
>          QDECREF(str);
>      }
>  }
> @@ -132,7 +132,7 @@ static void single_quote_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -880,7 +880,7 @@ static void vararg_string(void)
> 
>          g_assert(obj != NULL);
>          g_assert(qobject_type(obj) == QTYPE_QSTRING);
> -        
> +
>          str = qobject_to_qstring(obj);
>          g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
> 
> @@ -1144,7 +1144,7 @@ static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
>          helper.index = 0;
>          helper.objs = lhs->value.qlist;
>          helper.result = 1;
> -        
> +
>          qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
> 
>          return helper.result;
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index fa314e1..149eb4c 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -32,7 +32,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/json-streamer.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
> 
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 5
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 6a62d71..ceb18ae 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -32,7 +32,7 @@
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/notify.h"
>  #include "migration/migration.h"
>  #include "hw/hw.h"
> diff --git a/vl.c b/vl.c
> index ed1203e..eaf1d0c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -92,7 +92,7 @@ int main(int argc, char **argv)
>  #include "audio/audio.h"
>  #include "migration/migration.h"
>  #include "sysemu/kvm.h"
> -#include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qobject-json.h"
>  #include "qemu/option.h"
>  #include "qemu/config-file.h"
>  #include "qemu-options.h"
> 

I'd rather get rid completely of the other qjson, but the rename makes
sense anyway.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

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

* Re: [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor
  2015-12-10 23:53 ` [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor Eric Blake
@ 2015-12-11 11:10   ` Paolo Bonzini
  2015-12-11 13:42     ` Eric Blake
  2015-12-18 23:06     ` Eric Blake
  0 siblings, 2 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-12-11 11:10 UTC (permalink / raw)
  To: Eric Blake, qemu-devel



On 11/12/2015 00:53, Eric Blake wrote:
> Instead of rolling our own limited JSON outputter, we can just
> wrap the more full-featured JSON output Visitor.
> 
> This slightly changes the output (different spacing), but the
> result is still equivalent JSON contents.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qjson.c | 61 ++++++++++++++++++++++---------------------------------------
>  1 file changed, 22 insertions(+), 39 deletions(-)

I didn't find out which tree your patches apply to, but this:

diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..26986d8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -646,7 +646,7 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
     return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
 }
 
-static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
+static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, Visitor *vmdesc)
 {
     int64_t old_offset, size;
 
@@ -655,14 +655,14 @@ static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdes
     size = qemu_ftell_fast(f) - old_offset;
 
     if (vmdesc) {
-        json_prop_int(vmdesc, "size", size);
-        json_start_array(vmdesc, "fields");
-        json_start_object(vmdesc, NULL);
-        json_prop_str(vmdesc, "name", "data");
-        json_prop_int(vmdesc, "size", size);
-        json_prop_str(vmdesc, "type", "buffer");
-        json_end_object(vmdesc);
-        json_end_array(vmdesc);
+        visit_type_int(vmdesc, size, "size", &error_abort);
+        visit_start_list(vmdesc, NULL, 0, "fields", &error_abort);
+        visit_start_struct(vmdesc, NULL, 0, NULL, &error_abort);
+        visit_type_str(vmdesc, (char **)&"data", "name", &error_abort);
+        visit_type_int(vmdesc, size, "size", &error_abort);
+        visit_type_str(vmdesc, (char **)&"buffer", "type", &error_abort);
+        visit_end_struct(vmdesc);
+        visit_end_list(vmdesc);
     }
 }
 
@@ -1028,7 +1028,9 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
 
 void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
 {
-    QJSON *vmdesc;
+    JsonOutputVisitor *vmdesc_jov;
+    Visitor *vmdesc;
+    char *vmdesc_str;
     int vmdesc_len;
     SaveStateEntry *se;
     int ret;
@@ -1068,9 +1070,11 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
         return;
     }
 
-    vmdesc = qjson_new();
-    json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
-    json_start_array(vmdesc, "devices");
+    vmdesc_jov = json_output_visitor_new();
+    vmdesc = json_output_get_visitor(vmdesc_jov);
+    visit_start_struct(vmdesc, NULL, 0, NULL, &error_abort);
+    visit_type_int(vmdesc, TARGET_PAGE_SIZE, "page_size", &error_abort);
+    visit_start_list(vmdesc, NULL, 0, "devices", &error_abort);
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
 
         if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
@@ -1083,15 +1087,15 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
 
         trace_savevm_section_start(se->idstr, se->section_id);
 
-        json_start_object(vmdesc, NULL);
-        json_prop_str(vmdesc, "name", se->idstr);
-        json_prop_int(vmdesc, "instance_id", se->instance_id);
+        visit_start_struct(vmdesc, NULL, 0, NULL, &error_abort);
+        visit_type_str(vmdesc, (char **)&se->idstr, "name", &error_abort);
+        visit_type_int(vmdesc, se->instance_id, "instance_id", &error_abort);
 
         save_section_header(f, se, QEMU_VM_SECTION_FULL);
 
         vmstate_save(f, se, vmdesc);
 
-        json_end_object(vmdesc);
+        visit_end_struct(vmdesc);
         trace_savevm_section_end(se->idstr, se->section_id, 0);
         save_section_footer(f, se);
     }
@@ -1101,16 +1105,18 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
         qemu_put_byte(f, QEMU_VM_EOF);
     }
 
-    json_end_array(vmdesc);
-    qjson_finish(vmdesc);
-    vmdesc_len = strlen(qjson_get_str(vmdesc));
+    visit_end_list(vmdesc);
+    visit_end_struct(vmdesc);
+    vmdesc_str = json_output_get_string(vmdesc_jov);
+    vmdesc_len = strlen(vmdesc_str);
 
     if (should_send_vmdesc()) {
         qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
         qemu_put_be32(f, vmdesc_len);
-        qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
+        qemu_put_buffer(f, (uint8_t *)vmdesc_str, vmdesc_len);
     }
-    object_unref(OBJECT(vmdesc));
+    g_free(vmdesc_str);
+    json_output_visitor_cleanup(vmdesc_jov);
 
     qemu_fflush(f);
 }

compiles and is pretty much a 1:1 translation from the qjson.c API to the
visitor API (using this patch as a guide).  Feel free to include it and
remove qjson.c.  Alternatively, you can leave out this patch and I'll test
and submit the transition.

Paolo

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

* Re: [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor
  2015-12-11 11:10   ` Paolo Bonzini
@ 2015-12-11 13:42     ` Eric Blake
  2015-12-11 13:45       ` Paolo Bonzini
  2015-12-18 23:06     ` Eric Blake
  1 sibling, 1 reply; 21+ messages in thread
From: Eric Blake @ 2015-12-11 13:42 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]

On 12/11/2015 04:10 AM, Paolo Bonzini wrote:
> 
> 
> On 11/12/2015 00:53, Eric Blake wrote:
>> Instead of rolling our own limited JSON outputter, we can just
>> wrap the more full-featured JSON output Visitor.
>>
>> This slightly changes the output (different spacing), but the
>> result is still equivalent JSON contents.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  qjson.c | 61 ++++++++++++++++++++++---------------------------------------
>>  1 file changed, 22 insertions(+), 39 deletions(-)
> 
> I didn't find out which tree your patches apply to, but this:

Mentioned in the cover letter:

+ v8 or later of my qapi subset E patches (at the time of this mail,
only v7 has been posted):
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01181.html

Also available as a tag at this location (with prerequisites applied):
git fetch git://repo.or.cz/qemu/ericb.git qapi-jsonv1

(note that it is only a tag, not a branch, and you are right that it
doesn't yet apply to any maintainer's tree).


> @@ -1068,9 +1070,11 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
>          return;
>      }
>  
> -    vmdesc = qjson_new();
> -    json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
> -    json_start_array(vmdesc, "devices");
> +    vmdesc_jov = json_output_visitor_new();
> +    vmdesc = json_output_get_visitor(vmdesc_jov);
> +    visit_start_struct(vmdesc, NULL, 0, NULL, &error_abort);
> +    visit_type_int(vmdesc, TARGET_PAGE_SIZE, "page_size", &error_abort);
> +    visit_start_list(vmdesc, NULL, 0, "devices", &error_abort);
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {

Nice idea.

> compiles and is pretty much a 1:1 translation from the qjson.c API to the
> visitor API (using this patch as a guide).  Feel free to include it and
> remove qjson.c.  Alternatively, you can leave out this patch and I'll test
> and submit the transition.

Should I squash the two together, or keep my current patch and drop
qjson.c as a separate patch?

Also, while reading this, I noticed that qjson.h has signatures that
match JSON ('name' comes before 'value', when generating a "name":value
pair), while visitor.h has signatures that are backwards ('obj' comes
before 'name', even though the output will be "name":value-of-obj).  I'm
seriously debating about a tree-wide coccinelle patch to reverse the
order or arguments of all the visit_type_* functions.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor
  2015-12-11 13:42     ` Eric Blake
@ 2015-12-11 13:45       ` Paolo Bonzini
  0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-12-11 13:45 UTC (permalink / raw)
  To: Eric Blake, qemu-devel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 11/12/2015 14:42, Eric Blake wrote:
>>> compiles and is pretty much a 1:1 translation from the qjson.c
>>> API to the visitor API (using this patch as a guide).  Feel
>>> free to include it and remove qjson.c.  Alternatively, you can
>>> leave out this patch and I'll test and submit the transition.
> Should I squash the two together, or keep my current patch and
> drop qjson.c as a separate patch?

As you prefer.

> Also, while reading this, I noticed that qjson.h has signatures
> that match JSON ('name' comes before 'value', when generating a
> "name":value pair), while visitor.h has signatures that are
> backwards ('obj' comes before 'name', even though the output will
> be "name":value-of-obj).  I'm seriously debating about a tree-wide
> coccinelle patch to reverse the order or arguments of all the
> visit_type_* functions.

That would be doable, there aren't many users outside generated code.

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWatN5AAoJEL/70l94x66D8PQIAKEzIEUqjfhDMLhov4NwxGs3
LvyW+/w5AyDEZOJMayBuc3y7fSTbLp/eMQPsfBOIvBCMr/aEE4Tij0CzW3JE4FnH
IFCW94F8Q7vq8jTYKFU6222Nk7vUGOLG9y3jQSnLX4RzpRizhWjk73DtkED7YVu3
5J0Ef3+ZEobCZA9zOwQT7+pqc9ah/M6tGsZodvML2FopCk/SsTBauPjoSeP0knHE
1MJsu/KVjn1508yn1SY9d0HJKQx2dM3nzQRtp0xbWx7x8o7u826axpVQieq7YBpb
PaRW/vqiXvdnRkrCRHZAL6PJ4GLyquX42E7HjqCQjibA06fbtqmLoYQA2A/2Pyg=
=EJIn
-----END PGP SIGNATURE-----

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

* Re: [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor
  2015-12-11 11:10   ` Paolo Bonzini
  2015-12-11 13:42     ` Eric Blake
@ 2015-12-18 23:06     ` Eric Blake
  1 sibling, 0 replies; 21+ messages in thread
From: Eric Blake @ 2015-12-18 23:06 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

On 12/11/2015 04:10 AM, Paolo Bonzini wrote:
> 
> 
> On 11/12/2015 00:53, Eric Blake wrote:
>> Instead of rolling our own limited JSON outputter, we can just
>> wrap the more full-featured JSON output Visitor.
>>
>> This slightly changes the output (different spacing), but the
>> result is still equivalent JSON contents.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  qjson.c | 61 ++++++++++++++++++++++---------------------------------------
>>  1 file changed, 22 insertions(+), 39 deletions(-)
> 
> I didn't find out which tree your patches apply to, but this:
> 

>      if (vmdesc) {
> -        json_prop_int(vmdesc, "size", size);
> -        json_start_array(vmdesc, "fields");
> -        json_start_object(vmdesc, NULL);
> -        json_prop_str(vmdesc, "name", "data");
> -        json_prop_int(vmdesc, "size", size);
> -        json_prop_str(vmdesc, "type", "buffer");
> -        json_end_object(vmdesc);
> -        json_end_array(vmdesc);
> +        visit_type_int(vmdesc, size, "size", &error_abort);

visit_type_int() takes 'int64_t*', not 'int'.  In places, that means I'd
have to add a temporary variable to cover the fact that the input is not
the right type for writing '&size'

> @@ -1068,9 +1070,11 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
>          return;
>      }
>  
> -    vmdesc = qjson_new();
> -    json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
> -    json_start_array(vmdesc, "devices");
> +    vmdesc_jov = json_output_visitor_new();
> +    vmdesc = json_output_get_visitor(vmdesc_jov);
> +    visit_start_struct(vmdesc, NULL, 0, NULL, &error_abort);
> +    visit_type_int(vmdesc, TARGET_PAGE_SIZE, "page_size", &error_abort);

...or even cover the case where it is a constant whose address does not
even exist.

> 
> compiles and is pretty much a 1:1 translation from the qjson.c API to the
> visitor API (using this patch as a guide).  Feel free to include it and
> remove qjson.c.  Alternatively, you can leave out this patch and I'll test
> and submit the transition.

So the patch to avoid qjson.c is not quite 1:1. Still, I'll go ahead and
complete the patch for my v2, to see if you still like it.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

end of thread, other threads:[~2015-12-18 23:06 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-10 23:53 [Qemu-devel] [RFC PATCH 00/11] Add qapi-to-JSON output visitor Eric Blake
2015-12-10 23:53 ` [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h Eric Blake
2015-12-10 23:53   ` [Qemu-devel] " Eric Blake
2015-12-11 11:00   ` Paolo Bonzini
2015-12-11 11:00     ` [Qemu-devel] " Paolo Bonzini
2015-12-10 23:53 ` [Qemu-devel] [PATCH 02/11] qapi: Improve use of qmp/types.h Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 03/11] qapi: Factor out JSON string escaping Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 04/11] qapi: Factor out JSON number formatting Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 05/11] qapi: Use qstring_append_chr() where appropriate Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 06/11] qapi: Add qstring_append_format() Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 07/11] qapi: add json output visitor Eric Blake
2015-12-11  0:24   ` Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 08/11] qjson: Simplify by using json-output-visitor Eric Blake
2015-12-11 11:10   ` Paolo Bonzini
2015-12-11 13:42     ` Eric Blake
2015-12-11 13:45       ` Paolo Bonzini
2015-12-18 23:06     ` Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 09/11] qapi: Add qobject_to_json_pretty_prefix() Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 10/11] qapi: Support pretty printing in JSON output visitor Eric Blake
2015-12-10 23:53 ` [Qemu-devel] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter Eric Blake
2015-12-11  1:36   ` Fam Zheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.