All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/20] Immutable QString, and also one JSON writer less
@ 2020-12-11 17:11 Markus Armbruster
  2020-12-11 17:11 ` [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output Markus Armbruster
                   ` (20 more replies)
  0 siblings, 21 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Daniel P . Berrangé,
	Eduardo Habkost, qemu-block, Juan Quintela, Yuval Shaia, mdroth,
	Dr . David Alan Gilbert, Paolo Bonzini, Max Reitz

Based-on: <20201210161452.2813491-1-armbru@redhat.com>

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Yuval Shaia <yuval.shaia.ml@gmail.com>
Cc: qemu-block@nongnu.org

Markus Armbruster (20):
  hmp: Simplify how qmp_human_monitor_command() gets output
  monitor: Use GString instead of QString for output buffer
  qobject: Make qobject_to_json_pretty() take a pretty argument
  qobject: Use GString instead of QString to accumulate JSON
  qobject: Change qobject_to_json()'s value to GString
  Revert "qstring: add qstring_free()"
  hw/rdma: Replace QList by GQueue
  qobject: Move internals to qobject-internal.h
  qmp: Fix tracing of non-string command IDs
  block: Avoid qobject_get_try_str()
  Revert "qobject: let object_property_get_str() use new API"
  qobject: Drop qobject_get_try_str()
  qobject: Drop qstring_get_try_str()
  qobject: Factor quoted_str() out of to_json()
  qobject: Factor JSON writer out of qobject_to_json()
  migration: Replace migration's JSON writer by the general one
  json: Use GString instead of QString to accumulate strings
  keyval: Use GString to accumulate value strings
  block: Use GString instead of QString to build filenames
  qobject: Make QString immutable

 hw/rdma/rdma_backend_defs.h        |   2 +-
 hw/rdma/rdma_utils.h               |  15 +-
 include/migration/vmstate.h        |   7 +-
 include/qapi/qmp/json-writer.h     |  35 ++++
 include/qapi/qmp/qbool.h           |   2 -
 include/qapi/qmp/qdict.h           |   2 -
 include/qapi/qmp/qjson.h           |   4 +-
 include/qapi/qmp/qlist.h           |   2 -
 include/qapi/qmp/qnull.h           |   2 -
 include/qapi/qmp/qnum.h            |   3 -
 include/qapi/qmp/qobject.h         |   9 +-
 include/qapi/qmp/qstring.h         |  14 +-
 include/qemu/typedefs.h            |   4 +-
 migration/qjson.h                  |  29 ----
 monitor/monitor-internal.h         |   2 +-
 qobject/qobject-internal.h         |  39 +++++
 block.c                            |  23 +--
 block/rbd.c                        |   2 +-
 hw/display/virtio-gpu.c            |   2 +-
 hw/intc/s390_flic_kvm.c            |   2 +-
 hw/nvram/eeprom93xx.c              |   2 +-
 hw/nvram/fw_cfg.c                  |   2 +-
 hw/pci/msix.c                      |   2 +-
 hw/pci/pci.c                       |   4 +-
 hw/pci/shpc.c                      |   2 +-
 hw/rdma/rdma_backend.c             |  10 +-
 hw/rdma/rdma_utils.c               |  29 ++--
 hw/rtc/twl92230.c                  |   2 +-
 hw/scsi/scsi-bus.c                 |   2 +-
 hw/usb/redirect.c                  |   7 +-
 hw/virtio/virtio.c                 |   4 +-
 migration/qjson.c                  | 114 -------------
 migration/savevm.c                 |  53 ++++---
 migration/vmstate-types.c          |  38 ++---
 migration/vmstate.c                |  52 +++---
 monitor/misc.c                     |   6 +-
 monitor/monitor.c                  |  20 +--
 monitor/qmp.c                      |  46 +++---
 qemu-img.c                         |  33 ++--
 qga/main.c                         |  22 +--
 qobject/json-parser.c              |  30 ++--
 qobject/json-writer.c              | 247 +++++++++++++++++++++++++++++
 qobject/qbool.c                    |   1 +
 qobject/qdict.c                    |   1 +
 qobject/qjson.c                    | 144 ++++-------------
 qobject/qlist.c                    |   1 +
 qobject/qnull.c                    |   1 +
 qobject/qnum.c                     |   6 +-
 qobject/qobject.c                  |   1 +
 qobject/qstring.c                  | 117 +++-----------
 qom/object.c                       |   9 +-
 qom/object_interfaces.c            |   4 +-
 qom/qom-hmp-cmds.c                 |   7 +-
 target/alpha/machine.c             |   2 +-
 target/arm/machine.c               |   6 +-
 target/avr/machine.c               |   4 +-
 target/hppa/machine.c              |   4 +-
 target/microblaze/machine.c        |   2 +-
 target/mips/machine.c              |   4 +-
 target/openrisc/machine.c          |   2 +-
 target/ppc/machine.c               |  10 +-
 target/sparc/machine.c             |   2 +-
 tests/check-qjson.c                |  67 ++++----
 tests/check-qobject.c              |   3 +-
 tests/check-qstring.c              |  16 --
 tests/qtest/libqtest.c             |  20 ++-
 tests/test-visitor-serialization.c |   6 +-
 util/keyval.c                      |  11 +-
 migration/meson.build              |   1 -
 qobject/meson.build                |   5 +-
 70 files changed, 679 insertions(+), 705 deletions(-)
 create mode 100644 include/qapi/qmp/json-writer.h
 delete mode 100644 migration/qjson.h
 create mode 100644 qobject/qobject-internal.h
 delete mode 100644 migration/qjson.c
 create mode 100644 qobject/json-writer.c

-- 
2.26.2



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

* [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-16 18:53   ` Dr. David Alan Gilbert
  2020-12-11 17:11 ` [PATCH 02/20] monitor: Use GString instead of QString for output buffer Markus Armbruster
                   ` (19 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, Dr . David Alan Gilbert

Commit 48c043d0d1 "hmp: human-monitor-command: stop using the Memory
chardev driver" left us "if string is non-empty, duplicate it, else
duplicate the empty string".  Meh.  Duplicate it unconditionally.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor/misc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/monitor/misc.c b/monitor/misc.c
index 398211a034..6c3e8506a9 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -136,11 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     handle_hmp_command(&hmp, command_line);
 
     WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
-        if (qstring_get_length(hmp.common.outbuf) > 0) {
-            output = g_strdup(qstring_get_str(hmp.common.outbuf));
-        } else {
-            output = g_strdup("");
-        }
+        output = g_strdup(qstring_get_str(hmp.common.outbuf));
     }
 
 out:
-- 
2.26.2



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

* [PATCH 02/20] monitor: Use GString instead of QString for output buffer
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
  2020-12-11 17:11 ` [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-16 19:36   ` Dr. David Alan Gilbert
  2020-12-11 17:11 ` [PATCH 03/20] qobject: Make qobject_to_json_pretty() take a pretty argument Markus Armbruster
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, Dr . David Alan Gilbert

GString has a richer set of string operations than QString.  It should
be preferred to QString except where we need a QObject or reference
counting.  We don't here.  Switch to GString, and put its richer
interface to use.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor/monitor-internal.h |  2 +-
 monitor/misc.c             |  2 +-
 monitor/monitor.c          | 20 ++++++++------------
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index a6131554da..40903d6386 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -105,7 +105,7 @@ struct Monitor {
      * Members that are protected by the per-monitor lock
      */
     QLIST_HEAD(, mon_fd_t) fds;
-    QString *outbuf;
+    GString *outbuf;
     guint out_watch;
     /* Read under either BQL or mon_lock, written with BQL+mon_lock.  */
     int mux_out;
diff --git a/monitor/misc.c b/monitor/misc.c
index 6c3e8506a9..814d22de11 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -136,7 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     handle_hmp_command(&hmp, command_line);
 
     WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
-        output = g_strdup(qstring_get_str(hmp.common.outbuf));
+        output = g_strdup(hmp.common.outbuf->str);
     }
 
 out:
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 84222cd130..1e4a6b3f20 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -29,7 +29,6 @@
 #include "qapi/qapi-emit-events.h"
 #include "qapi/qapi-visit-control.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qstring.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include "sysemu/qtest.h"
@@ -181,22 +180,19 @@ static void monitor_flush_locked(Monitor *mon)
         return;
     }
 
-    buf = qstring_get_str(mon->outbuf);
-    len = qstring_get_length(mon->outbuf);
+    buf = mon->outbuf->str;
+    len = mon->outbuf->len;
 
     if (len && !mon->mux_out) {
         rc = qemu_chr_fe_write(&mon->chr, (const uint8_t *) buf, len);
         if ((rc < 0 && errno != EAGAIN) || (rc == len)) {
             /* all flushed or error */
-            qobject_unref(mon->outbuf);
-            mon->outbuf = qstring_new();
+            g_string_truncate(mon->outbuf, 0);
             return;
         }
         if (rc > 0) {
             /* partial write */
-            QString *tmp = qstring_from_str(buf + rc);
-            qobject_unref(mon->outbuf);
-            mon->outbuf = tmp;
+            g_string_erase(mon->outbuf, 0, rc);
         }
         if (mon->out_watch == 0) {
             mon->out_watch =
@@ -223,9 +219,9 @@ int monitor_puts(Monitor *mon, const char *str)
     for (i = 0; str[i]; i++) {
         c = str[i];
         if (c == '\n') {
-            qstring_append_chr(mon->outbuf, '\r');
+            g_string_append_c(mon->outbuf, '\r');
         }
-        qstring_append_chr(mon->outbuf, c);
+        g_string_append_c(mon->outbuf, c);
         if (c == '\n') {
             monitor_flush_locked(mon);
         }
@@ -602,7 +598,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
     }
     qemu_mutex_init(&mon->mon_lock);
     mon->is_qmp = is_qmp;
-    mon->outbuf = qstring_new();
+    mon->outbuf = g_string_new(NULL);
     mon->skip_flush = skip_flush;
     mon->use_io_thread = use_io_thread;
 }
@@ -616,7 +612,7 @@ void monitor_data_destroy(Monitor *mon)
     } else {
         readline_free(container_of(mon, MonitorHMP, common)->rs);
     }
-    qobject_unref(mon->outbuf);
+    g_string_free(mon->outbuf, true);
     qemu_mutex_destroy(&mon->mon_lock);
 }
 
-- 
2.26.2



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

* [PATCH 03/20] qobject: Make qobject_to_json_pretty() take a pretty argument
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
  2020-12-11 17:11 ` [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output Markus Armbruster
  2020-12-11 17:11 ` [PATCH 02/20] monitor: Use GString instead of QString for output buffer Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 04/20] qobject: Use GString instead of QString to accumulate JSON Markus Armbruster
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qjson.h |  2 +-
 monitor/qmp.c            |  2 +-
 qemu-img.c               |  8 ++++----
 qobject/qjson.c          | 28 +++++++++++-----------------
 qom/qom-hmp-cmds.c       |  2 +-
 tests/qtest/libqtest.c   |  2 +-
 6 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h
index 5ebbe5a118..82f4534f16 100644
--- a/include/qapi/qmp/qjson.h
+++ b/include/qapi/qmp/qjson.h
@@ -26,6 +26,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
     GCC_FMT_ATTR(1, 2);
 
 QString *qobject_to_json(const QObject *obj);
-QString *qobject_to_json_pretty(const QObject *obj);
+QString *qobject_to_json_pretty(const QObject *obj, bool pretty);
 
 #endif /* QJSON_H */
diff --git a/monitor/qmp.c b/monitor/qmp.c
index b42f8c6af3..1197c50b20 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -112,7 +112,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
     const QObject *data = QOBJECT(rsp);
     QString *json;
 
-    json = mon->pretty ? qobject_to_json_pretty(data) : qobject_to_json(data);
+    json = qobject_to_json_pretty(data, mon->pretty);
     assert(json != NULL);
 
     qstring_append_chr(json, '\n');
diff --git a/qemu-img.c b/qemu-img.c
index 8bdea40b58..59ccd4fdd2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -633,7 +633,7 @@ static void dump_json_image_check(ImageCheck *check, bool quiet)
 
     visit_type_ImageCheck(v, NULL, &check, &error_abort);
     visit_complete(v, &obj);
-    str = qobject_to_json_pretty(obj);
+    str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
     qprintf(quiet, "%s\n", qstring_get_str(str));
     qobject_unref(obj);
@@ -2796,7 +2796,7 @@ static void dump_json_image_info_list(ImageInfoList *list)
 
     visit_type_ImageInfoList(v, NULL, &list, &error_abort);
     visit_complete(v, &obj);
-    str = qobject_to_json_pretty(obj);
+    str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_unref(obj);
@@ -2812,7 +2812,7 @@ static void dump_json_image_info(ImageInfo *info)
 
     visit_type_ImageInfo(v, NULL, &info, &error_abort);
     visit_complete(v, &obj);
-    str = qobject_to_json_pretty(obj);
+    str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_unref(obj);
@@ -5243,7 +5243,7 @@ static void dump_json_block_measure_info(BlockMeasureInfo *info)
 
     visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort);
     visit_complete(v, &obj);
-    str = qobject_to_json_pretty(obj);
+    str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
     printf("%s\n", qstring_get_str(str));
     qobject_unref(obj);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f1f2c69704..523a4ab8de 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,8 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
     return qdict;
 }
 
-static void to_json(const QObject *obj, QString *str, int pretty, int indent);
-
 static void json_pretty_newline(QString *str, bool pretty, int indent)
 {
     int i;
@@ -163,7 +161,7 @@ static void json_pretty_newline(QString *str, bool pretty, int indent)
     }
 }
 
-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)
 {
     switch (qobject_type(obj)) {
     case QTYPE_QNULL:
@@ -294,20 +292,16 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
     }
 }
 
+QString *qobject_to_json_pretty(const QObject *obj, bool pretty)
+{
+    QString *str = qstring_new();
+
+    to_json(obj, str, pretty, 0);
+
+    return str;
+}
+
 QString *qobject_to_json(const QObject *obj)
 {
-    QString *str = qstring_new();
-
-    to_json(obj, str, 0, 0);
-
-    return str;
-}
-
-QString *qobject_to_json_pretty(const QObject *obj)
-{
-    QString *str = qstring_new();
-
-    to_json(obj, str, 1, 0);
-
-    return str;
+    return qobject_to_json_pretty(obj, false);
 }
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 8861a109d5..6b96dbe906 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -78,7 +78,7 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict)
     QObject *obj = qmp_qom_get(path, property, &err);
 
     if (err == NULL) {
-        QString *str = qobject_to_json_pretty(obj);
+        QString *str = qobject_to_json_pretty(obj, true);
         monitor_printf(mon, "%s\n", qstring_get_str(str));
         qobject_unref(str);
     }
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index e49f3a1e45..213fa4f8fe 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1197,7 +1197,7 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
 
     g_assert(response);
     if (!qdict_haskey(response, "return")) {
-        QString *s = qobject_to_json_pretty(QOBJECT(response));
+        QString *s = qobject_to_json_pretty(QOBJECT(response), true);
         g_test_message("%s", qstring_get_str(s));
         qobject_unref(s);
     }
-- 
2.26.2



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

* [PATCH 04/20] qobject: Use GString instead of QString to accumulate JSON
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (2 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 03/20] qobject: Make qobject_to_json_pretty() take a pretty argument Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString Markus Armbruster
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Use of GString for building the initial string is actually more
convenient here.  Change qobject_to_json() & friends to do that.

Once all such uses are replaced this way, QString can become immutable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 +
 qobject/qjson.c            | 85 +++++++++++++++++---------------------
 qobject/qstring.c          | 19 +++++++++
 3 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index e2e356e5e7..ae7698d6c7 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -25,6 +25,7 @@ struct QString {
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, size_t start, size_t end);
+QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 const char *qstring_get_try_str(const QString *qstring);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 523a4ab8de..e7100a539c 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,28 +149,23 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
     return qdict;
 }
 
-static void json_pretty_newline(QString *str, bool pretty, int indent)
+static void json_pretty_newline(GString *accu, bool pretty, int indent)
 {
-    int i;
-
     if (pretty) {
-        qstring_append(str, "\n");
-        for (i = 0; i < indent; i++) {
-            qstring_append(str, "    ");
-        }
+        g_string_append_printf(accu, "\n%*s", indent * 4, "");
     }
 }
 
-static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
+static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
 {
     switch (qobject_type(obj)) {
     case QTYPE_QNULL:
-        qstring_append(str, "null");
+        g_string_append(accu, "null");
         break;
     case QTYPE_QNUM: {
         QNum *val = qobject_to(QNum, obj);
         char *buffer = qnum_to_string(val);
-        qstring_append(str, buffer);
+        g_string_append(accu, buffer);
         g_free(buffer);
         break;
     }
@@ -178,35 +173,34 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
         QString *val = qobject_to(QString, obj);
         const char *ptr;
         int cp;
-        char buf[16];
         char *end;
 
         ptr = qstring_get_str(val);
-        qstring_append(str, "\"");
+        g_string_append_c(accu, '"');
 
         for (; *ptr; ptr = end) {
             cp = mod_utf8_codepoint(ptr, 6, &end);
             switch (cp) {
             case '\"':
-                qstring_append(str, "\\\"");
+                g_string_append(accu, "\\\"");
                 break;
             case '\\':
-                qstring_append(str, "\\\\");
+                g_string_append(accu, "\\\\");
                 break;
             case '\b':
-                qstring_append(str, "\\b");
+                g_string_append(accu, "\\b");
                 break;
             case '\f':
-                qstring_append(str, "\\f");
+                g_string_append(accu, "\\f");
                 break;
             case '\n':
-                qstring_append(str, "\\n");
+                g_string_append(accu, "\\n");
                 break;
             case '\r':
-                qstring_append(str, "\\r");
+                g_string_append(accu, "\\r");
                 break;
             case '\t':
-                qstring_append(str, "\\t");
+                g_string_append(accu, "\\t");
                 break;
             default:
                 if (cp < 0) {
@@ -214,20 +208,18 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
                 }
                 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));
+                    g_string_append_printf(accu, "\\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);
+                    g_string_append_printf(accu, "\\u%04X", cp);
                 } else {
-                    buf[0] = cp;
-                    buf[1] = 0;
+                    g_string_append_c(accu, cp);
                 }
-                qstring_append(str, buf);
             }
         };
 
-        qstring_append(str, "\"");
+        g_string_append_c(accu, '"');
         break;
     }
     case QTYPE_QDICT: {
@@ -237,25 +229,25 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
         const QDictEntry *entry;
         QString *qkey;
 
-        qstring_append(str, "{");
+        g_string_append_c(accu, '{');
 
         for (entry = qdict_first(val);
              entry;
              entry = qdict_next(val, entry)) {
-            qstring_append(str, sep);
-            json_pretty_newline(str, pretty, indent + 1);
+            g_string_append(accu, sep);
+            json_pretty_newline(accu, pretty, indent + 1);
 
             qkey = qstring_from_str(qdict_entry_key(entry));
-            to_json(QOBJECT(qkey), str, pretty, indent + 1);
+            to_json(QOBJECT(qkey), accu, pretty, indent + 1);
             qobject_unref(qkey);
 
-            qstring_append(str, ": ");
-            to_json(qdict_entry_value(entry), str, pretty, indent + 1);
+            g_string_append(accu, ": ");
+            to_json(qdict_entry_value(entry), accu, pretty, indent + 1);
             sep = comma;
         }
 
-        json_pretty_newline(str, pretty, indent);
-        qstring_append(str, "}");
+        json_pretty_newline(accu, pretty, indent);
+        g_string_append_c(accu, '}');
         break;
     }
     case QTYPE_QLIST: {
@@ -264,26 +256,26 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
         const char *sep = "";
         QListEntry *entry;
 
-        qstring_append(str, "[");
+        g_string_append_c(accu, '[');
 
         QLIST_FOREACH_ENTRY(val, entry) {
-            qstring_append(str, sep);
-            json_pretty_newline(str, pretty, indent + 1);
-            to_json(qlist_entry_obj(entry), str, pretty, indent + 1);
+            g_string_append(accu, sep);
+            json_pretty_newline(accu, pretty, indent + 1);
+            to_json(qlist_entry_obj(entry), accu, pretty, indent + 1);
             sep = comma;
         }
 
-        json_pretty_newline(str, pretty, indent);
-        qstring_append(str, "]");
+        json_pretty_newline(accu, pretty, indent);
+        g_string_append_c(accu, ']');
         break;
     }
     case QTYPE_QBOOL: {
         QBool *val = qobject_to(QBool, obj);
 
         if (qbool_get_bool(val)) {
-            qstring_append(str, "true");
+            g_string_append(accu, "true");
         } else {
-            qstring_append(str, "false");
+            g_string_append(accu, "false");
         }
         break;
     }
@@ -294,11 +286,10 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
 
 QString *qobject_to_json_pretty(const QObject *obj, bool pretty)
 {
-    QString *str = qstring_new();
+    GString *accu = g_string_new(NULL);
 
-    to_json(obj, str, pretty, 0);
-
-    return str;
+    to_json(obj, accu, pretty, 0);
+    return qstring_from_gstring(accu);
 }
 
 QString *qobject_to_json(const QObject *obj)
diff --git a/qobject/qstring.c b/qobject/qstring.c
index b66a2c35f2..af7c18ca73 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -66,6 +66,25 @@ QString *qstring_from_str(const char *str)
     return qstring_from_substr(str, 0, strlen(str));
 }
 
+/**
+ * qstring_from_gstring(): Convert a GString to a QString
+ *
+ * Return strong reference.
+ */
+
+QString *qstring_from_gstring(GString *gstr)
+{
+    QString *qstring;
+
+    qstring = g_malloc(sizeof(*qstring));
+    qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
+    qstring->length = gstr->len;
+    qstring->capacity = gstr->allocated_len;
+    qstring->string = g_string_free(gstr, false);
+    return qstring;
+}
+
+
 static void capacity_increase(QString *qstring, size_t len)
 {
     if (qstring->capacity < (qstring->length + len)) {
-- 
2.26.2



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

* [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (3 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 04/20] qobject: Use GString instead of QString to accumulate JSON Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2021-03-24  7:16   ` Thomas Huth
  2020-12-11 17:11 ` [PATCH 06/20] Revert "qstring: add qstring_free()" Markus Armbruster
                   ` (15 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

qobject_to_json() and qobject_to_json_pretty() build a GString, then
covert it to QString.  Just one of the callers actually needs a
QString: qemu_rbd_parse_filename().  A few others need a string they
can modify: qmp_send_response(), qga's send_response(), to_json_str(),
and qmp_fd_vsend_fds().  The remainder just need a string.

Change qobject_to_json() and qobject_to_json_pretty() to return the
GString.

qemu_rbd_parse_filename() now has to convert to QString.  All others
save a QString temporary.  to_json_str() actually becomes a bit
simpler, because GString provides more convenient modification
functions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qjson.h           |  4 +--
 block.c                            |  6 ++--
 block/rbd.c                        |  2 +-
 monitor/qmp.c                      | 14 ++++----
 qemu-img.c                         | 25 +++++++------
 qga/main.c                         | 22 ++++--------
 qobject/qjson.c                    |  6 ++--
 qom/object_interfaces.c            |  4 +--
 qom/qom-hmp-cmds.c                 |  7 ++--
 tests/check-qjson.c                | 56 ++++++++++++++----------------
 tests/qtest/libqtest.c             | 20 +++++------
 tests/test-visitor-serialization.c |  6 ++--
 12 files changed, 79 insertions(+), 93 deletions(-)

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h
index 82f4534f16..593b40b4e0 100644
--- a/include/qapi/qmp/qjson.h
+++ b/include/qapi/qmp/qjson.h
@@ -25,7 +25,7 @@ QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap)
 QDict *qdict_from_jsonf_nofail(const char *string, ...)
     GCC_FMT_ATTR(1, 2);
 
-QString *qobject_to_json(const QObject *obj);
-QString *qobject_to_json_pretty(const QObject *obj, bool pretty);
+GString *qobject_to_json(const QObject *obj);
+GString *qobject_to_json_pretty(const QObject *obj, bool pretty);
 
 #endif /* QJSON_H */
diff --git a/block.c b/block.c
index f1cedac362..487b2b1497 100644
--- a/block.c
+++ b/block.c
@@ -6979,13 +6979,13 @@ void bdrv_refresh_filename(BlockDriverState *bs)
     if (bs->exact_filename[0]) {
         pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
     } else {
-        QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
+        GString *json = qobject_to_json(QOBJECT(bs->full_open_options));
         if (snprintf(bs->filename, sizeof(bs->filename), "json:%s",
-                     qstring_get_str(json)) >= sizeof(bs->filename)) {
+                     json->str) >= sizeof(bs->filename)) {
             /* Give user a hint if we truncated things. */
             strcpy(bs->filename + sizeof(bs->filename) - 4, "...");
         }
-        qobject_unref(json);
+        g_string_free(json, true);
     }
 }
 
diff --git a/block/rbd.c b/block/rbd.c
index 9bd2bce716..9071a00e3f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -232,7 +232,7 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
 
     if (keypairs) {
         qdict_put(options, "=keyvalue-pairs",
-                  qobject_to_json(QOBJECT(keypairs)));
+                  qstring_from_gstring(qobject_to_json(QOBJECT(keypairs))));
     }
 
 done:
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 1197c50b20..374bb4b81c 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -110,15 +110,15 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
 void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
 {
     const QObject *data = QOBJECT(rsp);
-    QString *json;
+    GString *json;
 
     json = qobject_to_json_pretty(data, mon->pretty);
     assert(json != NULL);
 
-    qstring_append_chr(json, '\n');
-    monitor_puts(&mon->common, qstring_get_str(json));
+    g_string_append_c(json, '\n');
+    monitor_puts(&mon->common, json->str);
 
-    qobject_unref(json);
+    g_string_free(json, true);
 }
 
 /*
@@ -320,9 +320,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
     } /* else will fail qmp_dispatch() */
 
     if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
-        QString *req_json = qobject_to_json(req);
-        trace_handle_qmp_command(mon, qstring_get_str(req_json));
-        qobject_unref(req_json);
+        GString *req_json = qobject_to_json(req);
+        trace_handle_qmp_command(mon, req_json->str);
+        g_string_free(req_json, true);
     }
 
     if (qdict && qmp_is_oob(qdict)) {
diff --git a/qemu-img.c b/qemu-img.c
index 59ccd4fdd2..b8ac55a57d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -33,7 +33,6 @@
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qstring.h"
 #include "qemu/cutils.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
@@ -627,7 +626,7 @@ fail:
 
 static void dump_json_image_check(ImageCheck *check, bool quiet)
 {
-    QString *str;
+    GString *str;
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
 
@@ -635,10 +634,10 @@ static void dump_json_image_check(ImageCheck *check, bool quiet)
     visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
-    qprintf(quiet, "%s\n", qstring_get_str(str));
+    qprintf(quiet, "%s\n", str->str);
     qobject_unref(obj);
     visit_free(v);
-    qobject_unref(str);
+    g_string_free(str, true);
 }
 
 static void dump_human_image_check(ImageCheck *check, bool quiet)
@@ -2790,7 +2789,7 @@ static void dump_snapshots(BlockDriverState *bs)
 
 static void dump_json_image_info_list(ImageInfoList *list)
 {
-    QString *str;
+    GString *str;
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
 
@@ -2798,15 +2797,15 @@ static void dump_json_image_info_list(ImageInfoList *list)
     visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
-    printf("%s\n", qstring_get_str(str));
+    printf("%s\n", str->str);
     qobject_unref(obj);
     visit_free(v);
-    qobject_unref(str);
+    g_string_free(str, true);
 }
 
 static void dump_json_image_info(ImageInfo *info)
 {
-    QString *str;
+    GString *str;
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
 
@@ -2814,10 +2813,10 @@ static void dump_json_image_info(ImageInfo *info)
     visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
-    printf("%s\n", qstring_get_str(str));
+    printf("%s\n", str->str);
     qobject_unref(obj);
     visit_free(v);
-    qobject_unref(str);
+    g_string_free(str, true);
 }
 
 static void dump_human_image_info_list(ImageInfoList *list)
@@ -5237,7 +5236,7 @@ out:
 
 static void dump_json_block_measure_info(BlockMeasureInfo *info)
 {
-    QString *str;
+    GString *str;
     QObject *obj;
     Visitor *v = qobject_output_visitor_new(&obj);
 
@@ -5245,10 +5244,10 @@ static void dump_json_block_measure_info(BlockMeasureInfo *info)
     visit_complete(v, &obj);
     str = qobject_to_json_pretty(obj, true);
     assert(str != NULL);
-    printf("%s\n", qstring_get_str(str));
+    printf("%s\n", str->str);
     qobject_unref(obj);
     visit_free(v);
-    qobject_unref(str);
+    g_string_free(str, true);
 }
 
 static int img_measure(int argc, char **argv)
diff --git a/qga/main.c b/qga/main.c
index dea6a3aa64..e7f8f3b161 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -22,7 +22,6 @@
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qstring.h"
 #include "guest-agent-core.h"
 #include "qga-qapi-init-commands.h"
 #include "qapi/qmp/qerror.h"
@@ -528,8 +527,7 @@ fail:
 
 static int send_response(GAState *s, const QDict *rsp)
 {
-    const char *buf;
-    QString *payload_qstr, *response_qstr;
+    GString *response;
     GIOStatus status;
 
     g_assert(s->channel);
@@ -538,25 +536,19 @@ static int send_response(GAState *s, const QDict *rsp)
         return 0;
     }
 
-    payload_qstr = qobject_to_json(QOBJECT(rsp));
-    if (!payload_qstr) {
+    response = qobject_to_json(QOBJECT(rsp));
+    if (!response) {
         return -EINVAL;
     }
 
     if (s->delimit_response) {
         s->delimit_response = false;
-        response_qstr = qstring_new();
-        qstring_append_chr(response_qstr, QGA_SENTINEL_BYTE);
-        qstring_append(response_qstr, qstring_get_str(payload_qstr));
-        qobject_unref(payload_qstr);
-    } else {
-        response_qstr = payload_qstr;
+        g_string_prepend_c(response, QGA_SENTINEL_BYTE);
     }
 
-    qstring_append_chr(response_qstr, '\n');
-    buf = qstring_get_str(response_qstr);
-    status = ga_channel_write_all(s->channel, buf, strlen(buf));
-    qobject_unref(response_qstr);
+    g_string_append_c(response, '\n');
+    status = ga_channel_write_all(s->channel, response->str, response->len);
+    g_string_free(response, true);
     if (status != G_IO_STATUS_NORMAL) {
         return -EIO;
     }
diff --git a/qobject/qjson.c b/qobject/qjson.c
index e7100a539c..2f690c1816 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -284,15 +284,15 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
     }
 }
 
-QString *qobject_to_json_pretty(const QObject *obj, bool pretty)
+GString *qobject_to_json_pretty(const QObject *obj, bool pretty)
 {
     GString *accu = g_string_new(NULL);
 
     to_json(obj, accu, pretty, 0);
-    return qstring_from_gstring(accu);
+    return accu;
 }
 
-QString *qobject_to_json(const QObject *obj)
+GString *qobject_to_json(const QObject *obj)
 {
     return qobject_to_json_pretty(obj, false);
 }
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index ed896fe764..1e9ad6f08a 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -5,7 +5,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qstring.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qom/object_interfaces.h"
 #include "qemu/help_option.h"
@@ -207,7 +206,8 @@ char *object_property_help(const char *name, const char *type,
         g_string_append(str, description);
     }
     if (defval) {
-        g_autofree char *def_json = qstring_free(qobject_to_json(defval), TRUE);
+        g_autofree char *def_json = g_string_free(qobject_to_json(defval),
+                                                  true);
         g_string_append_printf(str, " (default: %s)", def_json);
     }
 
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 6b96dbe906..453fbfeabc 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -13,7 +13,6 @@
 #include "qapi/qapi-commands-qom.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qstring.h"
 #include "qom/object.h"
 
 void hmp_qom_list(Monitor *mon, const QDict *qdict)
@@ -78,9 +77,9 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict)
     QObject *obj = qmp_qom_get(path, property, &err);
 
     if (err == NULL) {
-        QString *str = qobject_to_json_pretty(obj, true);
-        monitor_printf(mon, "%s\n", qstring_get_str(str));
-        qobject_unref(str);
+        GString *str = qobject_to_json_pretty(obj, true);
+        monitor_printf(mon, "%s\n", str->str);
+        g_string_free(str, true);
     }
 
     qobject_unref(obj);
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index ca8fb816e9..337add0838 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -35,17 +35,15 @@ static QString *from_json_str(const char *jstr, bool single, Error **errp)
 
 static char *to_json_str(QString *str)
 {
-    QString *json = qobject_to_json(QOBJECT(str));
-    char *jstr;
+    GString *json = qobject_to_json(QOBJECT(str));
 
     if (!json) {
         return NULL;
     }
     /* peel off double quotes */
-    jstr = g_strndup(qstring_get_str(json) + 1,
-                     qstring_get_length(json) - 2);
-    qobject_unref(json);
-    return jstr;
+    g_string_truncate(json, json->len - 1);
+    g_string_erase(json, 0, 1);
+    return g_string_free(json, false);
 }
 
 static void escaped_string(void)
@@ -809,7 +807,7 @@ static void int_number(void)
     QNum *qnum;
     int64_t ival;
     uint64_t uval;
-    QString *str;
+    GString *str;
 
     for (i = 0; test_cases[i].encoded; i++) {
         qnum = qobject_to(QNum,
@@ -828,9 +826,9 @@ static void int_number(void)
                           (double)test_cases[i].decoded);
 
         str = qobject_to_json(QOBJECT(qnum));
-        g_assert_cmpstr(qstring_get_str(str), ==,
+        g_assert_cmpstr(str->str, ==,
                         test_cases[i].reencoded ?: test_cases[i].encoded);
-        qobject_unref(str);
+        g_string_free(str, true);
 
         qobject_unref(qnum);
     }
@@ -851,7 +849,7 @@ static void uint_number(void)
     QNum *qnum;
     int64_t ival;
     uint64_t uval;
-    QString *str;
+    GString *str;
 
     for (i = 0; test_cases[i].encoded; i++) {
         qnum = qobject_to(QNum,
@@ -865,9 +863,9 @@ static void uint_number(void)
                           (double)test_cases[i].decoded);
 
         str = qobject_to_json(QOBJECT(qnum));
-        g_assert_cmpstr(qstring_get_str(str), ==,
+        g_assert_cmpstr(str->str, ==,
                         test_cases[i].reencoded ?: test_cases[i].encoded);
-        qobject_unref(str);
+        g_string_free(str, true);
 
         qobject_unref(qnum);
     }
@@ -892,7 +890,7 @@ static void float_number(void)
     QNum *qnum;
     int64_t ival;
     uint64_t uval;
-    QString *str;
+    GString *str;
 
     for (i = 0; test_cases[i].encoded; i++) {
         qnum = qobject_to(QNum,
@@ -904,9 +902,9 @@ static void float_number(void)
         g_assert(!qnum_get_try_uint(qnum, &uval));
 
         str = qobject_to_json(QOBJECT(qnum));
-        g_assert_cmpstr(qstring_get_str(str), ==,
+        g_assert_cmpstr(str->str, ==,
                         test_cases[i].reencoded ?: test_cases[i].encoded);
-        qobject_unref(str);
+        g_string_free(str, true);
 
         qobject_unref(qnum);
     }
@@ -917,7 +915,7 @@ static void keyword_literal(void)
     QObject *obj;
     QBool *qbool;
     QNull *null;
-    QString *str;
+    GString *str;
 
     obj = qobject_from_json("true", &error_abort);
     qbool = qobject_to(QBool, obj);
@@ -925,8 +923,8 @@ static void keyword_literal(void)
     g_assert(qbool_get_bool(qbool) == true);
 
     str = qobject_to_json(obj);
-    g_assert(strcmp(qstring_get_str(str), "true") == 0);
-    qobject_unref(str);
+    g_assert_cmpstr(str->str, ==, "true");
+    g_string_free(str, true);
 
     qobject_unref(qbool);
 
@@ -936,8 +934,8 @@ static void keyword_literal(void)
     g_assert(qbool_get_bool(qbool) == false);
 
     str = qobject_to_json(obj);
-    g_assert(strcmp(qstring_get_str(str), "false") == 0);
-    qobject_unref(str);
+    g_assert_cmpstr(str->str, ==, "false");
+    g_string_free(str, true);
 
     qobject_unref(qbool);
 
@@ -1087,7 +1085,7 @@ static void simple_dict(void)
 
     for (i = 0; test_cases[i].encoded; i++) {
         QObject *obj;
-        QString *str;
+        GString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
@@ -1095,10 +1093,10 @@ static void simple_dict(void)
         str = qobject_to_json(obj);
         qobject_unref(obj);
 
-        obj = qobject_from_json(qstring_get_str(str), &error_abort);
+        obj = qobject_from_json(str->str, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_unref(obj);
-        qobject_unref(str);
+        g_string_free(str, true);
     }
 }
 
@@ -1196,7 +1194,7 @@ static void simple_list(void)
 
     for (i = 0; test_cases[i].encoded; i++) {
         QObject *obj;
-        QString *str;
+        GString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
@@ -1204,10 +1202,10 @@ static void simple_list(void)
         str = qobject_to_json(obj);
         qobject_unref(obj);
 
-        obj = qobject_from_json(qstring_get_str(str), &error_abort);
+        obj = qobject_from_json(str->str, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_unref(obj);
-        qobject_unref(str);
+        g_string_free(str, true);
     }
 }
 
@@ -1258,7 +1256,7 @@ static void simple_whitespace(void)
 
     for (i = 0; test_cases[i].encoded; i++) {
         QObject *obj;
-        QString *str;
+        GString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
@@ -1266,11 +1264,11 @@ static void simple_whitespace(void)
         str = qobject_to_json(obj);
         qobject_unref(obj);
 
-        obj = qobject_from_json(qstring_get_str(str), &error_abort);
+        obj = qobject_from_json(str->str, &error_abort);
         g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         qobject_unref(obj);
-        qobject_unref(str);
+        g_string_free(str, true);
     }
 }
 
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 213fa4f8fe..8e93b0a707 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -652,27 +652,25 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
     /* No need to send anything for an empty QObject.  */
     if (qobj) {
         int log = getenv("QTEST_LOG") != NULL;
-        QString *qstr = qobject_to_json(qobj);
-        const char *str;
+        GString *str = qobject_to_json(qobj);
 
         /*
          * BUG: QMP doesn't react to input until it sees a newline, an
          * object, or an array.  Work-around: give it a newline.
          */
-        qstring_append_chr(qstr, '\n');
-        str = qstring_get_str(qstr);
+        g_string_append_c(str, '\n');
 
         if (log) {
-            fprintf(stderr, "%s", str);
+            fprintf(stderr, "%s", str->str);
         }
         /* Send QMP request */
         if (fds && fds_num > 0) {
-            socket_send_fds(fd, fds, fds_num, str, qstring_get_length(qstr));
+            socket_send_fds(fd, fds, fds_num, str->str, str->len);
         } else {
-            socket_send(fd, str, qstring_get_length(qstr));
+            socket_send(fd, str->str, str->len);
         }
 
-        qobject_unref(qstr);
+        g_string_free(str, true);
         qobject_unref(qobj);
     }
 }
@@ -1197,9 +1195,9 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
 
     g_assert(response);
     if (!qdict_haskey(response, "return")) {
-        QString *s = qobject_to_json_pretty(QOBJECT(response), true);
-        g_test_message("%s", qstring_get_str(s));
-        qobject_unref(s);
+        GString *s = qobject_to_json_pretty(QOBJECT(response), true);
+        g_test_message("%s", s->str);
+        g_string_free(s, true);
     }
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 7eab18b7c6..bb387fa6e0 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1044,15 +1044,15 @@ static void qmp_deserialize(void **native_out, void *datap,
                             VisitorFunc visit, Error **errp)
 {
     QmpSerializeData *d = datap;
-    QString *output_json;
+    GString *output_json;
     QObject *obj_orig, *obj;
 
     visit_complete(d->qov, &d->obj);
     obj_orig = d->obj;
     output_json = qobject_to_json(obj_orig);
-    obj = qobject_from_json(qstring_get_str(output_json), &error_abort);
+    obj = qobject_from_json(output_json->str, &error_abort);
 
-    qobject_unref(output_json);
+    g_string_free(output_json, true);
     d->qiv = qobject_input_visitor_new(obj);
     qobject_unref(obj_orig);
     qobject_unref(obj);
-- 
2.26.2



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

* [PATCH 06/20] Revert "qstring: add qstring_free()"
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (4 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 07/20] hw/rdma: Replace QList by GQueue Markus Armbruster
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

This reverts commit 164c374b75f87c6765a705c4418ab7005a2d356f.

A free function for a reference-counted object is in bad taste.
Fortunately, this one is now also unused.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c          | 27 +++++----------------------
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index ae7698d6c7..ae5b4b44d2 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -34,7 +34,6 @@ 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);
 bool qstring_is_equal(const QObject *x, const QObject *y);
-char *qstring_free(QString *qstring, bool return_str);
 void qstring_destroy_obj(QObject *obj);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qstring.c b/qobject/qstring.c
index af7c18ca73..c1891beda0 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -168,33 +168,16 @@ bool qstring_is_equal(const QObject *x, const QObject *y)
                    qobject_to(QString, y)->string);
 }
 
-/**
- * qstring_free(): Free the memory allocated by a QString object
- *
- * Return: if @return_str, return the underlying string, to be
- * g_free(), otherwise NULL is returned.
- */
-char *qstring_free(QString *qstring, bool return_str)
-{
-    char *rv = NULL;
-
-    if (return_str) {
-        rv = qstring->string;
-    } else {
-        g_free(qstring->string);
-    }
-
-    g_free(qstring);
-
-    return rv;
-}
-
 /**
  * qstring_destroy_obj(): Free all memory allocated by a QString
  * object
  */
 void qstring_destroy_obj(QObject *obj)
 {
+    QString *qs;
+
     assert(obj != NULL);
-    qstring_free(qobject_to(QString, obj), FALSE);
+    qs = qobject_to(QString, obj);
+    g_free(qs->string);
+    g_free(qs);
 }
-- 
2.26.2



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

* [PATCH 07/20] hw/rdma: Replace QList by GQueue
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (5 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 06/20] Revert "qstring: add qstring_free()" Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 08/20] qobject: Move internals to qobject-internal.h Markus Armbruster
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, Yuval Shaia

RdmaProtectedQList provides a thread-safe queue of int64_t on top of a
QList.

rdma_protected_qlist_destroy() calls qlist_destroy_obj() directly.
qlist_destroy_obj() is actually for use by qobject_destroy() only.
The next commit will make that obvious.

The minimal fix would be calling qobject_unref() instead.  But QList
is actually a bad fit here.  It's designed for representing JSON
arrays.  We're better off with a GQueue here.  Replace.

Cc: Yuval Shaia <yuval.shaia.ml@gmail.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/rdma/rdma_backend_defs.h |  2 +-
 hw/rdma/rdma_utils.h        | 15 ++++++++-------
 hw/rdma/rdma_backend.c      | 10 +++++-----
 hw/rdma/rdma_utils.c        | 29 ++++++++++++++++-------------
 4 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
index 0b55be3503..4e6c0ad695 100644
--- a/hw/rdma/rdma_backend_defs.h
+++ b/hw/rdma/rdma_backend_defs.h
@@ -43,7 +43,7 @@ typedef struct RdmaBackendDev {
     struct ibv_context *context;
     struct ibv_comp_channel *channel;
     uint8_t port_num;
-    RdmaProtectedQList recv_mads_list;
+    RdmaProtectedGQueue recv_mads_list;
     RdmaCmMux rdmacm_mux;
 } RdmaBackendDev;
 
diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index e7babe96cb..9fd0efd940 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -28,10 +28,10 @@
 #define rdma_info_report(fmt, ...) \
     info_report("%s: " fmt, "rdma", ## __VA_ARGS__)
 
-typedef struct RdmaProtectedQList {
+typedef struct RdmaProtectedGQueue {
     QemuMutex lock;
-    QList *list;
-} RdmaProtectedQList;
+    GQueue *list;
+} RdmaProtectedGQueue;
 
 typedef struct RdmaProtectedGSList {
     QemuMutex lock;
@@ -40,10 +40,11 @@ typedef struct RdmaProtectedGSList {
 
 void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen);
 void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
-void rdma_protected_qlist_init(RdmaProtectedQList *list);
-void rdma_protected_qlist_destroy(RdmaProtectedQList *list);
-void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t value);
-int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list);
+void rdma_protected_gqueue_init(RdmaProtectedGQueue *list);
+void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list);
+void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list,
+                                        int64_t value);
+int64_t rdma_protected_gqueue_pop_int64(RdmaProtectedGQueue *list);
 void rdma_protected_gslist_init(RdmaProtectedGSList *list);
 void rdma_protected_gslist_destroy(RdmaProtectedGSList *list);
 void rdma_protected_gslist_append_int32(RdmaProtectedGSList *list,
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 5de010b1fa..6dcdfbbbe2 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -78,7 +78,7 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev)
     unsigned long cqe_ctx_id;
 
     do {
-        cqe_ctx_id = rdma_protected_qlist_pop_int64(&backend_dev->
+        cqe_ctx_id = rdma_protected_gqueue_pop_int64(&backend_dev->
                                                     recv_mads_list);
         if (cqe_ctx_id != -ENOENT) {
             qatomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe);
@@ -597,7 +597,7 @@ static unsigned int save_mad_recv_buffer(RdmaBackendDev *backend_dev,
     bctx->up_ctx = ctx;
     bctx->sge = *sge;
 
-    rdma_protected_qlist_append_int64(&backend_dev->recv_mads_list, bctx_id);
+    rdma_protected_gqueue_append_int64(&backend_dev->recv_mads_list, bctx_id);
 
     return 0;
 }
@@ -1111,7 +1111,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
 
     trace_mad_message("recv", msg->umad.mad, msg->umad_len);
 
-    cqe_ctx_id = rdma_protected_qlist_pop_int64(&backend_dev->recv_mads_list);
+    cqe_ctx_id = rdma_protected_gqueue_pop_int64(&backend_dev->recv_mads_list);
     if (cqe_ctx_id == -ENOENT) {
         rdma_warn_report("No more free MADs buffers, waiting for a while");
         sleep(THR_POLL_TO);
@@ -1185,7 +1185,7 @@ static int mad_init(RdmaBackendDev *backend_dev, CharBackend *mad_chr_be)
         return -EIO;
     }
 
-    rdma_protected_qlist_init(&backend_dev->recv_mads_list);
+    rdma_protected_gqueue_init(&backend_dev->recv_mads_list);
 
     enable_rdmacm_mux_async(backend_dev);
 
@@ -1205,7 +1205,7 @@ static void mad_fini(RdmaBackendDev *backend_dev)
 {
     disable_rdmacm_mux_async(backend_dev);
     qemu_chr_fe_disconnect(backend_dev->rdmacm_mux.chr_be);
-    rdma_protected_qlist_destroy(&backend_dev->recv_mads_list);
+    rdma_protected_gqueue_destroy(&backend_dev->recv_mads_list);
 }
 
 int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev,
diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index 698ed4716c..98df58f689 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -14,8 +14,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qnum.h"
 #include "trace.h"
 #include "rdma_utils.h"
 
@@ -54,41 +52,46 @@ void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len)
     }
 }
 
-void rdma_protected_qlist_init(RdmaProtectedQList *list)
+void rdma_protected_gqueue_init(RdmaProtectedGQueue *list)
 {
     qemu_mutex_init(&list->lock);
-    list->list = qlist_new();
+    list->list = g_queue_new();
 }
 
-void rdma_protected_qlist_destroy(RdmaProtectedQList *list)
+void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list)
 {
     if (list->list) {
-        qlist_destroy_obj(QOBJECT(list->list));
+        g_queue_free_full(list->list, g_free);
         qemu_mutex_destroy(&list->lock);
         list->list = NULL;
     }
 }
 
-void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t value)
+void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list,
+                                        int64_t value)
 {
     qemu_mutex_lock(&list->lock);
-    qlist_append_int(list->list, value);
+    g_queue_push_tail(list->list, g_memdup(&value, sizeof(value)));
     qemu_mutex_unlock(&list->lock);
 }
 
-int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list)
+int64_t rdma_protected_gqueue_pop_int64(RdmaProtectedGQueue *list)
 {
-    QObject *obj;
+    int64_t *valp;
+    int64_t val;
 
     qemu_mutex_lock(&list->lock);
-    obj = qlist_pop(list->list);
+
+    valp = g_queue_pop_head(list->list);
     qemu_mutex_unlock(&list->lock);
 
-    if (!obj) {
+    if (!valp) {
         return -ENOENT;
     }
 
-    return qnum_get_uint(qobject_to(QNum, obj));
+    val = *valp;
+    g_free(valp);
+    return val;
 }
 
 void rdma_protected_gslist_init(RdmaProtectedGSList *list)
-- 
2.26.2



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

* [PATCH 08/20] qobject: Move internals to qobject-internal.h
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (6 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 07/20] hw/rdma: Replace QList by GQueue Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 09/20] qmp: Fix tracing of non-string command IDs Markus Armbruster
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qbool.h   |  2 --
 include/qapi/qmp/qdict.h   |  2 --
 include/qapi/qmp/qlist.h   |  2 --
 include/qapi/qmp/qnull.h   |  2 --
 include/qapi/qmp/qnum.h    |  3 ---
 include/qapi/qmp/qobject.h |  9 +--------
 include/qapi/qmp/qstring.h |  2 --
 qobject/qobject-internal.h | 39 ++++++++++++++++++++++++++++++++++++++
 qobject/qbool.c            |  1 +
 qobject/qdict.c            |  1 +
 qobject/qlist.c            |  1 +
 qobject/qnull.c            |  1 +
 qobject/qnum.c             |  1 +
 qobject/qobject.c          |  1 +
 qobject/qstring.c          |  1 +
 15 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 qobject/qobject-internal.h

diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index 5f61e38e64..2f888d1057 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -23,7 +23,5 @@ struct QBool {
 
 QBool *qbool_from_bool(bool value);
 bool qbool_get_bool(const QBool *qb);
-bool qbool_is_equal(const QObject *x, const QObject *y);
-void qbool_destroy_obj(QObject *obj);
 
 #endif /* QBOOL_H */
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index da942347a7..9934539c1b 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -39,10 +39,8 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
 int qdict_haskey(const QDict *qdict, const char *key);
 QObject *qdict_get(const QDict *qdict, const char *key);
-bool qdict_is_equal(const QObject *x, const QObject *y);
 const QDictEntry *qdict_first(const QDict *qdict);
 const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
-void qdict_destroy_obj(QObject *obj);
 
 /* Helper to qdict_put_obj(), accepts any object */
 #define qdict_put(qdict, key, obj) \
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 595b7943e1..06e98ad5f4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -51,8 +51,6 @@ QObject *qlist_pop(QList *qlist);
 QObject *qlist_peek(QList *qlist);
 int qlist_empty(const QList *qlist);
 size_t qlist_size(const QList *qlist);
-bool qlist_is_equal(const QObject *x, const QObject *y);
-void qlist_destroy_obj(QObject *obj);
 
 static inline const QListEntry *qlist_first(const QList *qlist)
 {
diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
index c1426882c5..e84ecceedb 100644
--- a/include/qapi/qmp/qnull.h
+++ b/include/qapi/qmp/qnull.h
@@ -26,6 +26,4 @@ static inline QNull *qnull(void)
     return qobject_ref(&qnull_);
 }
 
-bool qnull_is_equal(const QObject *x, const QObject *y);
-
 #endif /* QNULL_H */
diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index bbae0a5ec8..7f84e20bfb 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -68,7 +68,4 @@ double qnum_get_double(QNum *qn);
 
 char *qnum_to_string(QNum *qn);
 
-bool qnum_is_equal(const QObject *x, const QObject *y);
-void qnum_destroy_obj(QObject *obj);
-
 #endif /* QNUM_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index fcfd549220..9003b71fd3 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -64,14 +64,6 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
 #define qobject_to(type, obj)                                       \
     ((type *)qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)))
 
-/* Initialize an object to default values */
-static inline void qobject_init(QObject *obj, QType type)
-{
-    assert(QTYPE_NONE < type && type < QTYPE__MAX);
-    obj->base.refcnt = 1;
-    obj->base.type = type;
-}
-
 static inline void qobject_ref_impl(QObject *obj)
 {
     if (obj) {
@@ -90,6 +82,7 @@ bool qobject_is_equal(const QObject *x, const QObject *y);
 
 /**
  * qobject_destroy(): Free resources used by the object
+ * For use via qobject_unref() only!
  */
 void qobject_destroy(QObject *obj);
 
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index ae5b4b44d2..e4ac761a22 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -33,7 +33,5 @@ const char *qobject_get_try_str(const QObject *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);
-bool qstring_is_equal(const QObject *x, const QObject *y);
-void qstring_destroy_obj(QObject *obj);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qobject-internal.h b/qobject/qobject-internal.h
new file mode 100644
index 0000000000..b310c8e1b5
--- /dev/null
+++ b/qobject/qobject-internal.h
@@ -0,0 +1,39 @@
+/*
+ * QObject internals
+ *
+ * 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 QOBJECT_INTERNAL_H
+#define QOBJECT_INTERNAL_H
+
+#include "qapi/qmp/qobject.h"
+
+static inline void qobject_init(QObject *obj, QType type)
+{
+    assert(QTYPE_NONE < type && type < QTYPE__MAX);
+    obj->base.refcnt = 1;
+    obj->base.type = type;
+}
+
+void qbool_destroy_obj(QObject *obj);
+bool qbool_is_equal(const QObject *x, const QObject *y);
+
+void qdict_destroy_obj(QObject *obj);
+bool qdict_is_equal(const QObject *x, const QObject *y);
+
+void qlist_destroy_obj(QObject *obj);
+bool qlist_is_equal(const QObject *x, const QObject *y);
+
+bool qnull_is_equal(const QObject *x, const QObject *y);
+
+void qnum_destroy_obj(QObject *obj);
+bool qnum_is_equal(const QObject *x, const QObject *y);
+
+void qstring_destroy_obj(QObject *obj);
+bool qstring_is_equal(const QObject *x, const QObject *y);
+
+#endif
diff --git a/qobject/qbool.c b/qobject/qbool.c
index 06dfc43498..16a600abb9 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qmp/qbool.h"
+#include "qobject-internal.h"
 
 /**
  * qbool_from_bool(): Create a new QBool from a bool
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 1079bd3f6f..d84443391e 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -16,6 +16,7 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qnull.h"
 #include "qapi/qmp/qstring.h"
+#include "qobject-internal.h"
 
 /**
  * qdict_new(): Create a new QDict
diff --git a/qobject/qlist.c b/qobject/qlist.c
index 1be95367d1..60562a1f52 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -17,6 +17,7 @@
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/queue.h"
+#include "qobject-internal.h"
 
 /**
  * qlist_new(): Create a new QList
diff --git a/qobject/qnull.c b/qobject/qnull.c
index 00870a1824..b26b368219 100644
--- a/qobject/qnull.c
+++ b/qobject/qnull.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qmp/qnull.h"
+#include "qobject-internal.h"
 
 QNull qnull_ = {
     .base = {
diff --git a/qobject/qnum.c b/qobject/qnum.c
index bf1240ecec..35ba41e61c 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -14,6 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qmp/qnum.h"
+#include "qobject-internal.h"
 
 /**
  * qnum_from_int(): Create a new QNum from an int64_t
diff --git a/qobject/qobject.c b/qobject/qobject.c
index 878dd76e79..d7077b8f2a 100644
--- a/qobject/qobject.c
+++ b/qobject/qobject.c
@@ -14,6 +14,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qstring.h"
+#include "qobject-internal.h"
 
 QEMU_BUILD_BUG_MSG(
     offsetof(QNull, base) != 0 ||
diff --git a/qobject/qstring.c b/qobject/qstring.c
index c1891beda0..d6724bd4e5 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qmp/qstring.h"
+#include "qobject-internal.h"
 
 /**
  * qstring_new(): Create a new empty QString
-- 
2.26.2



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

* [PATCH 09/20] qmp: Fix tracing of non-string command IDs
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (7 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 08/20] qobject: Move internals to qobject-internal.h Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 10/20] block: Avoid qobject_get_try_str() Markus Armbruster
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

Tracepoints monitor_qmp_cmd_in_band and
monitor_qmp_cmd_out_of_band (commit cf869d5317 "qmp: support
out-of-band (oob) execution") treat non-string "id" like absent "id".
Fix that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor/qmp.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/monitor/qmp.c b/monitor/qmp.c
index 374bb4b81c..8f91af32be 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -31,7 +31,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qstring.h"
 #include "trace.h"
 
 struct QMPRequest {
@@ -276,9 +275,15 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
             mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1;
         qemu_mutex_unlock(&mon->qmp_queue_lock);
         if (req_obj->req) {
-            QDict *qdict = qobject_to(QDict, req_obj->req);
-            QObject *id = qdict ? qdict_get(qdict, "id") : NULL;
-            trace_monitor_qmp_cmd_in_band(qobject_get_try_str(id) ?: "");
+            if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_IN_BAND)) {
+                QDict *qdict = qobject_to(QDict, req_obj->req);
+                QObject *id = qdict ? qdict_get(qdict, "id") : NULL;
+                GString *id_json;
+
+                id_json = id ? qobject_to_json(id) : g_string_new(NULL);
+                trace_monitor_qmp_cmd_in_band(id_json->str);
+                g_string_free(id_json, true);
+            }
             monitor_qmp_dispatch(mon, req_obj->req);
         } else {
             assert(req_obj->err);
@@ -308,17 +313,11 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
 static void handle_qmp_command(void *opaque, QObject *req, Error *err)
 {
     MonitorQMP *mon = opaque;
-    QObject *id = NULL;
-    QDict *qdict;
+    QDict *qdict = qobject_to(QDict, req);
     QMPRequest *req_obj;
 
     assert(!req != !err);
 
-    qdict = qobject_to(QDict, req);
-    if (qdict) {
-        id = qdict_get(qdict, "id");
-    } /* else will fail qmp_dispatch() */
-
     if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
         GString *req_json = qobject_to_json(req);
         trace_handle_qmp_command(mon, req_json->str);
@@ -327,7 +326,14 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
 
     if (qdict && qmp_is_oob(qdict)) {
         /* OOB commands are executed immediately */
-        trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) ?: "");
+        if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_OUT_OF_BAND)) {
+            QObject *id = qdict_get(qdict, "id");
+            GString *id_json;
+
+            id_json = id ? qobject_to_json(id) : g_string_new(NULL);
+            trace_monitor_qmp_cmd_out_of_band(id_json->str);
+            g_string_free(id_json, true);
+        }
         monitor_qmp_dispatch(mon, req);
         qobject_unref(req);
         return;
-- 
2.26.2



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

* [PATCH 10/20] block: Avoid qobject_get_try_str()
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (8 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 09/20] qmp: Fix tracing of non-string command IDs Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 18:28   ` Vladimir Sementsov-Ogievskiy
  2020-12-11 17:11 ` [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API" Markus Armbruster
                   ` (10 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, mdroth, qemu-block, Max Reitz

I'm about to remove qobject_get_try_str().  Use qstring_get_str()
instead.  Safe because the argument is known to be a QString here.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 487b2b1497..94d3a15081 100644
--- a/block.c
+++ b/block.c
@@ -4015,7 +4015,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
         new_backing_bs = NULL;
         break;
     case QTYPE_QSTRING:
-        str = qobject_get_try_str(value);
+        str = qstring_get_str(qobject_to(QString, value));
         new_backing_bs = bdrv_lookup_bs(NULL, str, errp);
         if (new_backing_bs == NULL) {
             return -EINVAL;
@@ -4278,8 +4278,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
                 }
 
                 if (child) {
-                    const char *str = qobject_get_try_str(new);
-                    if (!strcmp(child->bs->node_name, str)) {
+                    if (!strcmp(child->bs->node_name,
+                                qstring_get_str(qobject_to(QString, new)))) {
                         continue; /* Found child with this name, skip option */
                     }
                 }
-- 
2.26.2



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

* [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API"
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (9 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 10/20] block: Avoid qobject_get_try_str() Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 19:48   ` Eduardo Habkost
  2020-12-11 17:11 ` [PATCH 12/20] qobject: Drop qobject_get_try_str() Markus Armbruster
                   ` (9 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P . Berrangé, mdroth, Eduardo Habkost

Commit aafb21a0b9 "qobject: let object_property_get_str() use new API"
isn't much of a simplification.  Not worth having
object_property_get_str() differ from the other
object_property_get_FOO().  Revert.

This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qom/object.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 1065355233..89f5a63211 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1414,15 +1414,18 @@ char *object_property_get_str(Object *obj, const char *name,
                               Error **errp)
 {
     QObject *ret = object_property_get_qobject(obj, name, errp);
+    QString *qstring;
     char *retval;
 
     if (!ret) {
         return NULL;
     }
-
-    retval = g_strdup(qobject_get_try_str(ret));
-    if (!retval) {
+    qstring = qobject_to(QString, ret);
+    if (!qstring) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "string");
+        retval = NULL;
+    } else {
+        retval = g_strdup(qstring_get_str(qstring));
     }
 
     qobject_unref(ret);
-- 
2.26.2



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

* [PATCH 12/20] qobject: Drop qobject_get_try_str()
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (10 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API" Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 13/20] qobject: Drop qstring_get_try_str() Markus Armbruster
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c          | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index e4ac761a22..56034dae54 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -29,7 +29,6 @@ QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 const char *qstring_get_try_str(const QString *qstring);
-const char *qobject_get_try_str(const QObject *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);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index d6724bd4e5..cfe3f3bf00 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -149,17 +149,6 @@ const char *qstring_get_try_str(const QString *qstring)
     return qstring ? qstring_get_str(qstring) : NULL;
 }
 
-/**
- * qobject_get_try_str(): Return a pointer to the corresponding string
- *
- * NOTE: the string will only be returned if the object is valid, and
- * its type is QString, otherwise NULL is returned.
- */
-const char *qobject_get_try_str(const QObject *qstring)
-{
-    return qstring_get_try_str(qobject_to(QString, qstring));
-}
-
 /**
  * qstring_is_equal(): Test whether the two QStrings are equal
  */
-- 
2.26.2



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

* [PATCH 13/20] qobject: Drop qstring_get_try_str()
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (11 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 12/20] qobject: Drop qobject_get_try_str() Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 14/20] qobject: Factor quoted_str() out of to_json() Markus Armbruster
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

No users left outside tests/, and the ones in tests/ can just as well
use qstring_get_str().  Do that, and drop the function.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c          | 10 ----------
 tests/check-qjson.c        | 11 +++++------
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 56034dae54..53567db6c0 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -28,7 +28,6 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end);
 QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
-const char *qstring_get_try_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);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index cfe3f3bf00..ea86d80cf0 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -139,16 +139,6 @@ const char *qstring_get_str(const QString *qstring)
     return qstring->string;
 }
 
-/**
- * qstring_get_try_str(): Return a pointer to the stored string
- *
- * NOTE: will return NULL if qstring is not provided.
- */
-const char *qstring_get_try_str(const QString *qstring)
-{
-    return qstring ? qstring_get_str(qstring) : NULL;
-}
-
 /**
  * qstring_is_equal(): Test whether the two QStrings are equal
  */
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 337add0838..c845f91d43 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -89,7 +89,7 @@ static void escaped_string(void)
         for (j = 0; j < 2; j++) {
             if (test_cases[i].utf8_out) {
                 cstr = from_json_str(test_cases[i].json_in, j, &error_abort);
-                g_assert_cmpstr(qstring_get_try_str(cstr),
+                g_assert_cmpstr(qstring_get_str(cstr),
                                 ==, test_cases[i].utf8_out);
                 if (!test_cases[i].skip) {
                     jstr = to_json_str(cstr);
@@ -751,7 +751,7 @@ static void utf8_string(void)
             /* Parse @json_in, expect @utf8_out */
             if (utf8_out) {
                 str = from_json_str(json_in, j, &error_abort);
-                g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_out);
+                g_assert_cmpstr(qstring_get_str(str), ==, utf8_out);
                 qobject_unref(str);
             } else {
                 str = from_json_str(json_in, j, NULL);
@@ -782,7 +782,7 @@ static void utf8_string(void)
             /* Parse @json_out right back, unless it has replacements */
             if (!strstr(json_out, "\\uFFFD")) {
                 str = from_json_str(json_out, j, &error_abort);
-                g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_in);
+                g_assert_cmpstr(qstring_get_str(str), ==, utf8_in);
                 qobject_unref(str);
             }
         }
@@ -1021,9 +1021,8 @@ static void interpolation_valid(void)
 
     /* string */
 
-    qstr = qobject_to(QString,
-                     qobject_from_jsonf_nofail("%s", value_s));
-    g_assert_cmpstr(qstring_get_try_str(qstr), ==, value_s);
+    qstr = qobject_to(QString, qobject_from_jsonf_nofail("%s", value_s));
+    g_assert_cmpstr(qstring_get_str(qstr), ==, value_s);
     qobject_unref(qstr);
 
     /* object */
-- 
2.26.2



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

* [PATCH 14/20] qobject: Factor quoted_str() out of to_json()
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (12 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 13/20] qobject: Drop qstring_get_try_str() Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 15/20] qobject: Factor JSON writer out of qobject_to_json() Markus Armbruster
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qjson.c | 110 ++++++++++++++++++++++++------------------------
 1 file changed, 54 insertions(+), 56 deletions(-)

diff --git a/qobject/qjson.c b/qobject/qjson.c
index 2f690c1816..962214f5a7 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -156,6 +156,58 @@ static void json_pretty_newline(GString *accu, bool pretty, int indent)
     }
 }
 
+static void quoted_str(const char *str, GString *accu)
+{
+    const char *ptr;
+    int cp;
+    char *end;
+
+    g_string_append_c(accu, '"');
+
+    for (ptr = str; *ptr; ptr = end) {
+        cp = mod_utf8_codepoint(ptr, 6, &end);
+        switch (cp) {
+        case '\"':
+            g_string_append(accu, "\\\"");
+            break;
+        case '\\':
+            g_string_append(accu, "\\\\");
+            break;
+        case '\b':
+            g_string_append(accu, "\\b");
+            break;
+        case '\f':
+            g_string_append(accu, "\\f");
+            break;
+        case '\n':
+            g_string_append(accu, "\\n");
+            break;
+        case '\r':
+            g_string_append(accu, "\\r");
+            break;
+        case '\t':
+            g_string_append(accu, "\\t");
+            break;
+        default:
+            if (cp < 0) {
+                cp = 0xFFFD; /* replacement character */
+            }
+            if (cp > 0xFFFF) {
+                /* beyond BMP; need a surrogate pair */
+                g_string_append_printf(accu, "\\u%04X\\u%04X",
+                                       0xD800 + ((cp - 0x10000) >> 10),
+                                       0xDC00 + ((cp - 0x10000) & 0x3FF));
+            } else if (cp < 0x20 || cp >= 0x7F) {
+                g_string_append_printf(accu, "\\u%04X", cp);
+            } else {
+                g_string_append_c(accu, cp);
+            }
+        }
+    };
+
+    g_string_append_c(accu, '"');
+}
+
 static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
 {
     switch (qobject_type(obj)) {
@@ -170,56 +222,7 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
         break;
     }
     case QTYPE_QSTRING: {
-        QString *val = qobject_to(QString, obj);
-        const char *ptr;
-        int cp;
-        char *end;
-
-        ptr = qstring_get_str(val);
-        g_string_append_c(accu, '"');
-
-        for (; *ptr; ptr = end) {
-            cp = mod_utf8_codepoint(ptr, 6, &end);
-            switch (cp) {
-            case '\"':
-                g_string_append(accu, "\\\"");
-                break;
-            case '\\':
-                g_string_append(accu, "\\\\");
-                break;
-            case '\b':
-                g_string_append(accu, "\\b");
-                break;
-            case '\f':
-                g_string_append(accu, "\\f");
-                break;
-            case '\n':
-                g_string_append(accu, "\\n");
-                break;
-            case '\r':
-                g_string_append(accu, "\\r");
-                break;
-            case '\t':
-                g_string_append(accu, "\\t");
-                break;
-            default:
-                if (cp < 0) {
-                    cp = 0xFFFD; /* replacement character */
-                }
-                if (cp > 0xFFFF) {
-                    /* beyond BMP; need a surrogate pair */
-                    g_string_append_printf(accu, "\\u%04X\\u%04X",
-                                           0xD800 + ((cp - 0x10000) >> 10),
-                                           0xDC00 + ((cp - 0x10000) & 0x3FF));
-                } else if (cp < 0x20 || cp >= 0x7F) {
-                    g_string_append_printf(accu, "\\u%04X", cp);
-                } else {
-                    g_string_append_c(accu, cp);
-                }
-            }
-        };
-
-        g_string_append_c(accu, '"');
+        quoted_str(qstring_get_str(qobject_to(QString, obj)), accu);
         break;
     }
     case QTYPE_QDICT: {
@@ -227,7 +230,6 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
         const char *comma = pretty ? "," : ", ";
         const char *sep = "";
         const QDictEntry *entry;
-        QString *qkey;
 
         g_string_append_c(accu, '{');
 
@@ -236,11 +238,7 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
              entry = qdict_next(val, entry)) {
             g_string_append(accu, sep);
             json_pretty_newline(accu, pretty, indent + 1);
-
-            qkey = qstring_from_str(qdict_entry_key(entry));
-            to_json(QOBJECT(qkey), accu, pretty, indent + 1);
-            qobject_unref(qkey);
-
+            quoted_str(qdict_entry_key(entry), accu);
             g_string_append(accu, ": ");
             to_json(qdict_entry_value(entry), accu, pretty, indent + 1);
             sep = comma;
-- 
2.26.2



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

* [PATCH 15/20] qobject: Factor JSON writer out of qobject_to_json()
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (13 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 14/20] qobject: Factor quoted_str() out of to_json() Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 16/20] migration: Replace migration's JSON writer by the general one Markus Armbruster
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

We have two JSON writers written in C: qobject/qjson.c provides
qobject_to_json(), and migration/qjson.c provides a more low level
imperative interface.  They don't share code.  The latter tacitly
limits numbers to int64_t, and strings contents to characters that
don't need escaping.

Factor out qobject_to_json()'s JSON writer as qobject/json-writer.c.
Straightforward, except for numbers: since the writer is to be
independent of QObject, it can't use qnum_to_string().  Open-code it
instead.  This is actually an improvement of sorts, because it
liberates qnum_to_string() from JSON's needs: its JSON-related FIXMEs
move to the JSON writer, where they belong.

The next commit will replace migration/qjson.c.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/json-writer.h |  37 +++++
 qobject/json-writer.c          | 247 +++++++++++++++++++++++++++++++++
 qobject/qjson.c                | 125 +++++------------
 qobject/qnum.c                 |   5 -
 qobject/meson.build            |   5 +-
 5 files changed, 318 insertions(+), 101 deletions(-)
 create mode 100644 include/qapi/qmp/json-writer.h
 create mode 100644 qobject/json-writer.c

diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h
new file mode 100644
index 0000000000..708d129018
--- /dev/null
+++ b/include/qapi/qmp/json-writer.h
@@ -0,0 +1,37 @@
+/*
+ * JSON Writer
+ *
+ * Copyright (c) 2020 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>
+ *
+ * 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_WRITER_H
+#define JSON_WRITER_H
+
+typedef struct JSONWriter JSONWriter;
+
+JSONWriter *json_writer_new(bool pretty);
+const char *json_writer_get(JSONWriter *);
+GString *json_writer_get_and_free(JSONWriter *);
+void json_writer_free(JSONWriter *);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(JSONWriter, json_writer_free)
+
+void json_writer_start_object(JSONWriter *, const char *name);
+void json_writer_end_object(JSONWriter *);
+void json_writer_start_array(JSONWriter *, const char *name);
+void json_writer_end_array(JSONWriter *);
+void json_writer_bool(JSONWriter *, const char *name, bool val);
+void json_writer_null(JSONWriter *, const char *name);
+void json_writer_int64(JSONWriter *, const char *name, int64_t val);
+void json_writer_uint64(JSONWriter *, const char *name, uint64_t val);
+void json_writer_double(JSONWriter *, const char *name, double val);
+void json_writer_str(JSONWriter *, const char *name, const char *str);
+
+#endif
diff --git a/qobject/json-writer.c b/qobject/json-writer.c
new file mode 100644
index 0000000000..309a31d57a
--- /dev/null
+++ b/qobject/json-writer.c
@@ -0,0 +1,247 @@
+/*
+ * JSON Writer
+ *
+ * Copyright IBM, Corp. 2009
+ * Copyright (c) 2010-2020 Red Hat Inc.
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Markus Armbruster <armbru@redhat.com>
+ *
+ * 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 "qemu/osdep.h"
+#include "qapi/qmp/json-writer.h"
+#include "qemu/unicode.h"
+
+struct JSONWriter {
+    bool pretty;
+    bool need_comma;
+    GString *contents;
+    GByteArray *container_is_array;
+};
+
+JSONWriter *json_writer_new(bool pretty)
+{
+    JSONWriter *writer = g_new(JSONWriter, 1);
+
+    writer->pretty = pretty;
+    writer->need_comma = false;
+    writer->contents = g_string_new(NULL);
+    writer->container_is_array = g_byte_array_new();
+    return writer;
+}
+
+const char *json_writer_get(JSONWriter *writer)
+{
+    g_assert(!writer->container_is_array->len);
+    return writer->contents->str;
+}
+
+GString *json_writer_get_and_free(JSONWriter *writer)
+{
+    GString *contents = writer->contents;
+
+    writer->contents = NULL;
+    g_byte_array_free(writer->container_is_array, true);
+    g_free(writer);
+    return contents;
+}
+
+void json_writer_free(JSONWriter *writer)
+{
+    if (writer) {
+        g_string_free(json_writer_get_and_free(writer), true);
+    }
+}
+
+static void enter_container(JSONWriter *writer, bool is_array)
+{
+    unsigned depth = writer->container_is_array->len;
+
+    g_byte_array_set_size(writer->container_is_array, depth + 1);
+    writer->container_is_array->data[depth] = is_array;
+    writer->need_comma = false;
+}
+
+static void leave_container(JSONWriter *writer, bool is_array)
+{
+    unsigned depth = writer->container_is_array->len;
+
+    assert(depth);
+    assert(writer->container_is_array->data[depth - 1] == is_array);
+    g_byte_array_set_size(writer->container_is_array, depth - 1);
+    writer->need_comma = true;
+}
+
+static bool in_object(JSONWriter *writer)
+{
+    unsigned depth = writer->container_is_array->len;
+
+    return depth && !writer->container_is_array->data[depth - 1];
+}
+
+static void pretty_newline(JSONWriter *writer)
+{
+    if (writer->pretty) {
+        g_string_append_printf(writer->contents, "\n%*s",
+                               writer->container_is_array->len * 4, "");
+    }
+}
+
+static void pretty_newline_or_space(JSONWriter *writer)
+{
+    if (writer->pretty) {
+        g_string_append_printf(writer->contents, "\n%*s",
+                               writer->container_is_array->len * 4, "");
+    } else {
+        g_string_append_c(writer->contents, ' ');
+    }
+}
+
+static void quoted_str(JSONWriter *writer, const char *str)
+{
+    const char *ptr;
+    char *end;
+    int cp;
+
+    g_string_append_c(writer->contents, '"');
+
+    for (ptr = str; *ptr; ptr = end) {
+        cp = mod_utf8_codepoint(ptr, 6, &end);
+        switch (cp) {
+        case '\"':
+            g_string_append(writer->contents, "\\\"");
+            break;
+        case '\\':
+            g_string_append(writer->contents, "\\\\");
+            break;
+        case '\b':
+            g_string_append(writer->contents, "\\b");
+            break;
+        case '\f':
+            g_string_append(writer->contents, "\\f");
+            break;
+        case '\n':
+            g_string_append(writer->contents, "\\n");
+            break;
+        case '\r':
+            g_string_append(writer->contents, "\\r");
+            break;
+        case '\t':
+            g_string_append(writer->contents, "\\t");
+            break;
+        default:
+            if (cp < 0) {
+                cp = 0xFFFD; /* replacement character */
+            }
+            if (cp > 0xFFFF) {
+                /* beyond BMP; need a surrogate pair */
+                g_string_append_printf(writer->contents, "\\u%04X\\u%04X",
+                                       0xD800 + ((cp - 0x10000) >> 10),
+                                       0xDC00 + ((cp - 0x10000) & 0x3FF));
+            } else if (cp < 0x20 || cp >= 0x7F) {
+                g_string_append_printf(writer->contents, "\\u%04X", cp);
+            } else {
+                g_string_append_c(writer->contents, cp);
+            }
+        }
+    };
+
+    g_string_append_c(writer->contents, '"');
+}
+
+static void maybe_comma_name(JSONWriter *writer, const char *name)
+{
+    if (writer->need_comma) {
+        g_string_append_c(writer->contents, ',');
+        pretty_newline_or_space(writer);
+    } else {
+        if (writer->contents->len) {
+            pretty_newline(writer);
+        }
+        writer->need_comma = true;
+    }
+
+    if (in_object(writer)) {
+        quoted_str(writer, name);
+        g_string_append(writer->contents, ": ");
+    }
+}
+
+void json_writer_start_object(JSONWriter *writer, const char *name)
+{
+    maybe_comma_name(writer, name);
+    g_string_append_c(writer->contents, '{');
+    enter_container(writer, false);
+}
+
+void json_writer_end_object(JSONWriter *writer)
+{
+    leave_container(writer, false);
+    pretty_newline(writer);
+    g_string_append_c(writer->contents, '}');
+}
+
+void json_writer_start_array(JSONWriter *writer, const char *name)
+{
+    maybe_comma_name(writer, name);
+    g_string_append_c(writer->contents, '[');
+    enter_container(writer, true);
+}
+
+void json_writer_end_array(JSONWriter *writer)
+{
+    leave_container(writer, true);
+    pretty_newline(writer);
+    g_string_append_c(writer->contents, ']');
+}
+
+void json_writer_bool(JSONWriter *writer, const char *name, bool val)
+{
+    maybe_comma_name(writer, name);
+    g_string_append(writer->contents, val ? "true" : "false");
+}
+
+void json_writer_null(JSONWriter *writer, const char *name)
+{
+    maybe_comma_name(writer, name);
+    g_string_append(writer->contents, "null");
+}
+
+void json_writer_int64(JSONWriter *writer, const char *name, int64_t val)
+{
+    maybe_comma_name(writer, name);
+    g_string_append_printf(writer->contents, "%" PRId64, val);
+}
+
+void json_writer_uint64(JSONWriter *writer, const char *name, uint64_t val)
+{
+    maybe_comma_name(writer, name);
+    g_string_append_printf(writer->contents, "%" PRIu64, val);
+}
+
+void json_writer_double(JSONWriter *writer, const char *name, double val)
+{
+    maybe_comma_name(writer, name);
+
+    /*
+     * FIXME: g_string_append_printf() is locale dependent; but JSON
+     * requires numbers to be formatted as if in the C locale.
+     * Dependence on C locale is a pervasive issue in QEMU.
+     */
+    /*
+     * FIXME: This risks printing Inf or NaN, which are not valid
+     * JSON values.
+     */
+    g_string_append_printf(writer->contents, "%.17g", val);
+}
+
+void json_writer_str(JSONWriter *writer, const char *name, const char *str)
+{
+    maybe_comma_name(writer, name);
+    quoted_str(writer, str);
+}
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 962214f5a7..bcc376e626 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -14,13 +14,13 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/json-writer.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
-#include "qemu/unicode.h"
 
 typedef struct JSONParsingState
 {
@@ -149,132 +149,69 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
     return qdict;
 }
 
-static void json_pretty_newline(GString *accu, bool pretty, int indent)
-{
-    if (pretty) {
-        g_string_append_printf(accu, "\n%*s", indent * 4, "");
-    }
-}
-
-static void quoted_str(const char *str, GString *accu)
-{
-    const char *ptr;
-    int cp;
-    char *end;
-
-    g_string_append_c(accu, '"');
-
-    for (ptr = str; *ptr; ptr = end) {
-        cp = mod_utf8_codepoint(ptr, 6, &end);
-        switch (cp) {
-        case '\"':
-            g_string_append(accu, "\\\"");
-            break;
-        case '\\':
-            g_string_append(accu, "\\\\");
-            break;
-        case '\b':
-            g_string_append(accu, "\\b");
-            break;
-        case '\f':
-            g_string_append(accu, "\\f");
-            break;
-        case '\n':
-            g_string_append(accu, "\\n");
-            break;
-        case '\r':
-            g_string_append(accu, "\\r");
-            break;
-        case '\t':
-            g_string_append(accu, "\\t");
-            break;
-        default:
-            if (cp < 0) {
-                cp = 0xFFFD; /* replacement character */
-            }
-            if (cp > 0xFFFF) {
-                /* beyond BMP; need a surrogate pair */
-                g_string_append_printf(accu, "\\u%04X\\u%04X",
-                                       0xD800 + ((cp - 0x10000) >> 10),
-                                       0xDC00 + ((cp - 0x10000) & 0x3FF));
-            } else if (cp < 0x20 || cp >= 0x7F) {
-                g_string_append_printf(accu, "\\u%04X", cp);
-            } else {
-                g_string_append_c(accu, cp);
-            }
-        }
-    };
-
-    g_string_append_c(accu, '"');
-}
-
-static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
+static void to_json(JSONWriter *writer, const char *name,
+                    const QObject *obj)
 {
     switch (qobject_type(obj)) {
     case QTYPE_QNULL:
-        g_string_append(accu, "null");
+        json_writer_null(writer, name);
         break;
     case QTYPE_QNUM: {
         QNum *val = qobject_to(QNum, obj);
-        char *buffer = qnum_to_string(val);
-        g_string_append(accu, buffer);
-        g_free(buffer);
+
+        switch (val->kind) {
+        case QNUM_I64:
+            json_writer_int64(writer, name, val->u.i64);
+            break;
+        case QNUM_U64:
+            json_writer_uint64(writer, name, val->u.u64);
+            break;
+        case QNUM_DOUBLE:
+            json_writer_double(writer, name, val->u.dbl);
+            break;
+        default:
+            abort();
+        }
         break;
     }
     case QTYPE_QSTRING: {
-        quoted_str(qstring_get_str(qobject_to(QString, obj)), accu);
+        QString *val = qobject_to(QString, obj);
+
+        json_writer_str(writer, name, qstring_get_str(val));
         break;
     }
     case QTYPE_QDICT: {
         QDict *val = qobject_to(QDict, obj);
-        const char *comma = pretty ? "," : ", ";
-        const char *sep = "";
         const QDictEntry *entry;
 
-        g_string_append_c(accu, '{');
+        json_writer_start_object(writer, name);
 
         for (entry = qdict_first(val);
              entry;
              entry = qdict_next(val, entry)) {
-            g_string_append(accu, sep);
-            json_pretty_newline(accu, pretty, indent + 1);
-            quoted_str(qdict_entry_key(entry), accu);
-            g_string_append(accu, ": ");
-            to_json(qdict_entry_value(entry), accu, pretty, indent + 1);
-            sep = comma;
+            to_json(writer, qdict_entry_key(entry), qdict_entry_value(entry));
         }
 
-        json_pretty_newline(accu, pretty, indent);
-        g_string_append_c(accu, '}');
+        json_writer_end_object(writer);
         break;
     }
     case QTYPE_QLIST: {
         QList *val = qobject_to(QList, obj);
-        const char *comma = pretty ? "," : ", ";
-        const char *sep = "";
         QListEntry *entry;
 
-        g_string_append_c(accu, '[');
+        json_writer_start_array(writer, name);
 
         QLIST_FOREACH_ENTRY(val, entry) {
-            g_string_append(accu, sep);
-            json_pretty_newline(accu, pretty, indent + 1);
-            to_json(qlist_entry_obj(entry), accu, pretty, indent + 1);
-            sep = comma;
+            to_json(writer, NULL, qlist_entry_obj(entry));
         }
 
-        json_pretty_newline(accu, pretty, indent);
-        g_string_append_c(accu, ']');
+        json_writer_end_array(writer);
         break;
     }
     case QTYPE_QBOOL: {
         QBool *val = qobject_to(QBool, obj);
 
-        if (qbool_get_bool(val)) {
-            g_string_append(accu, "true");
-        } else {
-            g_string_append(accu, "false");
-        }
+        json_writer_bool(writer, name, qbool_get_bool(val));
         break;
     }
     default:
@@ -284,10 +221,10 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
 
 GString *qobject_to_json_pretty(const QObject *obj, bool pretty)
 {
-    GString *accu = g_string_new(NULL);
+    JSONWriter *writer = json_writer_new(pretty);
 
-    to_json(obj, accu, pretty, 0);
-    return accu;
+    to_json(writer, NULL, obj);
+    return json_writer_get_and_free(writer);
 }
 
 GString *qobject_to_json(const QObject *obj)
diff --git a/qobject/qnum.c b/qobject/qnum.c
index 35ba41e61c..5dd66938dd 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -168,11 +168,6 @@ char *qnum_to_string(QNum *qn)
     case QNUM_U64:
         return g_strdup_printf("%" PRIu64, qn->u.u64);
     case QNUM_DOUBLE:
-        /* FIXME: g_strdup_printf() is locale dependent; but JSON requires
-         * numbers to be formatted as if in the C locale. Dependence
-         * on C locale is a pervasive issue in QEMU. */
-        /* FIXME: This risks printing Inf or NaN, which are not valid
-         * JSON values. */
         /* 17 digits suffice for IEEE double */
         return g_strdup_printf("%.17g", qn->u.dbl);
     }
diff --git a/qobject/meson.build b/qobject/meson.build
index bb63c06b63..4683a852a2 100644
--- a/qobject/meson.build
+++ b/qobject/meson.build
@@ -1,3 +1,4 @@
-util_ss.add(files('qnull.c', 'qnum.c', 'qstring.c', 'qdict.c', 'qlist.c', 'qbool.c',
-  'qlit.c', 'qjson.c', 'qobject.c', 'json-lexer.c', 'json-streamer.c', 'json-parser.c',
+util_ss.add(files('qnull.c', 'qnum.c', 'qstring.c', 'qdict.c',
+  'qlist.c', 'qbool.c', 'qlit.c', 'qjson.c', 'qobject.c',
+  'json-writer.c', 'json-lexer.c', 'json-streamer.c', 'json-parser.c',
   'block-qdict.c'))
-- 
2.26.2



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

* [PATCH 16/20] migration: Replace migration's JSON writer by the general one
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (14 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 15/20] qobject: Factor JSON writer out of qobject_to_json() Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-16 19:46   ` Dr. David Alan Gilbert
  2020-12-11 17:11 ` [PATCH 17/20] json: Use GString instead of QString to accumulate strings Markus Armbruster
                   ` (4 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, Dr . David Alan Gilbert, Juan Quintela

Commit 8118f0950f "migration: Append JSON description of migration
stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
good fit, because it requires building a QObject to convert.  Instead,
migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
Add JSON writer".  It tacitly limits numbers to int64_t, and strings
contents to characters that don't need escaping, unlike
qobject_to_json().

The previous commit factored the JSON writer out of qobject_to_json().
Replace migration's JSON writer by it.

Cc: Juan Quintela <quintela@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/migration/vmstate.h    |   7 +-
 include/qapi/qmp/json-writer.h |   2 -
 include/qemu/typedefs.h        |   4 +-
 migration/qjson.h              |  29 ---------
 hw/display/virtio-gpu.c        |   2 +-
 hw/intc/s390_flic_kvm.c        |   2 +-
 hw/nvram/eeprom93xx.c          |   2 +-
 hw/nvram/fw_cfg.c              |   2 +-
 hw/pci/msix.c                  |   2 +-
 hw/pci/pci.c                   |   4 +-
 hw/pci/shpc.c                  |   2 +-
 hw/rtc/twl92230.c              |   2 +-
 hw/scsi/scsi-bus.c             |   2 +-
 hw/usb/redirect.c              |   7 +-
 hw/virtio/virtio.c             |   4 +-
 migration/qjson.c              | 114 ---------------------------------
 migration/savevm.c             |  53 +++++++--------
 migration/vmstate-types.c      |  38 +++++------
 migration/vmstate.c            |  52 +++++++--------
 target/alpha/machine.c         |   2 +-
 target/arm/machine.c           |   6 +-
 target/avr/machine.c           |   4 +-
 target/hppa/machine.c          |   4 +-
 target/microblaze/machine.c    |   2 +-
 target/mips/machine.c          |   4 +-
 target/openrisc/machine.c      |   2 +-
 target/ppc/machine.c           |  10 +--
 target/sparc/machine.c         |   2 +-
 migration/meson.build          |   1 -
 29 files changed, 114 insertions(+), 253 deletions(-)
 delete mode 100644 migration/qjson.h
 delete mode 100644 migration/qjson.c

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 4d71dc8fba..075ee80096 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -43,7 +43,7 @@ struct VMStateInfo {
     const char *name;
     int (*get)(QEMUFile *f, void *pv, size_t size, const VMStateField *field);
     int (*put)(QEMUFile *f, void *pv, size_t size, const VMStateField *field,
-               QJSON *vmdesc);
+               JSONWriter *vmdesc);
 };
 
 enum VMStateFlags {
@@ -1169,9 +1169,10 @@ extern const VMStateInfo vmstate_info_qlist;
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id);
 int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-                       void *opaque, QJSON *vmdesc);
+                       void *opaque, JSONWriter *vmdesc);
 int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
-                         void *opaque, QJSON *vmdesc, int version_id);
+                         void *opaque, JSONWriter *vmdesc,
+                         int version_id);
 
 bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
 
diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h
index 708d129018..b70ba64077 100644
--- a/include/qapi/qmp/json-writer.h
+++ b/include/qapi/qmp/json-writer.h
@@ -14,8 +14,6 @@
 #ifndef JSON_WRITER_H
 #define JSON_WRITER_H
 
-typedef struct JSONWriter JSONWriter;
-
 JSONWriter *json_writer_new(bool pretty);
 const char *json_writer_get(JSONWriter *);
 GString *json_writer_get_and_free(JSONWriter *);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 6281eae3b5..976b529dfb 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -57,8 +57,8 @@ typedef struct IOMMUMemoryRegion IOMMUMemoryRegion;
 typedef struct ISABus ISABus;
 typedef struct ISADevice ISADevice;
 typedef struct IsaDma IsaDma;
+typedef struct JSONWriter JSONWriter;
 typedef struct MACAddr MACAddr;
-typedef struct ReservedRegion ReservedRegion;
 typedef struct MachineClass MachineClass;
 typedef struct MachineState MachineState;
 typedef struct MemoryListener MemoryListener;
@@ -107,7 +107,6 @@ typedef struct QEMUSGList QEMUSGList;
 typedef struct QemuSpin QemuSpin;
 typedef struct QEMUTimer QEMUTimer;
 typedef struct QEMUTimerListGroup QEMUTimerListGroup;
-typedef struct QJSON QJSON;
 typedef struct QList QList;
 typedef struct QNull QNull;
 typedef struct QNum QNum;
@@ -115,6 +114,7 @@ typedef struct QObject QObject;
 typedef struct QString QString;
 typedef struct RAMBlock RAMBlock;
 typedef struct Range Range;
+typedef struct ReservedRegion ReservedRegion;
 typedef struct SavedIOTLB SavedIOTLB;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SSIBus SSIBus;
diff --git a/migration/qjson.h b/migration/qjson.h
deleted file mode 100644
index 1786bb5864..0000000000
--- a/migration/qjson.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * QEMU JSON writer
- *
- * Copyright Alexander Graf
- *
- * Authors:
- *  Alexander Graf <agraf@suse.de>
- *
- * 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 QEMU_QJSON_H
-#define QEMU_QJSON_H
-
-QJSON *qjson_new(void);
-void qjson_destroy(QJSON *json);
-void json_prop_str(QJSON *json, const char *name, const char *str);
-void json_prop_int(QJSON *json, const char *name, int64_t val);
-void json_end_array(QJSON *json);
-void json_start_array(QJSON *json, const char *name);
-void json_end_object(QJSON *json);
-void json_start_object(QJSON *json, const char *name);
-const char *qjson_get_str(QJSON *json);
-void qjson_finish(QJSON *json);
-
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy)
-
-#endif /* QEMU_QJSON_H */
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f3b71fa9c7..0e833a462b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -970,7 +970,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
 };
 
 static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
-                           const VMStateField *field, QJSON *vmdesc)
+                           const VMStateField *field, JSONWriter *vmdesc)
 {
     VirtIOGPU *g = opaque;
     struct virtio_gpu_simple_resource *res;
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 35d91afa55..b3fb9f8395 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -386,7 +386,7 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
  * reached
  */
 static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
-                         const VMStateField *field, QJSON *vmdesc)
+                         const VMStateField *field, JSONWriter *vmdesc)
 {
     KVMS390FLICState *flic = opaque;
     int len = FLIC_SAVE_INITIAL_SIZE;
diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
index ca6f591c84..a1b9c78844 100644
--- a/hw/nvram/eeprom93xx.c
+++ b/hw/nvram/eeprom93xx.c
@@ -104,7 +104,7 @@ static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_unused(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     fprintf(stderr, "uint16_from_uint8 is used only for backwards compatibility.\n");
     fprintf(stderr, "Never should be used to write a new state.\n");
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 282ba93e2e..4a15aadd36 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -582,7 +582,7 @@ static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_unused(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     fprintf(stderr, "uint32_as_uint16 is only used for backward compatibility.\n");
     fprintf(stderr, "This functions shouldn't be called.\n");
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 67e34f34d6..cd13ef558f 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -631,7 +631,7 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
 }
 
 static int put_msix_state(QEMUFile *f, void *pv, size_t size,
-                          const VMStateField *field, QJSON *vmdesc)
+                          const VMStateField *field, JSONWriter *vmdesc)
 {
     msix_save(pv, f);
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 0131d9d02c..25f2755c71 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -558,7 +558,7 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size,
 
 /* just put buffer */
 static int put_pci_config_device(QEMUFile *f, void *pv, size_t size,
-                                 const VMStateField *field, QJSON *vmdesc)
+                                 const VMStateField *field, JSONWriter *vmdesc)
 {
     const uint8_t **v = pv;
     assert(size == pci_config_size(container_of(pv, PCIDevice, config)));
@@ -596,7 +596,7 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_pci_irq_state(QEMUFile *f, void *pv, size_t size,
-                             const VMStateField *field, QJSON *vmdesc)
+                             const VMStateField *field, JSONWriter *vmdesc)
 {
     int i;
     PCIDevice *s = container_of(pv, PCIDevice, irq_state);
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index b00dce629c..4786a44996 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -699,7 +699,7 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
 }
 
 static int shpc_save(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     PCIDevice *d = container_of(pv, PCIDevice, shpc);
     qemu_put_buffer(f, d->shpc->config, SHPC_SIZEOF(d));
diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c
index f838913b37..d942908223 100644
--- a/hw/rtc/twl92230.c
+++ b/hw/rtc/twl92230.c
@@ -762,7 +762,7 @@ static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
-                               const VMStateField *field, QJSON *vmdesc)
+                               const VMStateField *field, JSONWriter *vmdesc)
 {
     int *v = pv;
     qemu_put_be16(f, *v);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index b901e701f0..086dd263e0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1634,7 +1634,7 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
 /* SCSI request list.  For simplicity, pv points to the whole device */
 
 static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
-                             const VMStateField *field, QJSON *vmdesc)
+                             const VMStateField *field, JSONWriter *vmdesc)
 {
     SCSIDevice *s = pv;
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 3238de6bb8..4ca7d47ef7 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2227,7 +2227,7 @@ static int usbredir_post_load(void *priv, int version_id)
 
 /* For usbredirparser migration */
 static int usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
-                               const VMStateField *field, QJSON *vmdesc)
+                               const VMStateField *field, JSONWriter *vmdesc)
 {
     USBRedirDevice *dev = priv;
     uint8_t *data;
@@ -2294,7 +2294,7 @@ static const VMStateInfo usbredir_parser_vmstate_info = {
 
 /* For buffered packets (iso/irq) queue migration */
 static int usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
-                              const VMStateField *field, QJSON *vmdesc)
+                              const VMStateField *field, JSONWriter *vmdesc)
 {
     struct endp_data *endp = priv;
     USBRedirDevice *dev = endp->dev;
@@ -2421,7 +2421,8 @@ static const VMStateDescription usbredir_ep_vmstate = {
 
 /* For PacketIdQueue migration */
 static int usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
-                                    const VMStateField *field, QJSON *vmdesc)
+                                    const VMStateField *field,
+                                    JSONWriter *vmdesc)
 {
     struct PacketIdQueue *q = priv;
     USBRedirDevice *dev = q->dev;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index eff35fab7c..b308026596 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2745,7 +2745,7 @@ static int get_extra_state(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_extra_state(QEMUFile *f, void *pv, size_t size,
-                           const VMStateField *field, QJSON *vmdesc)
+                           const VMStateField *field, JSONWriter *vmdesc)
 {
     VirtIODevice *vdev = pv;
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
@@ -2919,7 +2919,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
 
 /* A wrapper for use as a VMState .put function */
 static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
-                              const VMStateField *field, QJSON *vmdesc)
+                              const VMStateField *field, JSONWriter *vmdesc)
 {
     return virtio_save(VIRTIO_DEVICE(opaque), f);
 }
diff --git a/migration/qjson.c b/migration/qjson.c
deleted file mode 100644
index e9889bdcb0..0000000000
--- a/migration/qjson.c
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * A simple JSON writer
- *
- * Copyright Alexander Graf
- *
- * Authors:
- *  Alexander Graf <agraf@suse.de>
- *
- * 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.
- *
- */
-
-/*
- * Type QJSON lets you build JSON text.  Its interface mirrors (a
- * subset of) abstract JSON syntax.
- *
- * It does *not* detect incorrect use.  It happily produces invalid
- * JSON then.  This is what migration wants.
- *
- * QAPI output visitors also produce JSON text.  However, they do
- * assert their preconditions and invariants, and therefore abort on
- * incorrect use.
- */
-
-#include "qemu/osdep.h"
-#include "qapi/qmp/qstring.h"
-#include "qjson.h"
-
-struct QJSON {
-    QString *str;
-    bool omit_comma;
-};
-
-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->str, "\"");
-        qstring_append(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;
-}
-
-void json_end_object(QJSON *json)
-{
-    qstring_append(json->str, " }");
-    json->omit_comma = false;
-}
-
-void json_start_array(QJSON *json, const char *name)
-{
-    json_emit_element(json, name);
-    qstring_append(json->str, "[ ");
-    json->omit_comma = true;
-}
-
-void json_end_array(QJSON *json)
-{
-    qstring_append(json->str, " ]");
-    json->omit_comma = false;
-}
-
-void json_prop_int(QJSON *json, const char *name, int64_t val)
-{
-    json_emit_element(json, name);
-    qstring_append_int(json->str, 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, '"');
-}
-
-const char *qjson_get_str(QJSON *json)
-{
-    return qstring_get_str(json->str);
-}
-
-QJSON *qjson_new(void)
-{
-    QJSON *json = g_new0(QJSON, 1);
-
-    json->str = qstring_from_str("{ ");
-    json->omit_comma = true;
-    return json;
-}
-
-void qjson_finish(QJSON *json)
-{
-    json_end_object(json);
-}
-
-void qjson_destroy(QJSON *json)
-{
-    qobject_unref(json->str);
-    g_free(json);
-}
diff --git a/migration/savevm.c b/migration/savevm.c
index 5f937a2762..138a96de4d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -42,6 +42,7 @@
 #include "postcopy-ram.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-migration.h"
+#include "qapi/qmp/json-writer.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/error-report.h"
 #include "sysemu/cpus.h"
@@ -58,7 +59,6 @@
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/xen.h"
-#include "qjson.h"
 #include "migration/colo.h"
 #include "qemu/bitmap.h"
 #include "net/announce.h"
@@ -209,7 +209,7 @@ static int get_timer(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_timer(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     QEMUTimer *v = pv;
     timer_put(f, v);
@@ -406,7 +406,7 @@ static int get_capability(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_capability(QEMUFile *f, void *pv, size_t size,
-                          const VMStateField *field, QJSON *vmdesc)
+                          const VMStateField *field, JSONWriter *vmdesc)
 {
     MigrationCapability *capability = pv;
     const char *capability_str = MigrationCapability_str(*capability);
@@ -884,7 +884,8 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
     return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
 }
 
-static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
+static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
+                                   JSONWriter *vmdesc)
 {
     int64_t old_offset, size;
 
@@ -893,18 +894,19 @@ 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);
+        json_writer_int64(vmdesc, "size", size);
+        json_writer_start_array(vmdesc, "fields");
+        json_writer_start_object(vmdesc, NULL);
+        json_writer_str(vmdesc, "name", "data");
+        json_writer_int64(vmdesc, "size", size);
+        json_writer_str(vmdesc, "type", "buffer");
+        json_writer_end_object(vmdesc);
+        json_writer_end_array(vmdesc);
     }
 }
 
-static int vmstate_save(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
+static int vmstate_save(QEMUFile *f, SaveStateEntry *se,
+                        JSONWriter *vmdesc)
 {
     trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
     if (!se->vmsd) {
@@ -1357,14 +1359,15 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
                                                     bool in_postcopy,
                                                     bool inactivate_disks)
 {
-    g_autoptr(QJSON) vmdesc = NULL;
+    g_autoptr(JSONWriter) vmdesc = NULL;
     int vmdesc_len;
     SaveStateEntry *se;
     int ret;
 
-    vmdesc = qjson_new();
-    json_prop_int(vmdesc, "page_size", qemu_target_page_size());
-    json_start_array(vmdesc, "devices");
+    vmdesc = json_writer_new(false);
+    json_writer_start_object(vmdesc, NULL);
+    json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
+    json_writer_start_array(vmdesc, "devices");
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
 
         if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
@@ -1377,9 +1380,9 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
 
         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);
+        json_writer_start_object(vmdesc, NULL);
+        json_writer_str(vmdesc, "name", se->idstr);
+        json_writer_int64(vmdesc, "instance_id", se->instance_id);
 
         save_section_header(f, se, QEMU_VM_SECTION_FULL);
         ret = vmstate_save(f, se, vmdesc);
@@ -1390,7 +1393,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
         trace_savevm_section_end(se->idstr, se->section_id, 0);
         save_section_footer(f, se);
 
-        json_end_object(vmdesc);
+        json_writer_end_object(vmdesc);
     }
 
     if (inactivate_disks) {
@@ -1409,14 +1412,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
         qemu_put_byte(f, QEMU_VM_EOF);
     }
 
-    json_end_array(vmdesc);
-    qjson_finish(vmdesc);
-    vmdesc_len = strlen(qjson_get_str(vmdesc));
+    json_writer_end_array(vmdesc);
+    json_writer_end_object(vmdesc);
+    vmdesc_len = strlen(json_writer_get(vmdesc));
 
     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 *)json_writer_get(vmdesc), vmdesc_len);
     }
 
     return 0;
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index e22d41d73d..bf4d440308 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -29,7 +29,7 @@ static int get_bool(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_bool(QEMUFile *f, void *pv, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     bool *v = pv;
     qemu_put_byte(f, *v);
@@ -53,7 +53,7 @@ static int get_int8(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_int8(QEMUFile *f, void *pv, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     int8_t *v = pv;
     qemu_put_s8s(f, v);
@@ -77,7 +77,7 @@ static int get_int16(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_int16(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     int16_t *v = pv;
     qemu_put_sbe16s(f, v);
@@ -101,7 +101,7 @@ static int get_int32(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_int32(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     int32_t *v = pv;
     qemu_put_sbe32s(f, v);
@@ -178,7 +178,7 @@ static int get_int64(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_int64(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     int64_t *v = pv;
     qemu_put_sbe64s(f, v);
@@ -202,7 +202,7 @@ static int get_uint8(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_uint8(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     uint8_t *v = pv;
     qemu_put_8s(f, v);
@@ -226,7 +226,7 @@ static int get_uint16(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_uint16(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     uint16_t *v = pv;
     qemu_put_be16s(f, v);
@@ -250,7 +250,7 @@ static int get_uint32(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_uint32(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     uint32_t *v = pv;
     qemu_put_be32s(f, v);
@@ -300,7 +300,7 @@ static int get_uint64(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_uint64(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     uint64_t *v = pv;
     qemu_put_be64s(f, v);
@@ -325,7 +325,7 @@ static int get_nullptr(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_nullptr(QEMUFile *f, void *pv, size_t size,
-                        const VMStateField *field, QJSON *vmdesc)
+                        const VMStateField *field, JSONWriter *vmdesc)
 
 {
     if (pv == NULL) {
@@ -432,7 +432,7 @@ static int get_cpudouble(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_cpudouble(QEMUFile *f, void *pv, size_t size,
-                         const VMStateField *field, QJSON *vmdesc)
+                         const VMStateField *field, JSONWriter *vmdesc)
 {
     CPU_DoubleU *v = pv;
     qemu_put_be32s(f, &v->l.upper);
@@ -457,7 +457,7 @@ static int get_buffer(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_buffer(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     uint8_t *v = pv;
     qemu_put_buffer(f, v, size);
@@ -488,7 +488,7 @@ static int get_unused_buffer(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_unused_buffer(QEMUFile *f, void *pv, size_t size,
-                             const VMStateField *field, QJSON *vmdesc)
+                             const VMStateField *field, JSONWriter *vmdesc)
 {
     static const uint8_t buf[1024];
     int block_len;
@@ -530,7 +530,7 @@ static int get_tmp(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_tmp(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     const VMStateDescription *vmsd = field->vmsd;
     void *tmp = g_malloc(size);
@@ -573,7 +573,7 @@ static int get_bitmap(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_bitmap(QEMUFile *f, void *pv, size_t size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     unsigned long *bmp = pv;
     int i, idx = 0;
@@ -637,7 +637,7 @@ static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
 
 /* put for QTAILQ */
 static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,
-                      const VMStateField *field, QJSON *vmdesc)
+                      const VMStateField *field, JSONWriter *vmdesc)
 {
     const VMStateDescription *vmsd = field->vmsd;
     /* offset of the QTAILQ entry in a QTAILQ element*/
@@ -670,7 +670,7 @@ struct put_gtree_data {
     QEMUFile *f;
     const VMStateDescription *key_vmsd;
     const VMStateDescription *val_vmsd;
-    QJSON *vmdesc;
+    JSONWriter *vmdesc;
     int ret;
 };
 
@@ -703,7 +703,7 @@ static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data)
 }
 
 static int put_gtree(QEMUFile *f, void *pv, size_t unused_size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     bool direct_key = (!field->start);
     const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[1];
@@ -819,7 +819,7 @@ const VMStateInfo vmstate_info_gtree = {
 };
 
 static int put_qlist(QEMUFile *f, void *pv, size_t unused_size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     const VMStateDescription *vmsd = field->vmsd;
     /* offset of the QTAILQ entry in a QTAILQ element*/
diff --git a/migration/vmstate.c b/migration/vmstate.c
index e9d2aef66b..05f87cdddc 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -14,14 +14,14 @@
 #include "migration.h"
 #include "migration/vmstate.h"
 #include "savevm.h"
+#include "qapi/qmp/json-writer.h"
 #include "qemu-file.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "trace.h"
-#include "qjson.h"
 
 static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, QJSON *vmdesc);
+                                   void *opaque, JSONWriter *vmdesc);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque);
 
@@ -249,7 +249,8 @@ static bool vmsd_can_compress(const VMStateField *field)
     return true;
 }
 
-static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc,
+static void vmsd_desc_field_start(const VMStateDescription *vmsd,
+                                  JSONWriter *vmdesc,
                                   const VMStateField *field, int i, int max)
 {
     char *name, *old_name;
@@ -270,25 +271,26 @@ static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc,
         g_free(old_name);
     }
 
-    json_start_object(vmdesc, NULL);
-    json_prop_str(vmdesc, "name", name);
+    json_writer_start_object(vmdesc, NULL);
+    json_writer_str(vmdesc, "name", name);
     if (is_array) {
         if (can_compress) {
-            json_prop_int(vmdesc, "array_len", max);
+            json_writer_int64(vmdesc, "array_len", max);
         } else {
-            json_prop_int(vmdesc, "index", i);
+            json_writer_int64(vmdesc, "index", i);
         }
     }
-    json_prop_str(vmdesc, "type", vmfield_get_type_name(field));
+    json_writer_str(vmdesc, "type", vmfield_get_type_name(field));
 
     if (field->flags & VMS_STRUCT) {
-        json_start_object(vmdesc, "struct");
+        json_writer_start_object(vmdesc, "struct");
     }
 
     g_free(name);
 }
 
-static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
+static void vmsd_desc_field_end(const VMStateDescription *vmsd,
+                                JSONWriter *vmdesc,
                                 const VMStateField *field, size_t size, int i)
 {
     if (!vmdesc) {
@@ -297,11 +299,11 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
 
     if (field->flags & VMS_STRUCT) {
         /* We printed a struct in between, close its child object */
-        json_end_object(vmdesc);
+        json_writer_end_object(vmdesc);
     }
 
-    json_prop_int(vmdesc, "size", size);
-    json_end_object(vmdesc);
+    json_writer_int64(vmdesc, "size", size);
+    json_writer_end_object(vmdesc);
 }
 
 
@@ -316,13 +318,13 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
 
 
 int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-                       void *opaque, QJSON *vmdesc_id)
+                       void *opaque, JSONWriter *vmdesc_id)
 {
     return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id);
 }
 
 int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
-                         void *opaque, QJSON *vmdesc, int version_id)
+                         void *opaque, JSONWriter *vmdesc, int version_id)
 {
     int ret = 0;
     const VMStateField *field = vmsd->fields;
@@ -339,9 +341,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
     }
 
     if (vmdesc) {
-        json_prop_str(vmdesc, "vmsd_name", vmsd->name);
-        json_prop_int(vmdesc, "version", version_id);
-        json_start_array(vmdesc, "fields");
+        json_writer_str(vmdesc, "vmsd_name", vmsd->name);
+        json_writer_int64(vmdesc, "version", version_id);
+        json_writer_start_array(vmdesc, "fields");
     }
 
     while (field->name) {
@@ -353,7 +355,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
             int i, n_elems = vmstate_n_elems(opaque, field);
             int size = vmstate_size(opaque, field);
             int64_t old_offset, written_bytes;
-            QJSON *vmdesc_loop = vmdesc;
+            JSONWriter *vmdesc_loop = vmdesc;
 
             trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
             if (field->flags & VMS_POINTER) {
@@ -413,7 +415,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
     }
 
     if (vmdesc) {
-        json_end_array(vmdesc);
+        json_writer_end_array(vmdesc);
     }
 
     ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc);
@@ -491,7 +493,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
 }
 
 static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, QJSON *vmdesc)
+                                   void *opaque, JSONWriter *vmdesc)
 {
     const VMStateDescription **sub = vmsd->subsections;
     bool vmdesc_has_subsections = false;
@@ -507,11 +509,11 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
             if (vmdesc) {
                 /* Only create subsection array when we have any */
                 if (!vmdesc_has_subsections) {
-                    json_start_array(vmdesc, "subsections");
+                    json_writer_start_array(vmdesc, "subsections");
                     vmdesc_has_subsections = true;
                 }
 
-                json_start_object(vmdesc, NULL);
+                json_writer_start_object(vmdesc, NULL);
             }
 
             qemu_put_byte(f, QEMU_VM_SUBSECTION);
@@ -525,14 +527,14 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
             }
 
             if (vmdesc) {
-                json_end_object(vmdesc);
+                json_writer_end_object(vmdesc);
             }
         }
         sub++;
     }
 
     if (vmdesc_has_subsections) {
-        json_end_array(vmdesc);
+        json_writer_end_array(vmdesc);
     }
 
     return ret;
diff --git a/target/alpha/machine.c b/target/alpha/machine.c
index 9d20169d4f..2b7c8148ff 100644
--- a/target/alpha/machine.c
+++ b/target/alpha/machine.c
@@ -11,7 +11,7 @@ static int get_fpcr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_fpcr(QEMUFile *f, void *opaque, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     CPUAlphaState *env = opaque;
     qemu_put_be64(f, cpu_alpha_load_fpcr(env));
diff --git a/target/arm/machine.c b/target/arm/machine.c
index c5a2114f51..581852bc53 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -27,7 +27,7 @@ static int get_fpscr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_fpscr(QEMUFile *f, void *opaque, size_t size,
-                     const VMStateField *field, QJSON *vmdesc)
+                     const VMStateField *field, JSONWriter *vmdesc)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
@@ -573,7 +573,7 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_cpsr(QEMUFile *f, void *opaque, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
@@ -608,7 +608,7 @@ static int get_power(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_power(QEMUFile *f, void *opaque, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     ARMCPU *cpu = opaque;
 
diff --git a/target/avr/machine.c b/target/avr/machine.c
index e315442787..de264f57c3 100644
--- a/target/avr/machine.c
+++ b/target/avr/machine.c
@@ -34,7 +34,7 @@ static int get_sreg(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_sreg(QEMUFile *f, void *opaque, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     CPUAVRState *env = opaque;
     uint8_t sreg = cpu_get_sreg(env);
@@ -61,7 +61,7 @@ static int get_segment(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_segment(QEMUFile *f, void *opaque, size_t size,
-                       const VMStateField *field, QJSON *vmdesc)
+                       const VMStateField *field, JSONWriter *vmdesc)
 {
     uint32_t *ramp = opaque;
     uint8_t temp = *ramp >> 16;
diff --git a/target/hppa/machine.c b/target/hppa/machine.c
index b60b654efb..905991d7f9 100644
--- a/target/hppa/machine.c
+++ b/target/hppa/machine.c
@@ -52,7 +52,7 @@ static int get_psw(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_psw(QEMUFile *f, void *opaque, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     CPUHPPAState *env = opaque;
     qemu_put_betr(f, cpu_hppa_get_psw(env));
@@ -93,7 +93,7 @@ static int get_tlb(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_tlb(QEMUFile *f, void *opaque, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     hppa_tlb_entry *ent = opaque;
     uint32_t val = 0;
diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
index c2074bbdfe..d24def3992 100644
--- a/target/microblaze/machine.c
+++ b/target/microblaze/machine.c
@@ -46,7 +46,7 @@ static int get_msr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_msr(QEMUFile *f, void *opaque, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     CPUMBState *env = container_of(opaque, CPUMBState, msr);
 
diff --git a/target/mips/machine.c b/target/mips/machine.c
index 5b23e3e912..77afe654e9 100644
--- a/target/mips/machine.c
+++ b/target/mips/machine.c
@@ -31,7 +31,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_fpr(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     int i;
     fpr_t *v = pv;
@@ -156,7 +156,7 @@ static int get_tlb(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_tlb(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     r4k_tlb_t *v = pv;
 
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index b92985d99b..6239725c4f 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -55,7 +55,7 @@ static int get_sr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_sr(QEMUFile *f, void *opaque, size_t size,
-                  const VMStateField *field, QJSON *vmdesc)
+                  const VMStateField *field, JSONWriter *vmdesc)
 {
     CPUOpenRISCState *env = opaque;
     qemu_put_be32(f, cpu_get_sr(env));
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index c38e7b1268..2875307452 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -134,7 +134,7 @@ static int get_avr(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_avr(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     ppc_avr_t *v = pv;
 
@@ -166,7 +166,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_fpr(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     ppc_vsr_t *v = pv;
 
@@ -197,7 +197,7 @@ static int get_vsr(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_vsr(QEMUFile *f, void *pv, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     ppc_vsr_t *v = pv;
 
@@ -455,7 +455,7 @@ static int get_vscr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_vscr(QEMUFile *f, void *opaque, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     PowerPCCPU *cpu = opaque;
     qemu_put_be32(f, helper_mfvscr(&cpu->env));
@@ -580,7 +580,7 @@ static int get_slbe(QEMUFile *f, void *pv, size_t size,
 }
 
 static int put_slbe(QEMUFile *f, void *pv, size_t size,
-                    const VMStateField *field, QJSON *vmdesc)
+                    const VMStateField *field, JSONWriter *vmdesc)
 {
     ppc_slb_t *v = pv;
 
diff --git a/target/sparc/machine.c b/target/sparc/machine.c
index f38cf229af..917375c3a1 100644
--- a/target/sparc/machine.c
+++ b/target/sparc/machine.c
@@ -68,7 +68,7 @@ static int get_psr(QEMUFile *f, void *opaque, size_t size,
 }
 
 static int put_psr(QEMUFile *f, void *opaque, size_t size,
-                   const VMStateField *field, QJSON *vmdesc)
+                   const VMStateField *field, JSONWriter *vmdesc)
 {
     SPARCCPU *cpu = opaque;
     CPUSPARCState *env = &cpu->env;
diff --git a/migration/meson.build b/migration/meson.build
index 980e37865c..291adc1337 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -6,7 +6,6 @@ migration_files = files(
   'vmstate.c',
   'qemu-file-channel.c',
   'qemu-file.c',
-  'qjson.c',
 )
 softmmu_ss.add(migration_files)
 
-- 
2.26.2



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

* [PATCH 17/20] json: Use GString instead of QString to accumulate strings
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (15 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 16/20] migration: Replace migration's JSON writer by the general one Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 17:11 ` [PATCH 18/20] keyval: Use GString to accumulate value strings Markus Armbruster
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Change parse_string() to do build the initial string with GString.
This is another step towards making QString immutable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/json-parser.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index c0f521b56b..d351039b10 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -130,7 +130,7 @@ static int cvt4hex(const char *s)
 static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
 {
     const char *ptr = token->str;
-    QString *str;
+    GString *str;
     char quote;
     const char *beg;
     int cp, trailing;
@@ -140,7 +140,7 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
 
     assert(*ptr == '"' || *ptr == '\'');
     quote = *ptr++;
-    str = qstring_new();
+    str = g_string_new(NULL);
 
     while (*ptr != quote) {
         assert(*ptr);
@@ -149,31 +149,31 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
             beg = ptr++;
             switch (*ptr++) {
             case '"':
-                qstring_append_chr(str, '"');
+                g_string_append_c(str, '"');
                 break;
             case '\'':
-                qstring_append_chr(str, '\'');
+                g_string_append_c(str, '\'');
                 break;
             case '\\':
-                qstring_append_chr(str, '\\');
+                g_string_append_c(str, '\\');
                 break;
             case '/':
-                qstring_append_chr(str, '/');
+                g_string_append_c(str, '/');
                 break;
             case 'b':
-                qstring_append_chr(str, '\b');
+                g_string_append_c(str, '\b');
                 break;
             case 'f':
-                qstring_append_chr(str, '\f');
+                g_string_append_c(str, '\f');
                 break;
             case 'n':
-                qstring_append_chr(str, '\n');
+                g_string_append_c(str, '\n');
                 break;
             case 'r':
-                qstring_append_chr(str, '\r');
+                g_string_append_c(str, '\r');
                 break;
             case 't':
-                qstring_append_chr(str, '\t');
+                g_string_append_c(str, '\t');
                 break;
             case 'u':
                 cp = cvt4hex(ptr);
@@ -200,7 +200,7 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
                                 (int)(ptr - beg), beg);
                     goto out;
                 }
-                qstring_append(str, utf8_buf);
+                g_string_append(str, utf8_buf);
                 break;
             default:
                 parse_error(ctxt, token, "invalid escape sequence in string");
@@ -225,14 +225,14 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
             ptr = end;
             len = mod_utf8_encode(utf8_buf, sizeof(utf8_buf), cp);
             assert(len >= 0);
-            qstring_append(str, utf8_buf);
+            g_string_append(str, utf8_buf);
         }
     }
 
-    return str;
+    return qstring_from_gstring(str);
 
 out:
-    qobject_unref(str);
+    g_string_free(str, true);
     return NULL;
 }
 
-- 
2.26.2



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

* [PATCH 18/20] keyval: Use GString to accumulate value strings
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (16 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 17/20] json: Use GString instead of QString to accumulate strings Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-22  9:56   ` Paolo Bonzini
  2020-12-11 17:11 ` [PATCH 19/20] block: Use GString instead of QString to build filenames Markus Armbruster
                   ` (2 subsequent siblings)
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Change keyval_parse_one() to do build the initial string with GString.
This is another step towards making QString immutable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/keyval.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index 7f625ad33c..be34928813 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -189,7 +189,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
     QDict *cur;
     int ret;
     QObject *next;
-    QString *val;
+    GString *val;
 
     key = params;
     val_end = NULL;
@@ -263,7 +263,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
 
     if (key == implied_key) {
         assert(!*s);
-        val = qstring_from_substr(params, 0, val_end - params);
+        val = g_string_new_len(params, val_end - params);
         s = val_end;
         if (*s == ',') {
             s++;
@@ -276,7 +276,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
         }
         s++;
 
-        val = qstring_new();
+        val = g_string_new(NULL);
         for (;;) {
             if (!*s) {
                 break;
@@ -286,11 +286,12 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
                     break;
                 }
             }
-            qstring_append_chr(val, *s++);
+            g_string_append_c(val, *s++);
         }
     }
 
-    if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {
+    if (!keyval_parse_put(cur, key_in_cur, qstring_from_gstring(val),
+                          key, key_end, errp)) {
         return NULL;
     }
     return s;
-- 
2.26.2



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

* [PATCH 19/20] block: Use GString instead of QString to build filenames
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (17 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 18/20] keyval: Use GString to accumulate value strings Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-11 18:47   ` Vladimir Sementsov-Ogievskiy
  2020-12-11 17:11 ` [PATCH 20/20] qobject: Make QString immutable Markus Armbruster
  2020-12-22  9:59 ` [PATCH 00/20] Immutable QString, and also one JSON writer less Paolo Bonzini
  20 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, mdroth, qemu-block, Max Reitz

QString supports modifying its string, but it's quite limited: you can
only append.  Just one caller remains:
bdrv_parse_filename_strip_prefix() uses it just for building an
initial string.

Change it to do build the initial string with GString.  This is
another step towards making QString immutable.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 94d3a15081..75ffbe9092 100644
--- a/block.c
+++ b/block.c
@@ -216,7 +216,7 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
         /* Stripping the explicit protocol prefix may result in a protocol
          * prefix being (wrongly) detected (if the filename contains a colon) */
         if (path_has_protocol(filename)) {
-            QString *fat_filename;
+            GString *fat_filename;
 
             /* This means there is some colon before the first slash; therefore,
              * this cannot be an absolute path */
@@ -224,12 +224,13 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
 
             /* And we can thus fix the protocol detection issue by prefixing it
              * by "./" */
-            fat_filename = qstring_from_str("./");
-            qstring_append(fat_filename, filename);
+            fat_filename = g_string_new("./");
+            g_string_append(fat_filename, filename);
 
-            assert(!path_has_protocol(qstring_get_str(fat_filename)));
+            assert(!path_has_protocol(fat_filename->str));
 
-            qdict_put(options, "filename", fat_filename);
+            qdict_put(options, "filename",
+                      qstring_from_gstring(fat_filename));
         } else {
             /* If no protocol prefix was detected, we can use the shortened
              * filename as-is */
-- 
2.26.2



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

* [PATCH 20/20] qobject: Make QString immutable
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (18 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 19/20] block: Use GString instead of QString to build filenames Markus Armbruster
@ 2020-12-11 17:11 ` Markus Armbruster
  2020-12-22  9:59 ` [PATCH 00/20] Immutable QString, and also one JSON writer less Paolo Bonzini
  20 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-11 17:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth

The functions to modify a QString's string are all unused now.  Drop
them, and make the string immutable.  Saves 16 bytes per QString on my
system.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qstring.h |  8 +----
 qobject/qstring.c          | 65 ++------------------------------------
 tests/check-qobject.c      |  3 +-
 tests/check-qstring.c      | 16 ----------
 4 files changed, 4 insertions(+), 88 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 53567db6c0..1d8ba46936 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -17,19 +17,13 @@
 
 struct QString {
     struct QObjectBase_ base;
-    char *string;
-    size_t length;
-    size_t capacity;
+    const char *string;
 };
 
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, size_t start, size_t end);
 QString *qstring_from_gstring(GString *gstr);
-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);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qstring.c b/qobject/qstring.c
index ea86d80cf0..b4613899b9 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -24,14 +24,6 @@ QString *qstring_new(void)
     return qstring_from_str("");
 }
 
-/**
- * qstring_get_length(): Get the length of a QString
- */
-size_t qstring_get_length(const QString *qstring)
-{
-    return qstring->length;
-}
-
 /**
  * qstring_from_substr(): Create a new QString from a C string substring
  *
@@ -42,18 +34,9 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end)
     QString *qstring;
 
     assert(start <= end);
-
     qstring = g_malloc(sizeof(*qstring));
     qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
-
-    qstring->length = end - start;
-    qstring->capacity = qstring->length;
-
-    assert(qstring->capacity < SIZE_MAX);
-    qstring->string = g_malloc(qstring->capacity + 1);
-    memcpy(qstring->string, str + start, qstring->length);
-    qstring->string[qstring->length] = 0;
-
+    qstring->string = g_strndup(str + start, end - start);
     return qstring;
 }
 
@@ -79,55 +62,11 @@ QString *qstring_from_gstring(GString *gstr)
 
     qstring = g_malloc(sizeof(*qstring));
     qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
-    qstring->length = gstr->len;
-    qstring->capacity = gstr->allocated_len;
     qstring->string = g_string_free(gstr, false);
     return qstring;
 }
 
 
-static void capacity_increase(QString *qstring, size_t len)
-{
-    if (qstring->capacity < (qstring->length + len)) {
-        assert(len <= SIZE_MAX - qstring->capacity);
-        qstring->capacity += len;
-        assert(qstring->capacity <= SIZE_MAX / 2);
-        qstring->capacity *= 2; /* use exponential growth */
-
-        qstring->string = g_realloc(qstring->string, qstring->capacity + 1);
-    }
-}
-
-/* qstring_append(): Append a C string to a QString
- */
-void qstring_append(QString *qstring, const char *str)
-{
-    size_t len = strlen(str);
-
-    capacity_increase(qstring, len);
-    memcpy(qstring->string + qstring->length, str, len);
-    qstring->length += len;
-    qstring->string[qstring->length] = 0;
-}
-
-void qstring_append_int(QString *qstring, int64_t value)
-{
-    char num[32];
-
-    snprintf(num, sizeof(num), "%" PRId64, value);
-    qstring_append(qstring, num);
-}
-
-/**
- * qstring_append_chr(): Append a C char to a QString
- */
-void qstring_append_chr(QString *qstring, int c)
-{
-    capacity_increase(qstring, 1);
-    qstring->string[qstring->length++] = c;
-    qstring->string[qstring->length] = 0;
-}
-
 /**
  * qstring_get_str(): Return a pointer to the stored string
  *
@@ -158,6 +97,6 @@ void qstring_destroy_obj(QObject *obj)
 
     assert(obj != NULL);
     qs = qobject_to(QString, obj);
-    g_free(qs->string);
+    g_free((char *)qs->string);
     g_free(qs);
 }
diff --git a/tests/check-qobject.c b/tests/check-qobject.c
index 6b6deaeb8b..c1713d15af 100644
--- a/tests/check-qobject.c
+++ b/tests/check-qobject.c
@@ -155,8 +155,7 @@ static void qobject_is_equal_string_test(void)
     str_case = qstring_from_str("Foo");
 
     /* Should yield "foo" */
-    str_built = qstring_from_substr("form", 0, 2);
-    qstring_append_chr(str_built, 'o');
+    str_built = qstring_from_substr("buffoon", 3, 6);
 
     check_unequal(str_base, str_whitespace_0, str_whitespace_1,
                   str_whitespace_2, str_whitespace_3, str_case);
diff --git a/tests/check-qstring.c b/tests/check-qstring.c
index 2d079921e3..4bf9772093 100644
--- a/tests/check-qstring.c
+++ b/tests/check-qstring.c
@@ -47,21 +47,6 @@ static void qstring_get_str_test(void)
     qobject_unref(qstring);
 }
 
-static void qstring_append_chr_test(void)
-{
-    int i;
-    QString *qstring;
-    const char *str = "qstring append char unit-test";
-
-    qstring = qstring_new();
-
-    for (i = 0; str[i]; i++)
-        qstring_append_chr(qstring, str[i]);
-
-    g_assert(strcmp(str, qstring_get_str(qstring)) == 0);
-    qobject_unref(qstring);
-}
-
 static void qstring_from_substr_test(void)
 {
     QString *qs;
@@ -90,7 +75,6 @@ int main(int argc, char **argv)
 
     g_test_add_func("/public/from_str", qstring_from_str_test);
     g_test_add_func("/public/get_str", qstring_get_str_test);
-    g_test_add_func("/public/append_chr", qstring_append_chr_test);
     g_test_add_func("/public/from_substr", qstring_from_substr_test);
     g_test_add_func("/public/to_qstring", qobject_to_qstring_test);
 
-- 
2.26.2



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

* Re: [PATCH 10/20] block: Avoid qobject_get_try_str()
  2020-12-11 17:11 ` [PATCH 10/20] block: Avoid qobject_get_try_str() Markus Armbruster
@ 2020-12-11 18:28   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-12-11 18:28 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: Kevin Wolf, mdroth, qemu-block, Max Reitz

11.12.2020 20:11, Markus Armbruster wrote:
> I'm about to remove qobject_get_try_str().  Use qstring_get_str()
> instead.  Safe because the argument is known to be a QString here.
> 
> Cc: Kevin Wolf<kwolf@redhat.com>
> Cc: Max Reitz<mreitz@redhat.com>
> Cc:qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster<armbru@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir


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

* Re: [PATCH 19/20] block: Use GString instead of QString to build filenames
  2020-12-11 17:11 ` [PATCH 19/20] block: Use GString instead of QString to build filenames Markus Armbruster
@ 2020-12-11 18:47   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-12-11 18:47 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: Kevin Wolf, mdroth, qemu-block, Max Reitz

11.12.2020 20:11, Markus Armbruster wrote:
> QString supports modifying its string, but it's quite limited: you can
> only append.  Just one caller remains:
> bdrv_parse_filename_strip_prefix() uses it just for building an
> initial string.
> 
> Change it to do build the initial string with GString.  This is
> another step towards making QString immutable.
> 
> Cc: Kevin Wolf<kwolf@redhat.com>
> Cc: Max Reitz<mreitz@redhat.com>
> Cc:qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster<armbru@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir


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

* Re: [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API"
  2020-12-11 17:11 ` [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API" Markus Armbruster
@ 2020-12-11 19:48   ` Eduardo Habkost
  0 siblings, 0 replies; 36+ messages in thread
From: Eduardo Habkost @ 2020-12-11 19:48 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Paolo Bonzini, Daniel P . Berrangé, qemu-devel, mdroth

On Fri, Dec 11, 2020 at 06:11:43PM +0100, Markus Armbruster wrote:
> Commit aafb21a0b9 "qobject: let object_property_get_str() use new API"
> isn't much of a simplification.  Not worth having
> object_property_get_str() differ from the other
> object_property_get_FOO().  Revert.
> 
> This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo



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

* Re: [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output
  2020-12-11 17:11 ` [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output Markus Armbruster
@ 2020-12-16 18:53   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 36+ messages in thread
From: Dr. David Alan Gilbert @ 2020-12-16 18:53 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, mdroth

* Markus Armbruster (armbru@redhat.com) wrote:
> Commit 48c043d0d1 "hmp: human-monitor-command: stop using the Memory
> chardev driver" left us "if string is non-empty, duplicate it, else
> duplicate the empty string".  Meh.  Duplicate it unconditionally.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  monitor/misc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 398211a034..6c3e8506a9 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -136,11 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>      handle_hmp_command(&hmp, command_line);
>  
>      WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
> -        if (qstring_get_length(hmp.common.outbuf) > 0) {
> -            output = g_strdup(qstring_get_str(hmp.common.outbuf));
> -        } else {
> -            output = g_strdup("");
> -        }
> +        output = g_strdup(qstring_get_str(hmp.common.outbuf));
>      }
>  
>  out:
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH 02/20] monitor: Use GString instead of QString for output buffer
  2020-12-11 17:11 ` [PATCH 02/20] monitor: Use GString instead of QString for output buffer Markus Armbruster
@ 2020-12-16 19:36   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 36+ messages in thread
From: Dr. David Alan Gilbert @ 2020-12-16 19:36 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, mdroth

* Markus Armbruster (armbru@redhat.com) wrote:
> GString has a richer set of string operations than QString.  It should
> be preferred to QString except where we need a QObject or reference
> counting.  We don't here.  Switch to GString, and put its richer
> interface to use.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  monitor/monitor-internal.h |  2 +-
>  monitor/misc.c             |  2 +-
>  monitor/monitor.c          | 20 ++++++++------------
>  3 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index a6131554da..40903d6386 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -105,7 +105,7 @@ struct Monitor {
>       * Members that are protected by the per-monitor lock
>       */
>      QLIST_HEAD(, mon_fd_t) fds;
> -    QString *outbuf;
> +    GString *outbuf;
>      guint out_watch;
>      /* Read under either BQL or mon_lock, written with BQL+mon_lock.  */
>      int mux_out;
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 6c3e8506a9..814d22de11 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -136,7 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>      handle_hmp_command(&hmp, command_line);
>  
>      WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
> -        output = g_strdup(qstring_get_str(hmp.common.outbuf));
> +        output = g_strdup(hmp.common.outbuf->str);
>      }
>  
>  out:
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 84222cd130..1e4a6b3f20 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -29,7 +29,6 @@
>  #include "qapi/qapi-emit-events.h"
>  #include "qapi/qapi-visit-control.h"
>  #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qstring.h"
>  #include "qemu/error-report.h"
>  #include "qemu/option.h"
>  #include "sysemu/qtest.h"
> @@ -181,22 +180,19 @@ static void monitor_flush_locked(Monitor *mon)
>          return;
>      }
>  
> -    buf = qstring_get_str(mon->outbuf);
> -    len = qstring_get_length(mon->outbuf);
> +    buf = mon->outbuf->str;
> +    len = mon->outbuf->len;
>  
>      if (len && !mon->mux_out) {
>          rc = qemu_chr_fe_write(&mon->chr, (const uint8_t *) buf, len);
>          if ((rc < 0 && errno != EAGAIN) || (rc == len)) {
>              /* all flushed or error */
> -            qobject_unref(mon->outbuf);
> -            mon->outbuf = qstring_new();
> +            g_string_truncate(mon->outbuf, 0);
>              return;
>          }
>          if (rc > 0) {
>              /* partial write */
> -            QString *tmp = qstring_from_str(buf + rc);
> -            qobject_unref(mon->outbuf);
> -            mon->outbuf = tmp;
> +            g_string_erase(mon->outbuf, 0, rc);
>          }
>          if (mon->out_watch == 0) {
>              mon->out_watch =
> @@ -223,9 +219,9 @@ int monitor_puts(Monitor *mon, const char *str)
>      for (i = 0; str[i]; i++) {
>          c = str[i];
>          if (c == '\n') {
> -            qstring_append_chr(mon->outbuf, '\r');
> +            g_string_append_c(mon->outbuf, '\r');
>          }
> -        qstring_append_chr(mon->outbuf, c);
> +        g_string_append_c(mon->outbuf, c);
>          if (c == '\n') {
>              monitor_flush_locked(mon);
>          }
> @@ -602,7 +598,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
>      }
>      qemu_mutex_init(&mon->mon_lock);
>      mon->is_qmp = is_qmp;
> -    mon->outbuf = qstring_new();
> +    mon->outbuf = g_string_new(NULL);
>      mon->skip_flush = skip_flush;
>      mon->use_io_thread = use_io_thread;
>  }
> @@ -616,7 +612,7 @@ void monitor_data_destroy(Monitor *mon)
>      } else {
>          readline_free(container_of(mon, MonitorHMP, common)->rs);
>      }
> -    qobject_unref(mon->outbuf);
> +    g_string_free(mon->outbuf, true);
>      qemu_mutex_destroy(&mon->mon_lock);
>  }
>  
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH 16/20] migration: Replace migration's JSON writer by the general one
  2020-12-11 17:11 ` [PATCH 16/20] migration: Replace migration's JSON writer by the general one Markus Armbruster
@ 2020-12-16 19:46   ` Dr. David Alan Gilbert
  2020-12-17  7:10     ` Markus Armbruster
  0 siblings, 1 reply; 36+ messages in thread
From: Dr. David Alan Gilbert @ 2020-12-16 19:46 UTC (permalink / raw)
  To: Markus Armbruster, agraf; +Cc: Juan Quintela, qemu-devel, mdroth

* Markus Armbruster (armbru@redhat.com) wrote:
> Commit 8118f0950f "migration: Append JSON description of migration
> stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
> good fit, because it requires building a QObject to convert.  Instead,
> migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
> Add JSON writer".  It tacitly limits numbers to int64_t, and strings
> contents to characters that don't need escaping, unlike
> qobject_to_json().
> 
> The previous commit factored the JSON writer out of qobject_to_json().
> Replace migration's JSON writer by it.
> 
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

(Copying in Alex)

This looks OK to me, so:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

but, can I just check, have you checked scripts/analyze-migration.py is
still happy with the output?

Dave

> ---
>  include/migration/vmstate.h    |   7 +-
>  include/qapi/qmp/json-writer.h |   2 -
>  include/qemu/typedefs.h        |   4 +-
>  migration/qjson.h              |  29 ---------
>  hw/display/virtio-gpu.c        |   2 +-
>  hw/intc/s390_flic_kvm.c        |   2 +-
>  hw/nvram/eeprom93xx.c          |   2 +-
>  hw/nvram/fw_cfg.c              |   2 +-
>  hw/pci/msix.c                  |   2 +-
>  hw/pci/pci.c                   |   4 +-
>  hw/pci/shpc.c                  |   2 +-
>  hw/rtc/twl92230.c              |   2 +-
>  hw/scsi/scsi-bus.c             |   2 +-
>  hw/usb/redirect.c              |   7 +-
>  hw/virtio/virtio.c             |   4 +-
>  migration/qjson.c              | 114 ---------------------------------
>  migration/savevm.c             |  53 +++++++--------
>  migration/vmstate-types.c      |  38 +++++------
>  migration/vmstate.c            |  52 +++++++--------
>  target/alpha/machine.c         |   2 +-
>  target/arm/machine.c           |   6 +-
>  target/avr/machine.c           |   4 +-
>  target/hppa/machine.c          |   4 +-
>  target/microblaze/machine.c    |   2 +-
>  target/mips/machine.c          |   4 +-
>  target/openrisc/machine.c      |   2 +-
>  target/ppc/machine.c           |  10 +--
>  target/sparc/machine.c         |   2 +-
>  migration/meson.build          |   1 -
>  29 files changed, 114 insertions(+), 253 deletions(-)
>  delete mode 100644 migration/qjson.h
>  delete mode 100644 migration/qjson.c
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 4d71dc8fba..075ee80096 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -43,7 +43,7 @@ struct VMStateInfo {
>      const char *name;
>      int (*get)(QEMUFile *f, void *pv, size_t size, const VMStateField *field);
>      int (*put)(QEMUFile *f, void *pv, size_t size, const VMStateField *field,
> -               QJSON *vmdesc);
> +               JSONWriter *vmdesc);
>  };
>  
>  enum VMStateFlags {
> @@ -1169,9 +1169,10 @@ extern const VMStateInfo vmstate_info_qlist;
>  int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>                         void *opaque, int version_id);
>  int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
> -                       void *opaque, QJSON *vmdesc);
> +                       void *opaque, JSONWriter *vmdesc);
>  int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> -                         void *opaque, QJSON *vmdesc, int version_id);
> +                         void *opaque, JSONWriter *vmdesc,
> +                         int version_id);
>  
>  bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
>  
> diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h
> index 708d129018..b70ba64077 100644
> --- a/include/qapi/qmp/json-writer.h
> +++ b/include/qapi/qmp/json-writer.h
> @@ -14,8 +14,6 @@
>  #ifndef JSON_WRITER_H
>  #define JSON_WRITER_H
>  
> -typedef struct JSONWriter JSONWriter;
> -
>  JSONWriter *json_writer_new(bool pretty);
>  const char *json_writer_get(JSONWriter *);
>  GString *json_writer_get_and_free(JSONWriter *);
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 6281eae3b5..976b529dfb 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -57,8 +57,8 @@ typedef struct IOMMUMemoryRegion IOMMUMemoryRegion;
>  typedef struct ISABus ISABus;
>  typedef struct ISADevice ISADevice;
>  typedef struct IsaDma IsaDma;
> +typedef struct JSONWriter JSONWriter;
>  typedef struct MACAddr MACAddr;
> -typedef struct ReservedRegion ReservedRegion;
>  typedef struct MachineClass MachineClass;
>  typedef struct MachineState MachineState;
>  typedef struct MemoryListener MemoryListener;
> @@ -107,7 +107,6 @@ typedef struct QEMUSGList QEMUSGList;
>  typedef struct QemuSpin QemuSpin;
>  typedef struct QEMUTimer QEMUTimer;
>  typedef struct QEMUTimerListGroup QEMUTimerListGroup;
> -typedef struct QJSON QJSON;
>  typedef struct QList QList;
>  typedef struct QNull QNull;
>  typedef struct QNum QNum;
> @@ -115,6 +114,7 @@ typedef struct QObject QObject;
>  typedef struct QString QString;
>  typedef struct RAMBlock RAMBlock;
>  typedef struct Range Range;
> +typedef struct ReservedRegion ReservedRegion;
>  typedef struct SavedIOTLB SavedIOTLB;
>  typedef struct SHPCDevice SHPCDevice;
>  typedef struct SSIBus SSIBus;
> diff --git a/migration/qjson.h b/migration/qjson.h
> deleted file mode 100644
> index 1786bb5864..0000000000
> --- a/migration/qjson.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/*
> - * QEMU JSON writer
> - *
> - * Copyright Alexander Graf
> - *
> - * Authors:
> - *  Alexander Graf <agraf@suse.de>
> - *
> - * 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 QEMU_QJSON_H
> -#define QEMU_QJSON_H
> -
> -QJSON *qjson_new(void);
> -void qjson_destroy(QJSON *json);
> -void json_prop_str(QJSON *json, const char *name, const char *str);
> -void json_prop_int(QJSON *json, const char *name, int64_t val);
> -void json_end_array(QJSON *json);
> -void json_start_array(QJSON *json, const char *name);
> -void json_end_object(QJSON *json);
> -void json_start_object(QJSON *json, const char *name);
> -const char *qjson_get_str(QJSON *json);
> -void qjson_finish(QJSON *json);
> -
> -G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy)
> -
> -#endif /* QEMU_QJSON_H */
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index f3b71fa9c7..0e833a462b 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -970,7 +970,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
>  };
>  
>  static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
> -                           const VMStateField *field, QJSON *vmdesc)
> +                           const VMStateField *field, JSONWriter *vmdesc)
>  {
>      VirtIOGPU *g = opaque;
>      struct virtio_gpu_simple_resource *res;
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index 35d91afa55..b3fb9f8395 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -386,7 +386,7 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
>   * reached
>   */
>  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> -                         const VMStateField *field, QJSON *vmdesc)
> +                         const VMStateField *field, JSONWriter *vmdesc)
>  {
>      KVMS390FLICState *flic = opaque;
>      int len = FLIC_SAVE_INITIAL_SIZE;
> diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
> index ca6f591c84..a1b9c78844 100644
> --- a/hw/nvram/eeprom93xx.c
> +++ b/hw/nvram/eeprom93xx.c
> @@ -104,7 +104,7 @@ static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_unused(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      fprintf(stderr, "uint16_from_uint8 is used only for backwards compatibility.\n");
>      fprintf(stderr, "Never should be used to write a new state.\n");
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 282ba93e2e..4a15aadd36 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -582,7 +582,7 @@ static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_unused(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      fprintf(stderr, "uint32_as_uint16 is only used for backward compatibility.\n");
>      fprintf(stderr, "This functions shouldn't be called.\n");
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index 67e34f34d6..cd13ef558f 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -631,7 +631,7 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>  }
>  
>  static int put_msix_state(QEMUFile *f, void *pv, size_t size,
> -                          const VMStateField *field, QJSON *vmdesc)
> +                          const VMStateField *field, JSONWriter *vmdesc)
>  {
>      msix_save(pv, f);
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 0131d9d02c..25f2755c71 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -558,7 +558,7 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size,
>  
>  /* just put buffer */
>  static int put_pci_config_device(QEMUFile *f, void *pv, size_t size,
> -                                 const VMStateField *field, QJSON *vmdesc)
> +                                 const VMStateField *field, JSONWriter *vmdesc)
>  {
>      const uint8_t **v = pv;
>      assert(size == pci_config_size(container_of(pv, PCIDevice, config)));
> @@ -596,7 +596,7 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_pci_irq_state(QEMUFile *f, void *pv, size_t size,
> -                             const VMStateField *field, QJSON *vmdesc)
> +                             const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int i;
>      PCIDevice *s = container_of(pv, PCIDevice, irq_state);
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index b00dce629c..4786a44996 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -699,7 +699,7 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
>  }
>  
>  static int shpc_save(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      PCIDevice *d = container_of(pv, PCIDevice, shpc);
>      qemu_put_buffer(f, d->shpc->config, SHPC_SIZEOF(d));
> diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c
> index f838913b37..d942908223 100644
> --- a/hw/rtc/twl92230.c
> +++ b/hw/rtc/twl92230.c
> @@ -762,7 +762,7 @@ static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
> -                               const VMStateField *field, QJSON *vmdesc)
> +                               const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int *v = pv;
>      qemu_put_be16(f, *v);
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index b901e701f0..086dd263e0 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -1634,7 +1634,7 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
>  /* SCSI request list.  For simplicity, pv points to the whole device */
>  
>  static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
> -                             const VMStateField *field, QJSON *vmdesc)
> +                             const VMStateField *field, JSONWriter *vmdesc)
>  {
>      SCSIDevice *s = pv;
>      SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 3238de6bb8..4ca7d47ef7 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -2227,7 +2227,7 @@ static int usbredir_post_load(void *priv, int version_id)
>  
>  /* For usbredirparser migration */
>  static int usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
> -                               const VMStateField *field, QJSON *vmdesc)
> +                               const VMStateField *field, JSONWriter *vmdesc)
>  {
>      USBRedirDevice *dev = priv;
>      uint8_t *data;
> @@ -2294,7 +2294,7 @@ static const VMStateInfo usbredir_parser_vmstate_info = {
>  
>  /* For buffered packets (iso/irq) queue migration */
>  static int usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
> -                              const VMStateField *field, QJSON *vmdesc)
> +                              const VMStateField *field, JSONWriter *vmdesc)
>  {
>      struct endp_data *endp = priv;
>      USBRedirDevice *dev = endp->dev;
> @@ -2421,7 +2421,8 @@ static const VMStateDescription usbredir_ep_vmstate = {
>  
>  /* For PacketIdQueue migration */
>  static int usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
> -                                    const VMStateField *field, QJSON *vmdesc)
> +                                    const VMStateField *field,
> +                                    JSONWriter *vmdesc)
>  {
>      struct PacketIdQueue *q = priv;
>      USBRedirDevice *dev = q->dev;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index eff35fab7c..b308026596 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2745,7 +2745,7 @@ static int get_extra_state(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_extra_state(QEMUFile *f, void *pv, size_t size,
> -                           const VMStateField *field, QJSON *vmdesc)
> +                           const VMStateField *field, JSONWriter *vmdesc)
>  {
>      VirtIODevice *vdev = pv;
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> @@ -2919,7 +2919,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  
>  /* A wrapper for use as a VMState .put function */
>  static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
> -                              const VMStateField *field, QJSON *vmdesc)
> +                              const VMStateField *field, JSONWriter *vmdesc)
>  {
>      return virtio_save(VIRTIO_DEVICE(opaque), f);
>  }
> diff --git a/migration/qjson.c b/migration/qjson.c
> deleted file mode 100644
> index e9889bdcb0..0000000000
> --- a/migration/qjson.c
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -/*
> - * A simple JSON writer
> - *
> - * Copyright Alexander Graf
> - *
> - * Authors:
> - *  Alexander Graf <agraf@suse.de>
> - *
> - * 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.
> - *
> - */
> -
> -/*
> - * Type QJSON lets you build JSON text.  Its interface mirrors (a
> - * subset of) abstract JSON syntax.
> - *
> - * It does *not* detect incorrect use.  It happily produces invalid
> - * JSON then.  This is what migration wants.
> - *
> - * QAPI output visitors also produce JSON text.  However, they do
> - * assert their preconditions and invariants, and therefore abort on
> - * incorrect use.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/qmp/qstring.h"
> -#include "qjson.h"
> -
> -struct QJSON {
> -    QString *str;
> -    bool omit_comma;
> -};
> -
> -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->str, "\"");
> -        qstring_append(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;
> -}
> -
> -void json_end_object(QJSON *json)
> -{
> -    qstring_append(json->str, " }");
> -    json->omit_comma = false;
> -}
> -
> -void json_start_array(QJSON *json, const char *name)
> -{
> -    json_emit_element(json, name);
> -    qstring_append(json->str, "[ ");
> -    json->omit_comma = true;
> -}
> -
> -void json_end_array(QJSON *json)
> -{
> -    qstring_append(json->str, " ]");
> -    json->omit_comma = false;
> -}
> -
> -void json_prop_int(QJSON *json, const char *name, int64_t val)
> -{
> -    json_emit_element(json, name);
> -    qstring_append_int(json->str, 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, '"');
> -}
> -
> -const char *qjson_get_str(QJSON *json)
> -{
> -    return qstring_get_str(json->str);
> -}
> -
> -QJSON *qjson_new(void)
> -{
> -    QJSON *json = g_new0(QJSON, 1);
> -
> -    json->str = qstring_from_str("{ ");
> -    json->omit_comma = true;
> -    return json;
> -}
> -
> -void qjson_finish(QJSON *json)
> -{
> -    json_end_object(json);
> -}
> -
> -void qjson_destroy(QJSON *json)
> -{
> -    qobject_unref(json->str);
> -    g_free(json);
> -}
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5f937a2762..138a96de4d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -42,6 +42,7 @@
>  #include "postcopy-ram.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-migration.h"
> +#include "qapi/qmp/json-writer.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/cpus.h"
> @@ -58,7 +59,6 @@
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/xen.h"
> -#include "qjson.h"
>  #include "migration/colo.h"
>  #include "qemu/bitmap.h"
>  #include "net/announce.h"
> @@ -209,7 +209,7 @@ static int get_timer(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_timer(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      QEMUTimer *v = pv;
>      timer_put(f, v);
> @@ -406,7 +406,7 @@ static int get_capability(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_capability(QEMUFile *f, void *pv, size_t size,
> -                          const VMStateField *field, QJSON *vmdesc)
> +                          const VMStateField *field, JSONWriter *vmdesc)
>  {
>      MigrationCapability *capability = pv;
>      const char *capability_str = MigrationCapability_str(*capability);
> @@ -884,7 +884,8 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
>      return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
>  }
>  
> -static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
> +static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
> +                                   JSONWriter *vmdesc)
>  {
>      int64_t old_offset, size;
>  
> @@ -893,18 +894,19 @@ 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);
> +        json_writer_int64(vmdesc, "size", size);
> +        json_writer_start_array(vmdesc, "fields");
> +        json_writer_start_object(vmdesc, NULL);
> +        json_writer_str(vmdesc, "name", "data");
> +        json_writer_int64(vmdesc, "size", size);
> +        json_writer_str(vmdesc, "type", "buffer");
> +        json_writer_end_object(vmdesc);
> +        json_writer_end_array(vmdesc);
>      }
>  }
>  
> -static int vmstate_save(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
> +static int vmstate_save(QEMUFile *f, SaveStateEntry *se,
> +                        JSONWriter *vmdesc)
>  {
>      trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
>      if (!se->vmsd) {
> @@ -1357,14 +1359,15 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>                                                      bool in_postcopy,
>                                                      bool inactivate_disks)
>  {
> -    g_autoptr(QJSON) vmdesc = NULL;
> +    g_autoptr(JSONWriter) vmdesc = NULL;
>      int vmdesc_len;
>      SaveStateEntry *se;
>      int ret;
>  
> -    vmdesc = qjson_new();
> -    json_prop_int(vmdesc, "page_size", qemu_target_page_size());
> -    json_start_array(vmdesc, "devices");
> +    vmdesc = json_writer_new(false);
> +    json_writer_start_object(vmdesc, NULL);
> +    json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
> +    json_writer_start_array(vmdesc, "devices");
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>  
>          if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> @@ -1377,9 +1380,9 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>  
>          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);
> +        json_writer_start_object(vmdesc, NULL);
> +        json_writer_str(vmdesc, "name", se->idstr);
> +        json_writer_int64(vmdesc, "instance_id", se->instance_id);
>  
>          save_section_header(f, se, QEMU_VM_SECTION_FULL);
>          ret = vmstate_save(f, se, vmdesc);
> @@ -1390,7 +1393,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>          trace_savevm_section_end(se->idstr, se->section_id, 0);
>          save_section_footer(f, se);
>  
> -        json_end_object(vmdesc);
> +        json_writer_end_object(vmdesc);
>      }
>  
>      if (inactivate_disks) {
> @@ -1409,14 +1412,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>          qemu_put_byte(f, QEMU_VM_EOF);
>      }
>  
> -    json_end_array(vmdesc);
> -    qjson_finish(vmdesc);
> -    vmdesc_len = strlen(qjson_get_str(vmdesc));
> +    json_writer_end_array(vmdesc);
> +    json_writer_end_object(vmdesc);
> +    vmdesc_len = strlen(json_writer_get(vmdesc));
>  
>      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 *)json_writer_get(vmdesc), vmdesc_len);
>      }
>  
>      return 0;
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index e22d41d73d..bf4d440308 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -29,7 +29,7 @@ static int get_bool(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_bool(QEMUFile *f, void *pv, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      bool *v = pv;
>      qemu_put_byte(f, *v);
> @@ -53,7 +53,7 @@ static int get_int8(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_int8(QEMUFile *f, void *pv, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int8_t *v = pv;
>      qemu_put_s8s(f, v);
> @@ -77,7 +77,7 @@ static int get_int16(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_int16(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int16_t *v = pv;
>      qemu_put_sbe16s(f, v);
> @@ -101,7 +101,7 @@ static int get_int32(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_int32(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int32_t *v = pv;
>      qemu_put_sbe32s(f, v);
> @@ -178,7 +178,7 @@ static int get_int64(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_int64(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int64_t *v = pv;
>      qemu_put_sbe64s(f, v);
> @@ -202,7 +202,7 @@ static int get_uint8(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_uint8(QEMUFile *f, void *pv, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint8_t *v = pv;
>      qemu_put_8s(f, v);
> @@ -226,7 +226,7 @@ static int get_uint16(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_uint16(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint16_t *v = pv;
>      qemu_put_be16s(f, v);
> @@ -250,7 +250,7 @@ static int get_uint32(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_uint32(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint32_t *v = pv;
>      qemu_put_be32s(f, v);
> @@ -300,7 +300,7 @@ static int get_uint64(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_uint64(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint64_t *v = pv;
>      qemu_put_be64s(f, v);
> @@ -325,7 +325,7 @@ static int get_nullptr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_nullptr(QEMUFile *f, void *pv, size_t size,
> -                        const VMStateField *field, QJSON *vmdesc)
> +                        const VMStateField *field, JSONWriter *vmdesc)
>  
>  {
>      if (pv == NULL) {
> @@ -432,7 +432,7 @@ static int get_cpudouble(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_cpudouble(QEMUFile *f, void *pv, size_t size,
> -                         const VMStateField *field, QJSON *vmdesc)
> +                         const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPU_DoubleU *v = pv;
>      qemu_put_be32s(f, &v->l.upper);
> @@ -457,7 +457,7 @@ static int get_buffer(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_buffer(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint8_t *v = pv;
>      qemu_put_buffer(f, v, size);
> @@ -488,7 +488,7 @@ static int get_unused_buffer(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_unused_buffer(QEMUFile *f, void *pv, size_t size,
> -                             const VMStateField *field, QJSON *vmdesc)
> +                             const VMStateField *field, JSONWriter *vmdesc)
>  {
>      static const uint8_t buf[1024];
>      int block_len;
> @@ -530,7 +530,7 @@ static int get_tmp(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_tmp(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      const VMStateDescription *vmsd = field->vmsd;
>      void *tmp = g_malloc(size);
> @@ -573,7 +573,7 @@ static int get_bitmap(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_bitmap(QEMUFile *f, void *pv, size_t size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      unsigned long *bmp = pv;
>      int i, idx = 0;
> @@ -637,7 +637,7 @@ static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
>  
>  /* put for QTAILQ */
>  static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,
> -                      const VMStateField *field, QJSON *vmdesc)
> +                      const VMStateField *field, JSONWriter *vmdesc)
>  {
>      const VMStateDescription *vmsd = field->vmsd;
>      /* offset of the QTAILQ entry in a QTAILQ element*/
> @@ -670,7 +670,7 @@ struct put_gtree_data {
>      QEMUFile *f;
>      const VMStateDescription *key_vmsd;
>      const VMStateDescription *val_vmsd;
> -    QJSON *vmdesc;
> +    JSONWriter *vmdesc;
>      int ret;
>  };
>  
> @@ -703,7 +703,7 @@ static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data)
>  }
>  
>  static int put_gtree(QEMUFile *f, void *pv, size_t unused_size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      bool direct_key = (!field->start);
>      const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[1];
> @@ -819,7 +819,7 @@ const VMStateInfo vmstate_info_gtree = {
>  };
>  
>  static int put_qlist(QEMUFile *f, void *pv, size_t unused_size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      const VMStateDescription *vmsd = field->vmsd;
>      /* offset of the QTAILQ entry in a QTAILQ element*/
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index e9d2aef66b..05f87cdddc 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -14,14 +14,14 @@
>  #include "migration.h"
>  #include "migration/vmstate.h"
>  #include "savevm.h"
> +#include "qapi/qmp/json-writer.h"
>  #include "qemu-file.h"
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "trace.h"
> -#include "qjson.h"
>  
>  static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
> -                                   void *opaque, QJSON *vmdesc);
> +                                   void *opaque, JSONWriter *vmdesc);
>  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>                                     void *opaque);
>  
> @@ -249,7 +249,8 @@ static bool vmsd_can_compress(const VMStateField *field)
>      return true;
>  }
>  
> -static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc,
> +static void vmsd_desc_field_start(const VMStateDescription *vmsd,
> +                                  JSONWriter *vmdesc,
>                                    const VMStateField *field, int i, int max)
>  {
>      char *name, *old_name;
> @@ -270,25 +271,26 @@ static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc,
>          g_free(old_name);
>      }
>  
> -    json_start_object(vmdesc, NULL);
> -    json_prop_str(vmdesc, "name", name);
> +    json_writer_start_object(vmdesc, NULL);
> +    json_writer_str(vmdesc, "name", name);
>      if (is_array) {
>          if (can_compress) {
> -            json_prop_int(vmdesc, "array_len", max);
> +            json_writer_int64(vmdesc, "array_len", max);
>          } else {
> -            json_prop_int(vmdesc, "index", i);
> +            json_writer_int64(vmdesc, "index", i);
>          }
>      }
> -    json_prop_str(vmdesc, "type", vmfield_get_type_name(field));
> +    json_writer_str(vmdesc, "type", vmfield_get_type_name(field));
>  
>      if (field->flags & VMS_STRUCT) {
> -        json_start_object(vmdesc, "struct");
> +        json_writer_start_object(vmdesc, "struct");
>      }
>  
>      g_free(name);
>  }
>  
> -static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
> +static void vmsd_desc_field_end(const VMStateDescription *vmsd,
> +                                JSONWriter *vmdesc,
>                                  const VMStateField *field, size_t size, int i)
>  {
>      if (!vmdesc) {
> @@ -297,11 +299,11 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
>  
>      if (field->flags & VMS_STRUCT) {
>          /* We printed a struct in between, close its child object */
> -        json_end_object(vmdesc);
> +        json_writer_end_object(vmdesc);
>      }
>  
> -    json_prop_int(vmdesc, "size", size);
> -    json_end_object(vmdesc);
> +    json_writer_int64(vmdesc, "size", size);
> +    json_writer_end_object(vmdesc);
>  }
>  
>  
> @@ -316,13 +318,13 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
>  
>  
>  int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
> -                       void *opaque, QJSON *vmdesc_id)
> +                       void *opaque, JSONWriter *vmdesc_id)
>  {
>      return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id);
>  }
>  
>  int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> -                         void *opaque, QJSON *vmdesc, int version_id)
> +                         void *opaque, JSONWriter *vmdesc, int version_id)
>  {
>      int ret = 0;
>      const VMStateField *field = vmsd->fields;
> @@ -339,9 +341,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>      }
>  
>      if (vmdesc) {
> -        json_prop_str(vmdesc, "vmsd_name", vmsd->name);
> -        json_prop_int(vmdesc, "version", version_id);
> -        json_start_array(vmdesc, "fields");
> +        json_writer_str(vmdesc, "vmsd_name", vmsd->name);
> +        json_writer_int64(vmdesc, "version", version_id);
> +        json_writer_start_array(vmdesc, "fields");
>      }
>  
>      while (field->name) {
> @@ -353,7 +355,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>              int i, n_elems = vmstate_n_elems(opaque, field);
>              int size = vmstate_size(opaque, field);
>              int64_t old_offset, written_bytes;
> -            QJSON *vmdesc_loop = vmdesc;
> +            JSONWriter *vmdesc_loop = vmdesc;
>  
>              trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
>              if (field->flags & VMS_POINTER) {
> @@ -413,7 +415,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>      }
>  
>      if (vmdesc) {
> -        json_end_array(vmdesc);
> +        json_writer_end_array(vmdesc);
>      }
>  
>      ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc);
> @@ -491,7 +493,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>  }
>  
>  static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
> -                                   void *opaque, QJSON *vmdesc)
> +                                   void *opaque, JSONWriter *vmdesc)
>  {
>      const VMStateDescription **sub = vmsd->subsections;
>      bool vmdesc_has_subsections = false;
> @@ -507,11 +509,11 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
>              if (vmdesc) {
>                  /* Only create subsection array when we have any */
>                  if (!vmdesc_has_subsections) {
> -                    json_start_array(vmdesc, "subsections");
> +                    json_writer_start_array(vmdesc, "subsections");
>                      vmdesc_has_subsections = true;
>                  }
>  
> -                json_start_object(vmdesc, NULL);
> +                json_writer_start_object(vmdesc, NULL);
>              }
>  
>              qemu_put_byte(f, QEMU_VM_SUBSECTION);
> @@ -525,14 +527,14 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
>              }
>  
>              if (vmdesc) {
> -                json_end_object(vmdesc);
> +                json_writer_end_object(vmdesc);
>              }
>          }
>          sub++;
>      }
>  
>      if (vmdesc_has_subsections) {
> -        json_end_array(vmdesc);
> +        json_writer_end_array(vmdesc);
>      }
>  
>      return ret;
> diff --git a/target/alpha/machine.c b/target/alpha/machine.c
> index 9d20169d4f..2b7c8148ff 100644
> --- a/target/alpha/machine.c
> +++ b/target/alpha/machine.c
> @@ -11,7 +11,7 @@ static int get_fpcr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_fpcr(QEMUFile *f, void *opaque, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPUAlphaState *env = opaque;
>      qemu_put_be64(f, cpu_alpha_load_fpcr(env));
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index c5a2114f51..581852bc53 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -27,7 +27,7 @@ static int get_fpscr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_fpscr(QEMUFile *f, void *opaque, size_t size,
> -                     const VMStateField *field, QJSON *vmdesc)
> +                     const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ARMCPU *cpu = opaque;
>      CPUARMState *env = &cpu->env;
> @@ -573,7 +573,7 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_cpsr(QEMUFile *f, void *opaque, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ARMCPU *cpu = opaque;
>      CPUARMState *env = &cpu->env;
> @@ -608,7 +608,7 @@ static int get_power(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_power(QEMUFile *f, void *opaque, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ARMCPU *cpu = opaque;
>  
> diff --git a/target/avr/machine.c b/target/avr/machine.c
> index e315442787..de264f57c3 100644
> --- a/target/avr/machine.c
> +++ b/target/avr/machine.c
> @@ -34,7 +34,7 @@ static int get_sreg(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_sreg(QEMUFile *f, void *opaque, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPUAVRState *env = opaque;
>      uint8_t sreg = cpu_get_sreg(env);
> @@ -61,7 +61,7 @@ static int get_segment(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_segment(QEMUFile *f, void *opaque, size_t size,
> -                       const VMStateField *field, QJSON *vmdesc)
> +                       const VMStateField *field, JSONWriter *vmdesc)
>  {
>      uint32_t *ramp = opaque;
>      uint8_t temp = *ramp >> 16;
> diff --git a/target/hppa/machine.c b/target/hppa/machine.c
> index b60b654efb..905991d7f9 100644
> --- a/target/hppa/machine.c
> +++ b/target/hppa/machine.c
> @@ -52,7 +52,7 @@ static int get_psw(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_psw(QEMUFile *f, void *opaque, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPUHPPAState *env = opaque;
>      qemu_put_betr(f, cpu_hppa_get_psw(env));
> @@ -93,7 +93,7 @@ static int get_tlb(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_tlb(QEMUFile *f, void *opaque, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      hppa_tlb_entry *ent = opaque;
>      uint32_t val = 0;
> diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
> index c2074bbdfe..d24def3992 100644
> --- a/target/microblaze/machine.c
> +++ b/target/microblaze/machine.c
> @@ -46,7 +46,7 @@ static int get_msr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_msr(QEMUFile *f, void *opaque, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPUMBState *env = container_of(opaque, CPUMBState, msr);
>  
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index 5b23e3e912..77afe654e9 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -31,7 +31,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_fpr(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      int i;
>      fpr_t *v = pv;
> @@ -156,7 +156,7 @@ static int get_tlb(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_tlb(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      r4k_tlb_t *v = pv;
>  
> diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
> index b92985d99b..6239725c4f 100644
> --- a/target/openrisc/machine.c
> +++ b/target/openrisc/machine.c
> @@ -55,7 +55,7 @@ static int get_sr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_sr(QEMUFile *f, void *opaque, size_t size,
> -                  const VMStateField *field, QJSON *vmdesc)
> +                  const VMStateField *field, JSONWriter *vmdesc)
>  {
>      CPUOpenRISCState *env = opaque;
>      qemu_put_be32(f, cpu_get_sr(env));
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index c38e7b1268..2875307452 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -134,7 +134,7 @@ static int get_avr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_avr(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ppc_avr_t *v = pv;
>  
> @@ -166,7 +166,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_fpr(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ppc_vsr_t *v = pv;
>  
> @@ -197,7 +197,7 @@ static int get_vsr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_vsr(QEMUFile *f, void *pv, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ppc_vsr_t *v = pv;
>  
> @@ -455,7 +455,7 @@ static int get_vscr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_vscr(QEMUFile *f, void *opaque, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      PowerPCCPU *cpu = opaque;
>      qemu_put_be32(f, helper_mfvscr(&cpu->env));
> @@ -580,7 +580,7 @@ static int get_slbe(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  static int put_slbe(QEMUFile *f, void *pv, size_t size,
> -                    const VMStateField *field, QJSON *vmdesc)
> +                    const VMStateField *field, JSONWriter *vmdesc)
>  {
>      ppc_slb_t *v = pv;
>  
> diff --git a/target/sparc/machine.c b/target/sparc/machine.c
> index f38cf229af..917375c3a1 100644
> --- a/target/sparc/machine.c
> +++ b/target/sparc/machine.c
> @@ -68,7 +68,7 @@ static int get_psr(QEMUFile *f, void *opaque, size_t size,
>  }
>  
>  static int put_psr(QEMUFile *f, void *opaque, size_t size,
> -                   const VMStateField *field, QJSON *vmdesc)
> +                   const VMStateField *field, JSONWriter *vmdesc)
>  {
>      SPARCCPU *cpu = opaque;
>      CPUSPARCState *env = &cpu->env;
> diff --git a/migration/meson.build b/migration/meson.build
> index 980e37865c..291adc1337 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -6,7 +6,6 @@ migration_files = files(
>    'vmstate.c',
>    'qemu-file-channel.c',
>    'qemu-file.c',
> -  'qjson.c',
>  )
>  softmmu_ss.add(migration_files)
>  
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH 16/20] migration: Replace migration's JSON writer by the general one
  2020-12-16 19:46   ` Dr. David Alan Gilbert
@ 2020-12-17  7:10     ` Markus Armbruster
  2020-12-17  9:38       ` Dr. David Alan Gilbert
  2020-12-17 10:32       ` Dr. David Alan Gilbert
  0 siblings, 2 replies; 36+ messages in thread
From: Markus Armbruster @ 2020-12-17  7:10 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: qemu-devel, mdroth, agraf, Markus Armbruster, Juan Quintela

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> Commit 8118f0950f "migration: Append JSON description of migration
>> stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
>> good fit, because it requires building a QObject to convert.  Instead,
>> migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
>> Add JSON writer".  It tacitly limits numbers to int64_t, and strings
>> contents to characters that don't need escaping, unlike
>> qobject_to_json().
>> 
>> The previous commit factored the JSON writer out of qobject_to_json().
>> Replace migration's JSON writer by it.
>> 
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> (Copying in Alex)
>
> This looks OK to me, so:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> but, can I just check, have you checked scripts/analyze-migration.py is
> still happy with the output?

Good point.  I just did, following instructions in
docs/devel/migration.rst.  It prints stuff and succeeds.  Anything else
you'd like me to try?



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

* Re: [PATCH 16/20] migration: Replace migration's JSON writer by the general one
  2020-12-17  7:10     ` Markus Armbruster
@ 2020-12-17  9:38       ` Dr. David Alan Gilbert
  2020-12-17 10:32       ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 36+ messages in thread
From: Dr. David Alan Gilbert @ 2020-12-17  9:38 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: mdroth, agraf, qemu-devel, Juan Quintela

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Commit 8118f0950f "migration: Append JSON description of migration
> >> stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
> >> good fit, because it requires building a QObject to convert.  Instead,
> >> migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
> >> Add JSON writer".  It tacitly limits numbers to int64_t, and strings
> >> contents to characters that don't need escaping, unlike
> >> qobject_to_json().
> >> 
> >> The previous commit factored the JSON writer out of qobject_to_json().
> >> Replace migration's JSON writer by it.
> >> 
> >> Cc: Juan Quintela <quintela@redhat.com>
> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >
> > (Copying in Alex)
> >
> > This looks OK to me, so:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > but, can I just check, have you checked scripts/analyze-migration.py is
> > still happy with the output?
> 
> Good point.  I just did, following instructions in
> docs/devel/migration.rst.  It prints stuff and succeeds.  Anything else
> you'd like me to try?

If it's happy, I'm happy.

Dave

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH 16/20] migration: Replace migration's JSON writer by the general one
  2020-12-17  7:10     ` Markus Armbruster
  2020-12-17  9:38       ` Dr. David Alan Gilbert
@ 2020-12-17 10:32       ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 36+ messages in thread
From: Dr. David Alan Gilbert @ 2020-12-17 10:32 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: mdroth, agraf, qemu-devel, Juan Quintela

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Commit 8118f0950f "migration: Append JSON description of migration
> >> stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
> >> good fit, because it requires building a QObject to convert.  Instead,
> >> migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
> >> Add JSON writer".  It tacitly limits numbers to int64_t, and strings
> >> contents to characters that don't need escaping, unlike
> >> qobject_to_json().
> >> 
> >> The previous commit factored the JSON writer out of qobject_to_json().
> >> Replace migration's JSON writer by it.
> >> 
> >> Cc: Juan Quintela <quintela@redhat.com>
> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >
> > (Copying in Alex)
> >
> > This looks OK to me, so:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > but, can I just check, have you checked scripts/analyze-migration.py is
> > still happy with the output?
> 
> Good point.  I just did, following instructions in
> docs/devel/migration.rst.  It prints stuff and succeeds.  Anything else
> you'd like me to try?

If it's happy, I'm happy.

Dave

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH 18/20] keyval: Use GString to accumulate value strings
  2020-12-11 17:11 ` [PATCH 18/20] keyval: Use GString to accumulate value strings Markus Armbruster
@ 2020-12-22  9:56   ` Paolo Bonzini
  2021-01-11 13:05     ` Markus Armbruster
  0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2020-12-22  9:56 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth

On 11/12/20 18:11, Markus Armbruster wrote:
> QString supports modifying its string, but it's quite limited: you can
> only append.  The remaining callers use it for building an initial
> string, never for modifying it later.
> 
> Change keyval_parse_one() to do build the initial string with GString.
> This is another step towards making QString immutable.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

It's a bit unfortunate that the infamous "keyval: accept escaped commas 
in implied option" patch was already getting rid of mutable QString.

It used a completely different mechanism, namely unescaping the string 
in place.  This means that my patch was doing n+1 allocations, versus a 
best case of n and a generic case of O(n) for this patch.  The 
difference does not really matter, though I still like my code better.

Paolo

> ---
>   util/keyval.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/util/keyval.c b/util/keyval.c
> index 7f625ad33c..be34928813 100644
> --- a/util/keyval.c
> +++ b/util/keyval.c
> @@ -189,7 +189,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
>       QDict *cur;
>       int ret;
>       QObject *next;
> -    QString *val;
> +    GString *val;
>   
>       key = params;
>       val_end = NULL;
> @@ -263,7 +263,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
>   
>       if (key == implied_key) {
>           assert(!*s);
> -        val = qstring_from_substr(params, 0, val_end - params);
> +        val = g_string_new_len(params, val_end - params);
>           s = val_end;
>           if (*s == ',') {
>               s++;
> @@ -276,7 +276,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
>           }
>           s++;
>   
> -        val = qstring_new();
> +        val = g_string_new(NULL);
>           for (;;) {
>               if (!*s) {
>                   break;
> @@ -286,11 +286,12 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
>                       break;
>                   }
>               }
> -            qstring_append_chr(val, *s++);
> +            g_string_append_c(val, *s++);
>           }
>       }
>   
> -    if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {
> +    if (!keyval_parse_put(cur, key_in_cur, qstring_from_gstring(val),
> +                          key, key_end, errp)) {
>           return NULL;
>       }
>       return s;
> 



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

* Re: [PATCH 00/20] Immutable QString, and also one JSON writer less
  2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
                   ` (19 preceding siblings ...)
  2020-12-11 17:11 ` [PATCH 20/20] qobject: Make QString immutable Markus Armbruster
@ 2020-12-22  9:59 ` Paolo Bonzini
  20 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2020-12-22  9:59 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel

On 11/12/20 18:11, Markus Armbruster wrote:
> Based-on: <20201210161452.2813491-1-armbru@redhat.com>
> 
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Yuval Shaia <yuval.shaia.ml@gmail.com>
> Cc: qemu-block@nongnu.org

Looks good, my only comment was on conflicts with other pending series.

Paolo

> Markus Armbruster (20):
>    hmp: Simplify how qmp_human_monitor_command() gets output
>    monitor: Use GString instead of QString for output buffer
>    qobject: Make qobject_to_json_pretty() take a pretty argument
>    qobject: Use GString instead of QString to accumulate JSON
>    qobject: Change qobject_to_json()'s value to GString
>    Revert "qstring: add qstring_free()"
>    hw/rdma: Replace QList by GQueue
>    qobject: Move internals to qobject-internal.h
>    qmp: Fix tracing of non-string command IDs
>    block: Avoid qobject_get_try_str()
>    Revert "qobject: let object_property_get_str() use new API"
>    qobject: Drop qobject_get_try_str()
>    qobject: Drop qstring_get_try_str()
>    qobject: Factor quoted_str() out of to_json()
>    qobject: Factor JSON writer out of qobject_to_json()
>    migration: Replace migration's JSON writer by the general one
>    json: Use GString instead of QString to accumulate strings
>    keyval: Use GString to accumulate value strings
>    block: Use GString instead of QString to build filenames
>    qobject: Make QString immutable
> 
>   hw/rdma/rdma_backend_defs.h        |   2 +-
>   hw/rdma/rdma_utils.h               |  15 +-
>   include/migration/vmstate.h        |   7 +-
>   include/qapi/qmp/json-writer.h     |  35 ++++
>   include/qapi/qmp/qbool.h           |   2 -
>   include/qapi/qmp/qdict.h           |   2 -
>   include/qapi/qmp/qjson.h           |   4 +-
>   include/qapi/qmp/qlist.h           |   2 -
>   include/qapi/qmp/qnull.h           |   2 -
>   include/qapi/qmp/qnum.h            |   3 -
>   include/qapi/qmp/qobject.h         |   9 +-
>   include/qapi/qmp/qstring.h         |  14 +-
>   include/qemu/typedefs.h            |   4 +-
>   migration/qjson.h                  |  29 ----
>   monitor/monitor-internal.h         |   2 +-
>   qobject/qobject-internal.h         |  39 +++++
>   block.c                            |  23 +--
>   block/rbd.c                        |   2 +-
>   hw/display/virtio-gpu.c            |   2 +-
>   hw/intc/s390_flic_kvm.c            |   2 +-
>   hw/nvram/eeprom93xx.c              |   2 +-
>   hw/nvram/fw_cfg.c                  |   2 +-
>   hw/pci/msix.c                      |   2 +-
>   hw/pci/pci.c                       |   4 +-
>   hw/pci/shpc.c                      |   2 +-
>   hw/rdma/rdma_backend.c             |  10 +-
>   hw/rdma/rdma_utils.c               |  29 ++--
>   hw/rtc/twl92230.c                  |   2 +-
>   hw/scsi/scsi-bus.c                 |   2 +-
>   hw/usb/redirect.c                  |   7 +-
>   hw/virtio/virtio.c                 |   4 +-
>   migration/qjson.c                  | 114 -------------
>   migration/savevm.c                 |  53 ++++---
>   migration/vmstate-types.c          |  38 ++---
>   migration/vmstate.c                |  52 +++---
>   monitor/misc.c                     |   6 +-
>   monitor/monitor.c                  |  20 +--
>   monitor/qmp.c                      |  46 +++---
>   qemu-img.c                         |  33 ++--
>   qga/main.c                         |  22 +--
>   qobject/json-parser.c              |  30 ++--
>   qobject/json-writer.c              | 247 +++++++++++++++++++++++++++++
>   qobject/qbool.c                    |   1 +
>   qobject/qdict.c                    |   1 +
>   qobject/qjson.c                    | 144 ++++-------------
>   qobject/qlist.c                    |   1 +
>   qobject/qnull.c                    |   1 +
>   qobject/qnum.c                     |   6 +-
>   qobject/qobject.c                  |   1 +
>   qobject/qstring.c                  | 117 +++-----------
>   qom/object.c                       |   9 +-
>   qom/object_interfaces.c            |   4 +-
>   qom/qom-hmp-cmds.c                 |   7 +-
>   target/alpha/machine.c             |   2 +-
>   target/arm/machine.c               |   6 +-
>   target/avr/machine.c               |   4 +-
>   target/hppa/machine.c              |   4 +-
>   target/microblaze/machine.c        |   2 +-
>   target/mips/machine.c              |   4 +-
>   target/openrisc/machine.c          |   2 +-
>   target/ppc/machine.c               |  10 +-
>   target/sparc/machine.c             |   2 +-
>   tests/check-qjson.c                |  67 ++++----
>   tests/check-qobject.c              |   3 +-
>   tests/check-qstring.c              |  16 --
>   tests/qtest/libqtest.c             |  20 ++-
>   tests/test-visitor-serialization.c |   6 +-
>   util/keyval.c                      |  11 +-
>   migration/meson.build              |   1 -
>   qobject/meson.build                |   5 +-
>   70 files changed, 679 insertions(+), 705 deletions(-)
>   create mode 100644 include/qapi/qmp/json-writer.h
>   delete mode 100644 migration/qjson.h
>   create mode 100644 qobject/qobject-internal.h
>   delete mode 100644 migration/qjson.c
>   create mode 100644 qobject/json-writer.c
> 



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

* Re: [PATCH 18/20] keyval: Use GString to accumulate value strings
  2020-12-22  9:56   ` Paolo Bonzini
@ 2021-01-11 13:05     ` Markus Armbruster
  2021-01-11 13:51       ` Paolo Bonzini
  0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2021-01-11 13:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, mdroth

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/12/20 18:11, Markus Armbruster wrote:
>> QString supports modifying its string, but it's quite limited: you can
>> only append.  The remaining callers use it for building an initial
>> string, never for modifying it later.
>> Change keyval_parse_one() to do build the initial string with
>> GString.
>> This is another step towards making QString immutable.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> It's a bit unfortunate that the infamous "keyval: accept escaped
> commas in implied option" patch was already getting rid of mutable
> QString.
>
> It used a completely different mechanism, namely unescaping the string
> in place.  This means that my patch was doing n+1 allocations, versus
> a best case of n and a generic case of O(n) for this patch.  The 
> difference does not really matter, though I still like my code better.

My patch is not intended as a replacement of yours.  Mine does much
less.

I had to choose between creating a conflict and holding back my series
while we figure out what to do with your patch.  The dilemma is my own
doing; your patch is waiting just for me.  I picked the conflict.

I can look into rebasing your patch on top of mine.



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

* Re: [PATCH 18/20] keyval: Use GString to accumulate value strings
  2021-01-11 13:05     ` Markus Armbruster
@ 2021-01-11 13:51       ` Paolo Bonzini
  0 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2021-01-11 13:51 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, mdroth

On 11/01/21 14:05, Markus Armbruster wrote:
> I had to choose between creating a conflict and holding back my series
> while we figure out what to do with your patch.  The dilemma is my own
> doing; your patch is waiting just for me.  I picked the conflict.
> 
> I can look into rebasing your patch on top of mine.

No need to.  My patch also removes the use of GString, so the code after 
my patch can be exactly the same.  Or in other words, squashing a revert 
in front of my patch just works.

Paolo



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

* Re: [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString
  2020-12-11 17:11 ` [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString Markus Armbruster
@ 2021-03-24  7:16   ` Thomas Huth
  2021-03-24  8:14     ` Markus Armbruster
  0 siblings, 1 reply; 36+ messages in thread
From: Thomas Huth @ 2021-03-24  7:16 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel
  Cc: Paolo Bonzini, mdroth, Marc-André Lureau, Peter Maydell

On 11/12/2020 18.11, Markus Armbruster wrote:
> qobject_to_json() and qobject_to_json_pretty() build a GString, then
> covert it to QString.  Just one of the callers actually needs a
> QString: qemu_rbd_parse_filename().  A few others need a string they
> can modify: qmp_send_response(), qga's send_response(), to_json_str(),
> and qmp_fd_vsend_fds().  The remainder just need a string.
> 
> Change qobject_to_json() and qobject_to_json_pretty() to return the
> GString.
> 
> qemu_rbd_parse_filename() now has to convert to QString.  All others
> save a QString temporary.  to_json_str() actually becomes a bit
> simpler, because GString provides more convenient modification
> functions.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

  Hi Markus!

This patch broke the output of default values in the device help. With 
commit eab3a4678b07267c39e72:

$ ./qemu-system-x86_64 -device pvpanic,help
pvpanic options:
   events=<uint8>         -  (default: (null))
   ioport=<uint16>        -  (default: (null))
   pvpanic[0]=<child<qemu:memory-region>>

With the commit right before that one:

$ ./qemu-system-x86_64 -device pvpanic,help
pvpanic options:
   events=<uint8>         -  (default: 3)
   ioport=<uint16>        -  (default: 1285)
   pvpanic[0]=<child<qemu:memory-region>>

Any ideas how to fix that?

  Thomas



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

* Re: [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString
  2021-03-24  7:16   ` Thomas Huth
@ 2021-03-24  8:14     ` Markus Armbruster
  0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2021-03-24  8:14 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Peter Maydell, Paolo Bonzini, qemu-devel, Marc-André Lureau, mdroth

Thomas Huth <thuth@redhat.com> writes:

> On 11/12/2020 18.11, Markus Armbruster wrote:
>> qobject_to_json() and qobject_to_json_pretty() build a GString, then
>> covert it to QString.  Just one of the callers actually needs a
>> QString: qemu_rbd_parse_filename().  A few others need a string they
>> can modify: qmp_send_response(), qga's send_response(), to_json_str(),
>> and qmp_fd_vsend_fds().  The remainder just need a string.
>> Change qobject_to_json() and qobject_to_json_pretty() to return the
>> GString.
>> qemu_rbd_parse_filename() now has to convert to QString.  All others
>> save a QString temporary.  to_json_str() actually becomes a bit
>> simpler, because GString provides more convenient modification
>> functions.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
>  Hi Markus!
>
> This patch broke the output of default values in the device help. With
> commit eab3a4678b07267c39e72:
>
> $ ./qemu-system-x86_64 -device pvpanic,help
> pvpanic options:
>   events=<uint8>         -  (default: (null))
>   ioport=<uint16>        -  (default: (null))
>   pvpanic[0]=<child<qemu:memory-region>>
>
> With the commit right before that one:
>
> $ ./qemu-system-x86_64 -device pvpanic,help
> pvpanic options:
>   events=<uint8>         -  (default: 3)
>   ioport=<uint16>        -  (default: 1285)
>   pvpanic[0]=<child<qemu:memory-region>>
>
> Any ideas how to fix that?

Yes:

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index c3324b0f86..bd8a947a63 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -159,7 +159,7 @@ char *object_property_help(const char *name, const char *type,
     }
     if (defval) {
         g_autofree char *def_json = g_string_free(qobject_to_json(defval),
-                                                  true);
+                                                  false);
         g_string_append_printf(str, " (default: %s)", def_json);
     }
 

I'll post a proper patch.  Sorry for the dumb mistake, and thanks for
pinpointing the commit!



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

end of thread, other threads:[~2021-03-24  8:15 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 17:11 [PATCH 00/20] Immutable QString, and also one JSON writer less Markus Armbruster
2020-12-11 17:11 ` [PATCH 01/20] hmp: Simplify how qmp_human_monitor_command() gets output Markus Armbruster
2020-12-16 18:53   ` Dr. David Alan Gilbert
2020-12-11 17:11 ` [PATCH 02/20] monitor: Use GString instead of QString for output buffer Markus Armbruster
2020-12-16 19:36   ` Dr. David Alan Gilbert
2020-12-11 17:11 ` [PATCH 03/20] qobject: Make qobject_to_json_pretty() take a pretty argument Markus Armbruster
2020-12-11 17:11 ` [PATCH 04/20] qobject: Use GString instead of QString to accumulate JSON Markus Armbruster
2020-12-11 17:11 ` [PATCH 05/20] qobject: Change qobject_to_json()'s value to GString Markus Armbruster
2021-03-24  7:16   ` Thomas Huth
2021-03-24  8:14     ` Markus Armbruster
2020-12-11 17:11 ` [PATCH 06/20] Revert "qstring: add qstring_free()" Markus Armbruster
2020-12-11 17:11 ` [PATCH 07/20] hw/rdma: Replace QList by GQueue Markus Armbruster
2020-12-11 17:11 ` [PATCH 08/20] qobject: Move internals to qobject-internal.h Markus Armbruster
2020-12-11 17:11 ` [PATCH 09/20] qmp: Fix tracing of non-string command IDs Markus Armbruster
2020-12-11 17:11 ` [PATCH 10/20] block: Avoid qobject_get_try_str() Markus Armbruster
2020-12-11 18:28   ` Vladimir Sementsov-Ogievskiy
2020-12-11 17:11 ` [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API" Markus Armbruster
2020-12-11 19:48   ` Eduardo Habkost
2020-12-11 17:11 ` [PATCH 12/20] qobject: Drop qobject_get_try_str() Markus Armbruster
2020-12-11 17:11 ` [PATCH 13/20] qobject: Drop qstring_get_try_str() Markus Armbruster
2020-12-11 17:11 ` [PATCH 14/20] qobject: Factor quoted_str() out of to_json() Markus Armbruster
2020-12-11 17:11 ` [PATCH 15/20] qobject: Factor JSON writer out of qobject_to_json() Markus Armbruster
2020-12-11 17:11 ` [PATCH 16/20] migration: Replace migration's JSON writer by the general one Markus Armbruster
2020-12-16 19:46   ` Dr. David Alan Gilbert
2020-12-17  7:10     ` Markus Armbruster
2020-12-17  9:38       ` Dr. David Alan Gilbert
2020-12-17 10:32       ` Dr. David Alan Gilbert
2020-12-11 17:11 ` [PATCH 17/20] json: Use GString instead of QString to accumulate strings Markus Armbruster
2020-12-11 17:11 ` [PATCH 18/20] keyval: Use GString to accumulate value strings Markus Armbruster
2020-12-22  9:56   ` Paolo Bonzini
2021-01-11 13:05     ` Markus Armbruster
2021-01-11 13:51       ` Paolo Bonzini
2020-12-11 17:11 ` [PATCH 19/20] block: Use GString instead of QString to build filenames Markus Armbruster
2020-12-11 18:47   ` Vladimir Sementsov-Ogievskiy
2020-12-11 17:11 ` [PATCH 20/20] qobject: Make QString immutable Markus Armbruster
2020-12-22  9:59 ` [PATCH 00/20] Immutable QString, and also one JSON writer less Paolo Bonzini

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.