All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/7] Common macros for QAPI list growth
@ 2020-12-23 22:10 Eric Blake
  2020-12-23 22:10 ` [PATCH v3 1/7] net: Clarify early exit condition Eric Blake
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

I worked on rebasing this while not checking my emails, and now that
I'm writing it up, I see that Markus already has incorporated earlier
patches in the v2 series into his tree.  So I may have to rebase yet
again, but it's at least time for me to get this on list again.

v2 was here:
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg03457.html

Since then: address review comments, use the name 'tail' in more
places, rebase to master

Eric Blake (7):
  net: Clarify early exit condition
  rocker: Revamp fp_port_get_info
  migration: Refactor migrate_cap_add
  qapi: Use QAPI_LIST_PREPEND() where possible
  qapi: Introduce QAPI_LIST_APPEND
  qapi: Use QAPI_LIST_APPEND in trivial cases
  qapi: More complex uses of QAPI_LIST_APPEND

 docs/devel/writing-qmp-commands.txt |  12 +-
 hw/net/rocker/rocker_fp.h           |   2 +-
 include/qapi/util.h                 |  13 +++
 backends/hostmem.c                  |  10 +-
 block/dirty-bitmap.c                |   8 +-
 block/export/export.c               |   7 +-
 block/gluster.c                     |  17 +--
 block/qapi.c                        |  44 ++-----
 block/qcow2-bitmap.c                |  15 +--
 block/vmdk.c                        |   9 +-
 blockdev.c                          |  13 +--
 chardev/char.c                      |  20 ++--
 crypto/block-luks.c                 |   9 +-
 dump/dump.c                         |  22 +---
 hw/acpi/cpu.c                       |   8 +-
 hw/acpi/memory_hotplug.c            |   9 +-
 hw/core/machine-qmp-cmds.c          | 131 +++++++++------------
 hw/core/machine.c                   |  11 +-
 hw/mem/memory-device.c              |  12 +-
 hw/net/rocker/rocker.c              |   8 +-
 hw/net/rocker/rocker_fp.c           |  17 +--
 hw/net/rocker/rocker_of_dpa.c       |  20 +---
 hw/net/virtio-net.c                 |  21 ++--
 hw/pci/pci.c                        |  60 +++-------
 iothread.c                          |  12 +-
 job-qmp.c                           |  13 +--
 migration/migration.c               |  56 ++++-----
 migration/postcopy-ram.c            |   7 +-
 monitor/hmp-cmds.c                  |  48 ++++----
 monitor/misc.c                      |  25 ++--
 monitor/qmp-cmds-control.c          |  19 ++-
 net/net.c                           |  15 +--
 qemu-img.c                          |  13 +--
 qga/commands-posix-ssh.c            |   7 +-
 qga/commands-posix.c                | 172 +++++++++-------------------
 qga/commands-win32.c                | 131 +++++++--------------
 qga/commands.c                      |   6 +-
 qom/qom-qmp-cmds.c                  |  29 ++---
 scsi/pr-manager.c                   |  10 +-
 softmmu/tpm.c                       |  38 +-----
 target/arm/helper.c                 |   6 +-
 target/arm/monitor.c                |  13 +--
 target/i386/cpu.c                   |  29 ++---
 target/mips/cpu.c                   |   6 +-
 target/s390x/cpu_models.c           |  12 +-
 tests/test-clone-visitor.c          |   7 +-
 tests/test-qobject-output-visitor.c | 126 +++++++-------------
 tests/test-string-output-visitor.c  |   6 +-
 tests/test-visitor-serialization.c  | 113 +++---------------
 trace/qmp.c                         |  22 ++--
 ui/input.c                          |  16 ++-
 ui/spice-core.c                     |  27 ++---
 ui/vnc.c                            |  21 +---
 util/qemu-config.c                  |  14 +--
 target/ppc/translate_init.c.inc     |  12 +-
 55 files changed, 478 insertions(+), 1051 deletions(-)

-- 
2.29.2



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

* [PATCH v3 1/7] net: Clarify early exit condition
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
@ 2020-12-23 22:10 ` Eric Blake
  2021-01-13 12:57   ` Markus Armbruster
  2020-12-23 22:10 ` [PATCH v3 2/7] rocker: Revamp fp_port_get_info Eric Blake
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jason Wang, armbru

On first glance, the loop in qmp_query_rx_filter() has early return
paths that could leak any allocation of filter_list from a previous
iteration.  But on closer inspection, it is obvious that all of the
early exits are guarded by has_name, and that the bulk of the loop
body can be executed at most once if the user is filtering by name,
thus, any early exit coincides with an empty list.  Add asserts to
make this obvious.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 net/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/net.c b/net/net.c
index e1035f21d183..e581c8a26868 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1211,6 +1211,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
         if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
             if (has_name) {
                 error_setg(errp, "net client(%s) isn't a NIC", name);
+                assert(!filter_list);
                 return NULL;
             }
             continue;
@@ -1236,6 +1237,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
         } else if (has_name) {
             error_setg(errp, "net client(%s) doesn't support"
                        " rx-filter querying", name);
+            assert(!filter_list);
             return NULL;
         }

-- 
2.29.2



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

* [PATCH v3 2/7] rocker: Revamp fp_port_get_info
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
  2020-12-23 22:10 ` [PATCH v3 1/7] net: Clarify early exit condition Eric Blake
@ 2020-12-23 22:10 ` Eric Blake
  2021-01-13 12:57   ` Markus Armbruster
  2020-12-23 22:10 ` [PATCH v3 3/7] migration: Refactor migrate_cap_add Eric Blake
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jason Wang, Jiri Pirko, armbru

Instead of modifying the value member of a list element passed as a
parameter, and open-coding the manipulation of that list, it's nicer
to just return a freshly allocated value to be prepended to a list
using QAPI_LIST_PREPEND.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/rocker/rocker_fp.h |  2 +-
 hw/net/rocker/rocker.c    |  8 +-------
 hw/net/rocker/rocker_fp.c | 17 ++++++++++-------
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h
index dbe1dd329a4b..7ff57aac0180 100644
--- a/hw/net/rocker/rocker_fp.h
+++ b/hw/net/rocker/rocker_fp.h
@@ -28,7 +28,7 @@ int fp_port_eg(FpPort *port, const struct iovec *iov, int iovcnt);

 char *fp_port_get_name(FpPort *port);
 bool fp_port_get_link_up(FpPort *port);
-void fp_port_get_info(FpPort *port, RockerPortList *info);
+RockerPort *fp_port_get_info(FpPort *port);
 void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr);
 void fp_port_set_macaddr(FpPort *port, MACAddr *macaddr);
 uint8_t fp_port_get_learning(FpPort *port);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 1af1e6fa2f9b..c53a02315e54 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -127,13 +127,7 @@ RockerPortList *qmp_query_rocker_ports(const char *name, Error **errp)
     }

     for (i = r->fp_ports - 1; i >= 0; i--) {
-        RockerPortList *info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        struct fp_port *port = r->fp_port[i];
-
-        fp_port_get_info(port, info);
-        info->next = list;
-        list = info;
+        QAPI_LIST_PREPEND(list, fp_port_get_info(r->fp_port[i]));
     }

     return list;
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index 4aa7da79b81d..cbeed65bd5ec 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -51,14 +51,17 @@ bool fp_port_get_link_up(FpPort *port)
     return !qemu_get_queue(port->nic)->link_down;
 }

-void fp_port_get_info(FpPort *port, RockerPortList *info)
+RockerPort *fp_port_get_info(FpPort *port)
 {
-    info->value->name = g_strdup(port->name);
-    info->value->enabled = port->enabled;
-    info->value->link_up = fp_port_get_link_up(port);
-    info->value->speed = port->speed;
-    info->value->duplex = port->duplex;
-    info->value->autoneg = port->autoneg;
+    RockerPort *value = g_malloc0(sizeof(*value));
+
+    value->name = g_strdup(port->name);
+    value->enabled = port->enabled;
+    value->link_up = fp_port_get_link_up(port);
+    value->speed = port->speed;
+    value->duplex = port->duplex;
+    value->autoneg = port->autoneg;
+    return value;
 }

 void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr)
-- 
2.29.2



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

* [PATCH v3 3/7] migration: Refactor migrate_cap_add
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
  2020-12-23 22:10 ` [PATCH v3 1/7] net: Clarify early exit condition Eric Blake
  2020-12-23 22:10 ` [PATCH v3 2/7] rocker: Revamp fp_port_get_info Eric Blake
@ 2020-12-23 22:10 ` Eric Blake
  2021-01-13 12:58   ` Markus Armbruster
  2020-12-23 22:10 ` [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru, Dr. David Alan Gilbert, Juan Quintela

Instead of taking a list parameter and returning a new head at a
distance, just return the new item for the caller to insert into a
list via QAPI_LIST_PREPEND.  Update some variable names to avoid long
lines, and drop a useless comment.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 migration/migration.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index e0dbde4091c9..bba6e5148138 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1654,29 +1654,27 @@ void migrate_set_state(int *state, int old_state, int new_state)
     }
 }

-static MigrationCapabilityStatusList *migrate_cap_add(
-    MigrationCapabilityStatusList *list,
-    MigrationCapability index,
-    bool state)
+static MigrationCapabilityStatus *migrate_cap_add(MigrationCapability index,
+                                                  bool state)
 {
-    MigrationCapabilityStatusList *cap;
+    MigrationCapabilityStatus *cap;

-    cap = g_new0(MigrationCapabilityStatusList, 1);
-    cap->value = g_new0(MigrationCapabilityStatus, 1);
-    cap->value->capability = index;
-    cap->value->state = state;
-    cap->next = list;
+    cap = g_new0(MigrationCapabilityStatus, 1);
+    cap->capability = index;
+    cap->state = state;

     return cap;
 }

 void migrate_set_block_enabled(bool value, Error **errp)
 {
-    MigrationCapabilityStatusList *cap;
+    MigrationCapabilityStatusList *caps = NULL;
+    MigrationCapabilityStatus *cap;

-    cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
-    qmp_migrate_set_capabilities(cap, errp);
-    qapi_free_MigrationCapabilityStatusList(cap);
+    cap = migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value);
+    QAPI_LIST_PREPEND(caps, cap);
+    qmp_migrate_set_capabilities(caps, errp);
+    qapi_free_MigrationCapabilityStatusList(caps);
 }

 static void migrate_set_block_incremental(MigrationState *s, bool value)
@@ -3863,13 +3861,12 @@ static bool migration_object_check(MigrationState *ms, Error **errp)

     for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
         if (ms->enabled_capabilities[i]) {
-            head = migrate_cap_add(head, i, true);
+            QAPI_LIST_PREPEND(head, migrate_cap_add(i, true));
         }
     }

     ret = migrate_caps_check(cap_list, head, errp);

-    /* It works with head == NULL */
     qapi_free_MigrationCapabilityStatusList(head);

     return ret;
-- 
2.29.2



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

* [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
                   ` (2 preceding siblings ...)
  2020-12-23 22:10 ` [PATCH v3 3/7] migration: Refactor migrate_cap_add Eric Blake
@ 2020-12-23 22:10 ` Eric Blake
  2021-01-13 13:00   ` Markus Armbruster
  2020-12-23 22:11 ` [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Thomas Huth,
	Gerd Hoffmann, open list:GLUSTER, Juan Quintela,
	David Hildenbrand, armbru, Halil Pasic, Christian Borntraeger,
	Marc-André Lureau, Aleksandar Rikalo, Jiri Pirko,
	Eduardo Habkost, Michael Roth, Richard Henderson,
	Dr. David Alan Gilbert, Greg Kurz, open list:S390 KVM CPUs,
	open list:ARM TCG CPUs, Stefan Hajnoczi, David Gibson,
	Kevin Wolf, open list:GLUSTER, Daniel P. Berrangé,
	Cornelia Huck, Philippe Mathieu-Daudé,
	Max Reitz, open list:PowerPC TCG CPUs, Paolo Bonzini,
	Aurelien Jarno

Anywhere we create a list of just one item or by prepending items
(typically because order doesn't matter), we can use the
QAPI_LIST_PREPEND macro.  But places where we must keep the list in
order by appending remain open-coded until later patches.

Note that as a side effect, this also performs a cleanup of two minor
issues in qga/commands-posix.c: the old code was performing
 new = g_malloc0(sizeof(*ret));
which 1) is confusing because you have to verify whether 'new' and
'ret' are variables with the same type, and 2) would conflict with C++
compilation (not an actual problem for this file, but makes
copy-and-paste harder).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/writing-qmp-commands.txt |  12 +--
 block/gluster.c                     |   4 +-
 block/qapi.c                        |   7 +-
 chardev/char.c                      |  20 ++---
 hw/core/machine-qmp-cmds.c          |  38 +++-------
 hw/core/machine.c                   |  11 +--
 hw/net/rocker/rocker_of_dpa.c       |  20 ++---
 hw/net/virtio-net.c                 |  21 ++----
 migration/migration.c               |   7 +-
 migration/postcopy-ram.c            |   7 +-
 monitor/hmp-cmds.c                  |  13 ++--
 monitor/misc.c                      |  25 +++---
 monitor/qmp-cmds-control.c          |  10 +--
 qemu-img.c                          |   5 +-
 qga/commands-posix-ssh.c            |   7 +-
 qga/commands-posix.c                |  47 +++---------
 qga/commands-win32.c                |  32 ++------
 qga/commands.c                      |   6 +-
 qom/qom-qmp-cmds.c                  |  29 ++-----
 target/arm/helper.c                 |   6 +-
 target/arm/monitor.c                |  13 +---
 target/i386/cpu.c                   |   6 +-
 target/mips/cpu.c                   |   6 +-
 target/s390x/cpu_models.c           |  12 +--
 tests/test-clone-visitor.c          |   7 +-
 tests/test-qobject-output-visitor.c |  42 +++++------
 tests/test-visitor-serialization.c  | 113 ++++------------------------
 trace/qmp.c                         |  22 +++---
 ui/input.c                          |  16 ++--
 ui/vnc.c                            |  21 ++----
 util/qemu-config.c                  |  14 +---
 target/ppc/translate_init.c.inc     |  12 +--
 32 files changed, 169 insertions(+), 442 deletions(-)

diff --git a/docs/devel/writing-qmp-commands.txt b/docs/devel/writing-qmp-commands.txt
index 28984686c970..258e63bff5ee 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -531,15 +531,11 @@ TimerAlarmMethodList *qmp_query_alarm_methods(Error **errp)
     bool current = true;

     for (p = alarm_timers; p->name; p++) {
-        TimerAlarmMethodList *info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->method_name = g_strdup(p->name);
-        info->value->current = current;
-
+        TimerAlarmMethod *value = g_malloc0(*value);
+        value->method_name = g_strdup(p->name);
+        value->current = current;
+        QAPI_LIST_PREPEND(method_list, value);
         current = false;
-
-        info->next = method_list;
-        method_list = info;
     }

     return method_list;
diff --git a/block/gluster.c b/block/gluster.c
index 4f1448e2bc88..1f8699b93835 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -359,8 +359,8 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
         return -EINVAL;
     }

-    gconf->server = g_new0(SocketAddressList, 1);
-    gconf->server->value = gsconf = g_new0(SocketAddress, 1);
+    gsconf = g_new0(SocketAddress, 1);
+    QAPI_LIST_PREPEND(gconf->server, gsconf);

     /* transport */
     if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
diff --git a/block/qapi.c b/block/qapi.c
index 036da085eea6..0ca206f559fe 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -486,12 +486,7 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
     ds->account_failed = stats->account_failed;

     while ((ts = block_acct_interval_next(stats, ts))) {
-        BlockDeviceTimedStatsList *timed_stats =
-            g_malloc0(sizeof(*timed_stats));
         BlockDeviceTimedStats *dev_stats = g_malloc0(sizeof(*dev_stats));
-        timed_stats->next = ds->timed_stats;
-        timed_stats->value = dev_stats;
-        ds->timed_stats = timed_stats;

         TimedAverage *rd = &ts->latency[BLOCK_ACCT_READ];
         TimedAverage *wr = &ts->latency[BLOCK_ACCT_WRITE];
@@ -515,6 +510,8 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
             block_acct_queue_depth(ts, BLOCK_ACCT_READ);
         dev_stats->avg_wr_queue_depth =
             block_acct_queue_depth(ts, BLOCK_ACCT_WRITE);
+
+        QAPI_LIST_PREPEND(ds->timed_stats, dev_stats);
     }

     bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_READ],
diff --git a/chardev/char.c b/chardev/char.c
index a9b8c5a9aac6..288efebd1257 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -776,15 +776,13 @@ static int qmp_query_chardev_foreach(Object *obj, void *data)
 {
     Chardev *chr = CHARDEV(obj);
     ChardevInfoList **list = data;
-    ChardevInfoList *info = g_malloc0(sizeof(*info));
+    ChardevInfo *value = g_malloc0(sizeof(*value));

-    info->value = g_malloc0(sizeof(*info->value));
-    info->value->label = g_strdup(chr->label);
-    info->value->filename = g_strdup(chr->filename);
-    info->value->frontend_open = chr->be && chr->be->fe_open;
+    value->label = g_strdup(chr->label);
+    value->filename = g_strdup(chr->filename);
+    value->frontend_open = chr->be && chr->be->fe_open;

-    info->next = *list;
-    *list = info;
+    QAPI_LIST_PREPEND(*list, value);

     return 0;
 }
@@ -803,12 +801,10 @@ static void
 qmp_prepend_backend(const char *name, void *opaque)
 {
     ChardevBackendInfoList **list = opaque;
-    ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
+    ChardevBackendInfo *value = g_new0(ChardevBackendInfo, 1);

-    info->value = g_malloc0(sizeof(*info->value));
-    info->value->name = g_strdup(name);
-    info->next = *list;
-    *list = info;
+    value->name = g_strdup(name);
+    QAPI_LIST_PREPEND(*list, value);
 }

 ChardevBackendInfoList *qmp_query_chardev_backends(Error **errp)
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 87f14140a381..156223a344ed 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -215,7 +215,6 @@ MachineInfoList *qmp_query_machines(Error **errp)

     for (el = machines; el; el = el->next) {
         MachineClass *mc = el->data;
-        MachineInfoList *entry;
         MachineInfo *info;

         info = g_malloc0(sizeof(*info));
@@ -243,10 +242,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
             info->has_default_ram_id = true;
         }

-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = mach_list;
-        mach_list = entry;
+        QAPI_LIST_PREPEND(mach_list, info);
     }

     g_slist_free(machines);
@@ -297,41 +293,31 @@ void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
 static int query_memdev(Object *obj, void *opaque)
 {
     MemdevList **list = opaque;
-    MemdevList *m = NULL;
+    Memdev *m;
     QObject *host_nodes;
     Visitor *v;

     if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
         m = g_malloc0(sizeof(*m));

-        m->value = g_malloc0(sizeof(*m->value));
+        m->id = g_strdup(object_get_canonical_path_component(obj));
+        m->has_id = !!m->id;

-        m->value->id = g_strdup(object_get_canonical_path_component(obj));
-        m->value->has_id = !!m->value->id;
-
-        m->value->size = object_property_get_uint(obj, "size",
-                                                  &error_abort);
-        m->value->merge = object_property_get_bool(obj, "merge",
-                                                   &error_abort);
-        m->value->dump = object_property_get_bool(obj, "dump",
-                                                  &error_abort);
-        m->value->prealloc = object_property_get_bool(obj,
-                                                      "prealloc",
-                                                      &error_abort);
-        m->value->policy = object_property_get_enum(obj,
-                                                    "policy",
-                                                    "HostMemPolicy",
-                                                    &error_abort);
+        m->size = object_property_get_uint(obj, "size", &error_abort);
+        m->merge = object_property_get_bool(obj, "merge", &error_abort);
+        m->dump = object_property_get_bool(obj, "dump", &error_abort);
+        m->prealloc = object_property_get_bool(obj, "prealloc", &error_abort);
+        m->policy = object_property_get_enum(obj, "policy", "HostMemPolicy",
+                                             &error_abort);
         host_nodes = object_property_get_qobject(obj,
                                                  "host-nodes",
                                                  &error_abort);
         v = qobject_input_visitor_new(host_nodes);
-        visit_type_uint16List(v, NULL, &m->value->host_nodes, &error_abort);
+        visit_type_uint16List(v, NULL, &m->host_nodes, &error_abort);
         visit_free(v);
         qobject_unref(host_nodes);

-        m->next = *list;
-        *list = m;
+        QAPI_LIST_PREPEND(*list, m);
     }

     return 0;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 05dcaf09c9e3..de3b8f1b3180 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -504,11 +504,7 @@ static void machine_set_nvdimm_persistence(Object *obj, const char *value,

 void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type)
 {
-    strList *item = g_new0(strList, 1);
-
-    item->value = g_strdup(type);
-    item->next = mc->allowed_dynamic_sysbus_devices;
-    mc->allowed_dynamic_sysbus_devices = item;
+    QAPI_LIST_PREPEND(mc->allowed_dynamic_sysbus_devices, g_strdup(type));
 }

 static void validate_sysbus_device(SysBusDevice *sbdev, void *opaque)
@@ -569,7 +565,6 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)

     for (i = 0; i < machine->possible_cpus->len; i++) {
         Object *cpu;
-        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
         HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);

         cpu_item->type = g_strdup(machine->possible_cpus->cpus[i].type);
@@ -582,9 +577,7 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
             cpu_item->has_qom_path = true;
             cpu_item->qom_path = object_get_canonical_path(cpu);
         }
-        list_item->value = cpu_item;
-        list_item->next = head;
-        head = list_item;
+        QAPI_LIST_PREPEND(head, cpu_item);
     }
     return head;
 }
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index 8e347d1ee4a6..b3b8c5bb6d4b 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2296,7 +2296,6 @@ static void of_dpa_flow_fill(void *cookie, void *value, void *user_data)
     struct of_dpa_flow_key *key = &flow->key;
     struct of_dpa_flow_key *mask = &flow->mask;
     struct of_dpa_flow_fill_context *flow_context = user_data;
-    RockerOfDpaFlowList *new;
     RockerOfDpaFlow *nflow;
     RockerOfDpaFlowKey *nkey;
     RockerOfDpaFlowMask *nmask;
@@ -2307,8 +2306,7 @@ static void of_dpa_flow_fill(void *cookie, void *value, void *user_data)
         return;
     }

-    new = g_malloc0(sizeof(*new));
-    nflow = new->value = g_malloc0(sizeof(*nflow));
+    nflow = g_malloc0(sizeof(*nflow));
     nkey = nflow->key = g_malloc0(sizeof(*nkey));
     nmask = nflow->mask = g_malloc0(sizeof(*nmask));
     naction = nflow->action = g_malloc0(sizeof(*naction));
@@ -2424,8 +2422,7 @@ static void of_dpa_flow_fill(void *cookie, void *value, void *user_data)
         naction->new_vlan_id = flow->action.apply.new_vlan_id;
     }

-    new->next = flow_context->list;
-    flow_context->list = new;
+    QAPI_LIST_PREPEND(flow_context->list, nflow);
 }

 RockerOfDpaFlowList *qmp_query_rocker_of_dpa_flows(const char *name,
@@ -2469,9 +2466,7 @@ static void of_dpa_group_fill(void *key, void *value, void *user_data)
 {
     struct of_dpa_group *group = value;
     struct of_dpa_group_fill_context *flow_context = user_data;
-    RockerOfDpaGroupList *new;
     RockerOfDpaGroup *ngroup;
-    struct uint32List *id;
     int i;

     if (flow_context->type != 9 &&
@@ -2479,8 +2474,7 @@ static void of_dpa_group_fill(void *key, void *value, void *user_data)
         return;
     }

-    new = g_malloc0(sizeof(*new));
-    ngroup = new->value = g_malloc0(sizeof(*ngroup));
+    ngroup = g_malloc0(sizeof(*ngroup));

     ngroup->id = group->id;

@@ -2525,10 +2519,7 @@ static void of_dpa_group_fill(void *key, void *value, void *user_data)
         ngroup->index = ROCKER_GROUP_INDEX_GET(group->id);
         for (i = 0; i < group->l2_flood.group_count; i++) {
             ngroup->has_group_ids = true;
-            id = g_malloc0(sizeof(*id));
-            id->value = group->l2_flood.group_ids[i];
-            id->next = ngroup->group_ids;
-            ngroup->group_ids = id;
+            QAPI_LIST_PREPEND(ngroup->group_ids, group->l2_flood.group_ids[i]);
         }
         break;
     case ROCKER_OF_DPA_GROUP_TYPE_L3_UCAST:
@@ -2557,8 +2548,7 @@ static void of_dpa_group_fill(void *key, void *value, void *user_data)
         break;
     }

-    new->next = flow_context->list;
-    flow_context->list = new;
+    QAPI_LIST_PREPEND(flow_context->list, ngroup);
 }

 RockerOfDpaGroupList *qmp_query_rocker_of_dpa_groups(const char *name,
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 044ac95f6f28..8356eeec1316 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -437,17 +437,14 @@ static void rxfilter_notify(NetClientState *nc)

 static intList *get_vlan_table(VirtIONet *n)
 {
-    intList *list, *entry;
+    intList *list;
     int i, j;

     list = NULL;
     for (i = 0; i < MAX_VLAN >> 5; i++) {
         for (j = 0; n->vlans[i] && j <= 0x1f; j++) {
             if (n->vlans[i] & (1U << j)) {
-                entry = g_malloc0(sizeof(*entry));
-                entry->value = (i << 5) + j;
-                entry->next = list;
-                list = entry;
+                QAPI_LIST_PREPEND(list, (i << 5) + j);
             }
         }
     }
@@ -460,7 +457,7 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
     VirtIONet *n = qemu_get_nic_opaque(nc);
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     RxFilterInfo *info;
-    strList *str_list, *entry;
+    strList *str_list;
     int i;

     info = g_malloc0(sizeof(*info));
@@ -491,19 +488,15 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)

     str_list = NULL;
     for (i = 0; i < n->mac_table.first_multi; i++) {
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = qemu_mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
-        entry->next = str_list;
-        str_list = entry;
+        QAPI_LIST_PREPEND(str_list,
+                      qemu_mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN));
     }
     info->unicast_table = str_list;

     str_list = NULL;
     for (i = n->mac_table.first_multi; i < n->mac_table.in_use; i++) {
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = qemu_mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
-        entry->next = str_list;
-        str_list = entry;
+        QAPI_LIST_PREPEND(str_list,
+                      qemu_mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN));
     }
     info->multicast_table = str_list;
     info->vlan_table = get_vlan_table(n);
diff --git a/migration/migration.c b/migration/migration.c
index bba6e5148138..805712488e4d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -406,12 +406,9 @@ int migration_incoming_enable_colo(void)
 void migrate_add_address(SocketAddress *address)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
-    SocketAddressList *addrs;

-    addrs = g_new0(SocketAddressList, 1);
-    addrs->next = mis->socket_address_list;
-    mis->socket_address_list = addrs;
-    addrs->value = QAPI_CLONE(SocketAddress, address);
+    QAPI_LIST_PREPEND(mis->socket_address_list,
+                      QAPI_CLONE(SocketAddress, address));
 }

 static void qemu_start_incoming_migration(const char *uri, Error **errp)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index d99842eb1be8..ab482adef10b 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -145,14 +145,11 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
 static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
-    uint32List *list = NULL, *entry = NULL;
+    uint32List *list = NULL;
     int i;

     for (i = ms->smp.cpus - 1; i >= 0; i--) {
-        entry = g_new0(uint32List, 1);
-        entry->value = ctx->vcpu_blocktime[i];
-        entry->next = list;
-        list = entry;
+        QAPI_LIST_PREPEND(list, ctx->vcpu_blocktime[i]);
     }

     return list;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 65d8ff48494b..73b79df7d875 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1255,7 +1255,8 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
     const char *cap = qdict_get_str(qdict, "capability");
     bool state = qdict_get_bool(qdict, "state");
     Error *err = NULL;
-    MigrationCapabilityStatusList *caps = g_malloc0(sizeof(*caps));
+    MigrationCapabilityStatusList *caps = NULL;
+    MigrationCapabilityStatus *value;
     int val;

     val = qapi_enum_parse(&MigrationCapability_lookup, cap, -1, &err);
@@ -1263,14 +1264,14 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
         goto end;
     }

-    caps->value = g_malloc0(sizeof(*caps->value));
-    caps->value->capability = val;
-    caps->value->state = state;
-    caps->next = NULL;
+    value = g_malloc0(sizeof(*value));
+    value->capability = val;
+    value->state = state;
+    QAPI_LIST_PREPEND(caps, value);
     qmp_migrate_set_capabilities(caps, &err);
+    qapi_free_MigrationCapabilityStatusList(caps);

 end:
-    qapi_free_MigrationCapabilityStatusList(caps);
     hmp_handle_error(mon, err);
 }

diff --git a/monitor/misc.c b/monitor/misc.c
index fde6e36a0b54..f2ee7cd77a10 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1430,33 +1430,26 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)

     QEMU_LOCK_GUARD(&mon_fdsets_lock);
     QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
-        FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info));
-        FdsetFdInfoList *fdsetfd_list = NULL;
+        FdsetInfo *fdset_info = g_malloc0(sizeof(*fdset_info));

-        fdset_info->value = g_malloc0(sizeof(*fdset_info->value));
-        fdset_info->value->fdset_id = mon_fdset->id;
+        fdset_info->fdset_id = mon_fdset->id;

         QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
-            FdsetFdInfoList *fdsetfd_info;
+            FdsetFdInfo *fdsetfd_info;

             fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info));
-            fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value));
-            fdsetfd_info->value->fd = mon_fdset_fd->fd;
+            fdsetfd_info->fd = mon_fdset_fd->fd;
             if (mon_fdset_fd->opaque) {
-                fdsetfd_info->value->has_opaque = true;
-                fdsetfd_info->value->opaque = g_strdup(mon_fdset_fd->opaque);
+                fdsetfd_info->has_opaque = true;
+                fdsetfd_info->opaque = g_strdup(mon_fdset_fd->opaque);
             } else {
-                fdsetfd_info->value->has_opaque = false;
+                fdsetfd_info->has_opaque = false;
             }

-            fdsetfd_info->next = fdsetfd_list;
-            fdsetfd_list = fdsetfd_info;
+            QAPI_LIST_PREPEND(fdset_info->fds, fdsetfd_info);
         }

-        fdset_info->value->fds = fdsetfd_list;
-
-        fdset_info->next = fdset_list;
-        fdset_list = fdset_info;
+        QAPI_LIST_PREPEND(fdset_list, fdset_info);
     }

     return fdset_list;
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index a456762f6a84..17514f495965 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -138,18 +138,18 @@ EventInfoList *qmp_query_events(Error **errp)
      * QAPIEvent_str() and QAPIEvent_lookup[].  When the command goes,
      * they should go, too.
      */
-    EventInfoList *info, *ev_list = NULL;
+    EventInfoList *ev_list = NULL;
     QAPIEvent e;

     for (e = 0 ; e < QAPI_EVENT__MAX ; e++) {
         const char *event_name = QAPIEvent_str(e);
+        EventInfo *info;
+
         assert(event_name != NULL);
         info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->name = g_strdup(event_name);
+        info->name = g_strdup(event_name);

-        info->next = ev_list;
-        ev_list = info;
+        QAPI_LIST_PREPEND(ev_list, info);
     }

     return ev_list;
diff --git a/qemu-img.c b/qemu-img.c
index 8bdea40b58d1..d599659c7f29 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1652,14 +1652,13 @@ static void do_dirty_bitmap_merge(const char *dst_node, const char *dst_name,
                                   Error **errp)
 {
     BlockDirtyBitmapMergeSource *merge_src;
-    BlockDirtyBitmapMergeSourceList *list;
+    BlockDirtyBitmapMergeSourceList *list = NULL;

     merge_src = g_new0(BlockDirtyBitmapMergeSource, 1);
     merge_src->type = QTYPE_QDICT;
     merge_src->u.external.node = g_strdup(src_node);
     merge_src->u.external.name = g_strdup(src_name);
-    list = g_new0(BlockDirtyBitmapMergeSourceList, 1);
-    list->value = merge_src;
+    QAPI_LIST_PREPEND(list, merge_src);
     qmp_block_dirty_bitmap_merge(dst_node, dst_name, list, errp);
     qapi_free_BlockDirtyBitmapMergeSourceList(list);
 }
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index 749167e82d4f..2dda136d64f3 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -293,17 +293,12 @@ qmp_guest_ssh_get_authorized_keys(const char *username, Error **errp)

     ret = g_new0(GuestAuthorizedKeys, 1);
     for (i = 0; authkeys[i] != NULL; i++) {
-        strList *new;
-
         g_strstrip(authkeys[i]);
         if (!authkeys[i][0] || authkeys[i][0] == '#') {
             continue;
         }

-        new = g_new0(strList, 1);
-        new->value = g_strdup(authkeys[i]);
-        new->next = ret->keys;
-        ret->keys = new;
+        QAPI_LIST_PREPEND(ret->keys, g_strdup(authkeys[i]));
     }

     return g_steal_pointer(&ret);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index c089e3812006..849923b260d7 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1036,7 +1036,6 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
 {
     GuestDiskAddress *disk;
     GuestPCIAddress *pciaddr;
-    GuestDiskAddressList *list = NULL;
     bool has_hwinf;
 #ifdef CONFIG_LIBUDEV
     struct udev *udev = NULL;
@@ -1053,9 +1052,6 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
     disk->pci_controller = pciaddr;
     disk->bus_type = GUEST_DISK_BUS_TYPE_UNKNOWN;

-    list = g_new0(GuestDiskAddressList, 1);
-    list->value = disk;
-
 #ifdef CONFIG_LIBUDEV
     udev = udev_new();
     udevice = udev_device_new_from_syspath(udev, syspath);
@@ -1089,10 +1085,9 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
     }

     if (has_hwinf || disk->has_dev || disk->has_serial) {
-        list->next = fs->disk;
-        fs->disk = list;
+        QAPI_LIST_PREPEND(fs->disk, disk);
     } else {
-        qapi_free_GuestDiskAddressList(list);
+        qapi_free_GuestDiskAddress(disk);
     }
 }

@@ -1288,7 +1283,6 @@ static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
     disk->has_dependencies = true;
     while ((dep = g_dir_read_name(dp_deps)) != NULL) {
         g_autofree char *dep_dir = NULL;
-        strList *dep_item = NULL;
         char *dev_name;

         /* Add dependent disks */
@@ -1296,10 +1290,7 @@ static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
         dev_name = get_device_for_syspath(dep_dir);
         if (dev_name != NULL) {
             g_debug("  adding dependent device: %s", dev_name);
-            dep_item = g_new0(strList, 1);
-            dep_item->value = dev_name;
-            dep_item->next = disk->dependencies;
-            disk->dependencies = dep_item;
+            QAPI_LIST_PREPEND(disk->dependencies, dev_name);
         }
     }
     g_dir_close(dp_deps);
@@ -1318,7 +1309,7 @@ static GuestDiskInfoList *get_disk_partitions(
     const char *disk_name, const char *disk_dir,
     const char *disk_dev)
 {
-    GuestDiskInfoList *item, *ret = list;
+    GuestDiskInfoList *ret = list;
     struct dirent *de_disk;
     DIR *dp_disk = NULL;
     size_t len = strlen(disk_name);
@@ -1352,15 +1343,9 @@ static GuestDiskInfoList *get_disk_partitions(
         partition->name = dev_name;
         partition->partition = true;
         /* Add parent disk as dependent for easier tracking of hierarchy */
-        partition->dependencies = g_new0(strList, 1);
-        partition->dependencies->value = g_strdup(disk_dev);
-        partition->has_dependencies = true;
-
-        item = g_new0(GuestDiskInfoList, 1);
-        item->value = partition;
-        item->next = ret;
-        ret = item;
+        QAPI_LIST_PREPEND(partition->dependencies, g_strdup(disk_dev));

+        QAPI_LIST_PREPEND(ret, partition);
     }
     closedir(dp_disk);

@@ -1369,7 +1354,7 @@ static GuestDiskInfoList *get_disk_partitions(

 GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
 {
-    GuestDiskInfoList *item, *ret = NULL;
+    GuestDiskInfoList *ret = NULL;
     GuestDiskInfo *disk;
     DIR *dp = NULL;
     struct dirent *de = NULL;
@@ -1415,10 +1400,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
         disk->partition = false;
         disk->alias = get_alias_for_syspath(disk_dir);
         disk->has_alias = (disk->alias != NULL);
-        item = g_new0(GuestDiskInfoList, 1);
-        item->value = disk;
-        item->next = ret;
-        ret = item;
+        QAPI_LIST_PREPEND(ret, disk);

         /* Get address for non-virtual devices */
         bool is_virtual = is_disk_virtual(disk_dir, &local_err);
@@ -1495,7 +1477,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
 {
     FsMountList mounts;
     struct FsMount *mount;
-    GuestFilesystemInfoList *new, *ret = NULL;
+    GuestFilesystemInfoList *ret = NULL;
     Error *local_err = NULL;

     QTAILQ_INIT(&mounts);
@@ -1508,10 +1490,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
     QTAILQ_FOREACH(mount, &mounts, next) {
         g_debug("Building guest fsinfo for '%s'", mount->dirname);

-        new = g_malloc0(sizeof(*ret));
-        new->value = build_guest_fsinfo(mount, &local_err);
-        new->next = ret;
-        ret = new;
+        QAPI_LIST_PREPEND(ret, build_guest_fsinfo(mount, &local_err));
         if (local_err) {
             error_propagate(errp, local_err);
             qapi_free_GuestFilesystemInfoList(ret);
@@ -1777,7 +1756,6 @@ GuestFilesystemTrimResponse *
 qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
 {
     GuestFilesystemTrimResponse *response;
-    GuestFilesystemTrimResultList *list;
     GuestFilesystemTrimResult *result;
     int ret = 0;
     FsMountList mounts;
@@ -1801,10 +1779,7 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
         result = g_malloc0(sizeof(*result));
         result->path = g_strdup(mount->dirname);

-        list = g_malloc0(sizeof(*list));
-        list->value = result;
-        list->next = response->paths;
-        response->paths = list;
+        QAPI_LIST_PREPEND(response->paths, result);

         fd = qemu_open_old(mount->dirname, O_RDONLY);
         if (fd == -1) {
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index ba1fd07d0625..684639bd131e 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -874,7 +874,7 @@ err_close:
 static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
 {
     Error *local_err = NULL;
-    GuestDiskAddressList *list = NULL, *cur_item = NULL;
+    GuestDiskAddressList *list = NULL;
     GuestDiskAddress *disk = NULL;
     int i;
     HANDLE vol_h;
@@ -926,10 +926,8 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
                 error_free(local_err);
                 goto out;
             }
-            list = g_malloc0(sizeof(*list));
-            list->value = disk;
+            QAPI_LIST_PREPEND(list, disk);
             disk = NULL;
-            list->next = NULL;
             goto out;
         } else {
             error_setg_win32(errp, GetLastError(),
@@ -960,11 +958,8 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
             error_propagate(errp, local_err);
             goto out;
         }
-        cur_item = g_malloc0(sizeof(*list));
-        cur_item->value = disk;
+        QAPI_LIST_PREPEND(list, disk);
         disk = NULL;
-        cur_item->next = list;
-        list = cur_item;
     }


@@ -982,7 +977,7 @@ out:
 GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
 {
     ERRP_GUARD();
-    GuestDiskInfoList *new = NULL, *ret = NULL;
+    GuestDiskInfoList *ret = NULL;
     HDEVINFO dev_info;
     SP_DEVICE_INTERFACE_DATA dev_iface_data;
     int i;
@@ -1064,10 +1059,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
             disk->has_address = true;
         }

-        new = g_malloc0(sizeof(GuestDiskInfoList));
-        new->value = disk;
-        new->next = ret;
-        ret = new;
+        QAPI_LIST_PREPEND(ret, disk);
     }

     SetupDiDestroyDeviceInfoList(dev_info);
@@ -1165,7 +1157,7 @@ free:
 GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
 {
     HANDLE vol_h;
-    GuestFilesystemInfoList *new, *ret = NULL;
+    GuestFilesystemInfoList *ret = NULL;
     char guid[256];

     vol_h = FindFirstVolume(guid, sizeof(guid));
@@ -1183,10 +1175,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
             error_free(local_err);
             continue;
         }
-        new = g_malloc(sizeof(*ret));
-        new->value = info;
-        new->next = ret;
-        ret = new;
+        QAPI_LIST_PREPEND(ret, info);
     } while (FindNextVolume(vol_h, guid, sizeof(guid)));

     if (GetLastError() != ERROR_NO_MORE_FILES) {
@@ -1330,7 +1319,6 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)

     do {
         GuestFilesystemTrimResult *res;
-        GuestFilesystemTrimResultList *list;
         PWCHAR uc_path;
         DWORD char_count = 0;
         char *path, *out;
@@ -1369,11 +1357,7 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)

         res->path = path;

-        list = g_new0(GuestFilesystemTrimResultList, 1);
-        list->value = res;
-        list->next = resp->paths;
-
-        resp->paths = list;
+        QAPI_LIST_PREPEND(resp->paths, res);

         memset(argv, 0, sizeof(argv));
         argv[0] = (gchar *)"defrag.exe";
diff --git a/qga/commands.c b/qga/commands.c
index 3dcd5fbe5c4d..e866fc7081bb 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -66,17 +66,13 @@ static void qmp_command_info(const QmpCommand *cmd, void *opaque)
 {
     GuestAgentInfo *info = opaque;
     GuestAgentCommandInfo *cmd_info;
-    GuestAgentCommandInfoList *cmd_info_list;

     cmd_info = g_new0(GuestAgentCommandInfo, 1);
     cmd_info->name = g_strdup(qmp_command_name(cmd));
     cmd_info->enabled = qmp_command_is_enabled(cmd);
     cmd_info->success_response = qmp_has_success_response(cmd);

-    cmd_info_list = g_new0(GuestAgentCommandInfoList, 1);
-    cmd_info_list->value = cmd_info;
-    cmd_info_list->next = info->supported_commands;
-    info->supported_commands = cmd_info_list;
+    QAPI_LIST_PREPEND(info->supported_commands, cmd_info);
 }

 struct GuestAgentInfo *qmp_guest_info(Error **errp)
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 2dd233f293bb..b40ac39f3008 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -46,14 +46,12 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)

     object_property_iter_init(&iter, obj);
     while ((prop = object_property_iter_next(&iter))) {
-        ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry));
+        ObjectPropertyInfo *value = g_malloc0(sizeof(ObjectPropertyInfo));

-        entry->value = g_malloc0(sizeof(ObjectPropertyInfo));
-        entry->next = props;
-        props = entry;
+        QAPI_LIST_PREPEND(props, value);

-        entry->value->name = g_strdup(prop->name);
-        entry->value->type = g_strdup(prop->type);
+        value->name = g_strdup(prop->name);
+        value->type = g_strdup(prop->type);
     }

     return props;
@@ -90,7 +88,7 @@ QObject *qmp_qom_get(const char *path, const char *property, Error **errp)

 static void qom_list_types_tramp(ObjectClass *klass, void *data)
 {
-    ObjectTypeInfoList *e, **pret = data;
+    ObjectTypeInfoList **pret = data;
     ObjectTypeInfo *info;
     ObjectClass *parent = object_class_get_parent(klass);

@@ -102,10 +100,7 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
         info->parent = g_strdup(object_class_get_name(parent));
     }

-    e = g_malloc0(sizeof(*e));
-    e->value = info;
-    e->next = *pret;
-    *pret = e;
+    QAPI_LIST_PREPEND(*pret, info);
 }

 ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
@@ -150,7 +145,6 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
     object_property_iter_init(&iter, obj);
     while ((prop = object_property_iter_next(&iter))) {
         ObjectPropertyInfo *info;
-        ObjectPropertyInfoList *entry;

         /* Skip Object and DeviceState properties */
         if (strcmp(prop->name, "type") == 0 ||
@@ -176,10 +170,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
         info->default_value = qobject_ref(prop->defval);
         info->has_default_value = !!info->default_value;

-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = prop_list;
-        prop_list = entry;
+        QAPI_LIST_PREPEND(prop_list, info);
     }

     object_unref(obj);
@@ -217,7 +208,6 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
     }
     while ((prop = object_property_iter_next(&iter))) {
         ObjectPropertyInfo *info;
-        ObjectPropertyInfoList *entry;

         info = g_malloc0(sizeof(*info));
         info->name = g_strdup(prop->name);
@@ -225,10 +215,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         info->has_description = !!prop->description;
         info->description = g_strdup(prop->description);

-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = prop_list;
-        prop_list = entry;
+        QAPI_LIST_PREPEND(prop_list, info);
     }

     object_unref(obj);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7b8bcd69030f..2d0d4cd1e102 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8283,7 +8283,6 @@ static void arm_cpu_add_definition(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;
     const char *typename;

@@ -8293,10 +8292,7 @@ static void arm_cpu_add_definition(gpointer data, gpointer user_data)
                            strlen(typename) - strlen("-" TYPE_ARM_CPU));
     info->q_typename = g_strdup(typename);

-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
-    entry->next = *cpu_list;
-    *cpu_list = entry;
+    QAPI_LIST_PREPEND(*cpu_list, info);
 }

 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 169d8a64b651..198b14e95e2e 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -42,15 +42,6 @@ static GICCapability *gic_cap_new(int version)
     return cap;
 }

-static GICCapabilityList *gic_cap_list_add(GICCapabilityList *head,
-                                           GICCapability *cap)
-{
-    GICCapabilityList *item = g_new0(GICCapabilityList, 1);
-    item->value = cap;
-    item->next = head;
-    return item;
-}
-
 static inline void gic_cap_kvm_probe(GICCapability *v2, GICCapability *v3)
 {
 #ifdef CONFIG_KVM
@@ -84,8 +75,8 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)

     gic_cap_kvm_probe(v2, v3);

-    head = gic_cap_list_add(head, v2);
-    head = gic_cap_list_add(head, v3);
+    QAPI_LIST_PREPEND(head, v2);
+    QAPI_LIST_PREPEND(head, v3);

     return head;
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0d20e156f258..35459a38bb1c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5014,7 +5014,6 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
     ObjectClass *oc = data;
     X86CPUClass *cc = X86_CPU_CLASS(oc);
     CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;

     info = g_malloc0(sizeof(*info));
@@ -5039,10 +5038,7 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
         info->has_alias_of = !!info->alias_of;
     }

-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
-    entry->next = *cpu_list;
-    *cpu_list = entry;
+    QAPI_LIST_PREPEND(*cpu_list, info);
 }

 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index aadc6f8e74d7..b2cd69ff7f9a 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -543,7 +543,6 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
     CpuDefinitionInfoList **cpu_list = user_data;
-    CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;
     const char *typename;

@@ -553,10 +552,7 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data)
                            strlen(typename) - strlen("-" TYPE_MIPS_CPU));
     info->q_typename = g_strdup(typename);

-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
-    entry->next = *cpu_list;
-    *cpu_list = entry;
+    QAPI_LIST_PREPEND(*cpu_list, info);
 }

 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index b5abff8befea..a23fd3e32b77 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -427,7 +427,6 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
 {
     struct CpuDefinitionInfoListData *cpu_list_data = opaque;
     CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
-    CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;
     char *name = g_strdup(object_class_get_name(klass));
     S390CPUClass *scc = S390_CPU_CLASS(klass);
@@ -454,10 +453,7 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
         object_unref(obj);
     }

-    entry = g_new0(CpuDefinitionInfoList, 1);
-    entry->value = info;
-    entry->next = *cpu_list;
-    *cpu_list = entry;
+    QAPI_LIST_PREPEND(*cpu_list, info);
 }

 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
@@ -624,12 +620,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
 static void list_add_feat(const char *name, void *opaque)
 {
     strList **last = (strList **) opaque;
-    strList *entry;

-    entry = g_new0(strList, 1);
-    entry->value = g_strdup(name);
-    entry->next = *last;
-    *last = entry;
+    QAPI_LIST_PREPEND(*last, g_strdup(name));
 }

 CpuModelCompareInfo *qmp_query_cpu_model_comparison(CpuModelInfo *infoa,
diff --git a/tests/test-clone-visitor.c b/tests/test-clone-visitor.c
index 5e1e8b2f5e8a..4944b3d85734 100644
--- a/tests/test-clone-visitor.c
+++ b/tests/test-clone-visitor.c
@@ -65,16 +65,13 @@ static void test_clone_alternate(void)

 static void test_clone_list_union(void)
 {
-    uint8List *src, *dst;
+    uint8List *src = NULL, *dst;
     uint8List *tmp = NULL;
     int i;

     /* Build list in reverse */
     for (i = 10; i; i--) {
-        src = g_new0(uint8List, 1);
-        src->next = tmp;
-        src->value = i;
-        tmp = src;
+        QAPI_LIST_PREPEND(src, i);
     }

     dst = QAPI_CLONE(uint8List, src);
diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index 1c856d9bd20a..b20ab8b29b85 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -223,7 +223,8 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
                                   const void *unused)
 {
     const char *value_str = "list value";
-    TestStructList *p, *head = NULL;
+    TestStruct *value;
+    TestStructList *head = NULL;
     const int max_items = 10;
     bool value_bool = true;
     int value_int = 10;
@@ -233,14 +234,12 @@ static void test_visitor_out_list(TestOutputVisitorData *data,

     /* Build the list in reverse order... */
     for (i = 0; i < max_items; i++) {
-        p = g_malloc0(sizeof(*p));
-        p->value = g_malloc0(sizeof(*p->value));
-        p->value->integer = value_int + (max_items - i - 1);
-        p->value->boolean = value_bool;
-        p->value->string = g_strdup(value_str);
+        value = g_malloc0(sizeof(*value));
+        value->integer = value_int + (max_items - i - 1);
+        value->boolean = value_bool;
+        value->string = g_strdup(value_str);

-        p->next = head;
-        head = p;
+        QAPI_LIST_PREPEND(head, value);
     }

     visit_type_TestStructList(data->ov, NULL, &head, &error_abort);
@@ -270,26 +269,25 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
 static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
                                             const void *unused)
 {
-    UserDefTwoList *p, *head = NULL;
+    UserDefTwo *value;
+    UserDefTwoList *head = NULL;
     const char string[] = "foo bar";
     int i, max_count = 1024;

     for (i = 0; i < max_count; i++) {
-        p = g_malloc0(sizeof(*p));
-        p->value = g_malloc0(sizeof(*p->value));
+        value = g_malloc0(sizeof(*value));

-        p->value->string0 = g_strdup(string);
-        p->value->dict1 = g_new0(UserDefTwoDict, 1);
-        p->value->dict1->string1 = g_strdup(string);
-        p->value->dict1->dict2 = g_new0(UserDefTwoDictDict, 1);
-        p->value->dict1->dict2->userdef = g_new0(UserDefOne, 1);
-        p->value->dict1->dict2->userdef->string = g_strdup(string);
-        p->value->dict1->dict2->userdef->integer = 42;
-        p->value->dict1->dict2->string = g_strdup(string);
-        p->value->dict1->has_dict3 = false;
+        value->string0 = g_strdup(string);
+        value->dict1 = g_new0(UserDefTwoDict, 1);
+        value->dict1->string1 = g_strdup(string);
+        value->dict1->dict2 = g_new0(UserDefTwoDictDict, 1);
+        value->dict1->dict2->userdef = g_new0(UserDefOne, 1);
+        value->dict1->dict2->userdef->string = g_strdup(string);
+        value->dict1->dict2->userdef->integer = 42;
+        value->dict1->dict2->string = g_strdup(string);
+        value->dict1->has_dict3 = false;

-        p->next = head;
-        head = p;
+        QAPI_LIST_PREPEND(head, value);
     }

     qapi_free_UserDefTwoList(head);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 1c5a8b94ea87..12275e56d862 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -351,135 +351,51 @@ static void test_primitive_lists(gconstpointer opaque)
     for (i = 0; i < 32; i++) {
         switch (pl.type) {
         case PTYPE_STRING: {
-            strList *tmp = g_new0(strList, 1);
-            tmp->value = g_strdup(pt->value.string);
-            if (pl.value.strings == NULL) {
-                pl.value.strings = tmp;
-            } else {
-                tmp->next = pl.value.strings;
-                pl.value.strings = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.strings, g_strdup(pt->value.string));
             break;
         }
         case PTYPE_INTEGER: {
-            intList *tmp = g_new0(intList, 1);
-            tmp->value = pt->value.integer;
-            if (pl.value.integers == NULL) {
-                pl.value.integers = tmp;
-            } else {
-                tmp->next = pl.value.integers;
-                pl.value.integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.integers, pt->value.integer);
             break;
         }
         case PTYPE_S8: {
-            int8List *tmp = g_new0(int8List, 1);
-            tmp->value = pt->value.s8;
-            if (pl.value.s8_integers == NULL) {
-                pl.value.s8_integers = tmp;
-            } else {
-                tmp->next = pl.value.s8_integers;
-                pl.value.s8_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.s8_integers, pt->value.s8);
             break;
         }
         case PTYPE_S16: {
-            int16List *tmp = g_new0(int16List, 1);
-            tmp->value = pt->value.s16;
-            if (pl.value.s16_integers == NULL) {
-                pl.value.s16_integers = tmp;
-            } else {
-                tmp->next = pl.value.s16_integers;
-                pl.value.s16_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.s16_integers, pt->value.s16);
             break;
         }
         case PTYPE_S32: {
-            int32List *tmp = g_new0(int32List, 1);
-            tmp->value = pt->value.s32;
-            if (pl.value.s32_integers == NULL) {
-                pl.value.s32_integers = tmp;
-            } else {
-                tmp->next = pl.value.s32_integers;
-                pl.value.s32_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.s32_integers, pt->value.s32);
             break;
         }
         case PTYPE_S64: {
-            int64List *tmp = g_new0(int64List, 1);
-            tmp->value = pt->value.s64;
-            if (pl.value.s64_integers == NULL) {
-                pl.value.s64_integers = tmp;
-            } else {
-                tmp->next = pl.value.s64_integers;
-                pl.value.s64_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.s64_integers, pt->value.s64);
             break;
         }
         case PTYPE_U8: {
-            uint8List *tmp = g_new0(uint8List, 1);
-            tmp->value = pt->value.u8;
-            if (pl.value.u8_integers == NULL) {
-                pl.value.u8_integers = tmp;
-            } else {
-                tmp->next = pl.value.u8_integers;
-                pl.value.u8_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.u8_integers, pt->value.u8);
             break;
         }
         case PTYPE_U16: {
-            uint16List *tmp = g_new0(uint16List, 1);
-            tmp->value = pt->value.u16;
-            if (pl.value.u16_integers == NULL) {
-                pl.value.u16_integers = tmp;
-            } else {
-                tmp->next = pl.value.u16_integers;
-                pl.value.u16_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.u16_integers, pt->value.u16);
             break;
         }
         case PTYPE_U32: {
-            uint32List *tmp = g_new0(uint32List, 1);
-            tmp->value = pt->value.u32;
-            if (pl.value.u32_integers == NULL) {
-                pl.value.u32_integers = tmp;
-            } else {
-                tmp->next = pl.value.u32_integers;
-                pl.value.u32_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.u32_integers, pt->value.u32);
             break;
         }
         case PTYPE_U64: {
-            uint64List *tmp = g_new0(uint64List, 1);
-            tmp->value = pt->value.u64;
-            if (pl.value.u64_integers == NULL) {
-                pl.value.u64_integers = tmp;
-            } else {
-                tmp->next = pl.value.u64_integers;
-                pl.value.u64_integers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.u64_integers, pt->value.u64);
             break;
         }
         case PTYPE_NUMBER: {
-            numberList *tmp = g_new0(numberList, 1);
-            tmp->value = pt->value.number;
-            if (pl.value.numbers == NULL) {
-                pl.value.numbers = tmp;
-            } else {
-                tmp->next = pl.value.numbers;
-                pl.value.numbers = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.numbers, pt->value.number);
             break;
         }
         case PTYPE_BOOLEAN: {
-            boolList *tmp = g_new0(boolList, 1);
-            tmp->value = pt->value.boolean;
-            if (pl.value.booleans == NULL) {
-                pl.value.booleans = tmp;
-            } else {
-                tmp->next = pl.value.booleans;
-                pl.value.booleans = tmp;
-            }
+            QAPI_LIST_PREPEND(pl.value.booleans, pt->value.boolean);
             break;
         }
         default:
@@ -704,10 +620,7 @@ static void test_nested_struct_list(gconstpointer opaque)
     int i = 0;

     for (i = 0; i < 8; i++) {
-        tmp = g_new0(UserDefTwoList, 1);
-        tmp->value = nested_struct_create();
-        tmp->next = listp;
-        listp = tmp;
+        QAPI_LIST_PREPEND(listp, nested_struct_create());
     }

     ops->serialize(listp, &serialize_data, visit_nested_struct_list,
diff --git a/trace/qmp.c b/trace/qmp.c
index 38246e1aa692..85f81e47cc6d 100644
--- a/trace/qmp.c
+++ b/trace/qmp.c
@@ -92,39 +92,37 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
     /* Get states (all errors checked above) */
     trace_event_iter_init(&iter, name);
     while ((ev = trace_event_iter_next(&iter)) != NULL) {
-        TraceEventInfoList *elem;
+        TraceEventInfo *value;
         bool is_vcpu = trace_event_is_vcpu(ev);
         if (has_vcpu && !is_vcpu) {
             continue;
         }

-        elem = g_new(TraceEventInfoList, 1);
-        elem->value = g_new(TraceEventInfo, 1);
-        elem->value->vcpu = is_vcpu;
-        elem->value->name = g_strdup(trace_event_get_name(ev));
+        value = g_new(TraceEventInfo, 1);
+        value->vcpu = is_vcpu;
+        value->name = g_strdup(trace_event_get_name(ev));

         if (!trace_event_get_state_static(ev)) {
-            elem->value->state = TRACE_EVENT_STATE_UNAVAILABLE;
+            value->state = TRACE_EVENT_STATE_UNAVAILABLE;
         } else {
             if (has_vcpu) {
                 if (is_vcpu) {
                     if (trace_event_get_vcpu_state_dynamic(cpu, ev)) {
-                        elem->value->state = TRACE_EVENT_STATE_ENABLED;
+                        value->state = TRACE_EVENT_STATE_ENABLED;
                     } else {
-                        elem->value->state = TRACE_EVENT_STATE_DISABLED;
+                        value->state = TRACE_EVENT_STATE_DISABLED;
                     }
                 }
                 /* else: already skipped above */
             } else {
                 if (trace_event_get_state_dynamic(ev)) {
-                    elem->value->state = TRACE_EVENT_STATE_ENABLED;
+                    value->state = TRACE_EVENT_STATE_ENABLED;
                 } else {
-                    elem->value->state = TRACE_EVENT_STATE_DISABLED;
+                    value->state = TRACE_EVENT_STATE_DISABLED;
                 }
             }
         }
-        elem->next = events;
-        events = elem;
+        QAPI_LIST_PREPEND(events, value);
     }

     return events;
diff --git a/ui/input.c b/ui/input.c
index 4791b089c746..8ac407dec485 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -571,7 +571,7 @@ void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
 MouseInfoList *qmp_query_mice(Error **errp)
 {
     MouseInfoList *mice_list = NULL;
-    MouseInfoList *info;
+    MouseInfo *info;
     QemuInputHandlerState *s;
     bool current = true;

@@ -581,16 +581,14 @@ MouseInfoList *qmp_query_mice(Error **errp)
             continue;
         }

-        info = g_new0(MouseInfoList, 1);
-        info->value = g_new0(MouseInfo, 1);
-        info->value->index = s->id;
-        info->value->name = g_strdup(s->handler->name);
-        info->value->absolute = s->handler->mask & INPUT_EVENT_MASK_ABS;
-        info->value->current = current;
+        info = g_new0(MouseInfo, 1);
+        info->index = s->id;
+        info->name = g_strdup(s->handler->name);
+        info->absolute = s->handler->mask & INPUT_EVENT_MASK_ABS;
+        info->current = current;

         current = false;
-        info->next = mice_list;
-        mice_list = info;
+        QAPI_LIST_PREPEND(mice_list, info);
     }

     return mice_list;
diff --git a/ui/vnc.c b/ui/vnc.c
index 34e6dc437c0f..7452ac7df2ce 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -365,14 +365,11 @@ static VncDisplay *vnc_display_find(const char *id)

 static VncClientInfoList *qmp_query_client_list(VncDisplay *vd)
 {
-    VncClientInfoList *cinfo, *prev = NULL;
+    VncClientInfoList *prev = NULL;
     VncState *client;

     QTAILQ_FOREACH(client, &vd->clients, next) {
-        cinfo = g_new0(VncClientInfoList, 1);
-        cinfo->value = qmp_query_vnc_client(client);
-        cinfo->next = prev;
-        prev = cinfo;
+        QAPI_LIST_PREPEND(prev, qmp_query_vnc_client(client));
     }
     return prev;
 }
@@ -453,7 +450,6 @@ static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
                                                   int subauth,
                                                   VncServerInfo2List *prev)
 {
-    VncServerInfo2List *list;
     VncServerInfo2 *info;
     Error *err = NULL;
     SocketAddress *addr;
@@ -476,10 +472,8 @@ static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
     qmp_query_auth(auth, subauth, &info->auth,
                    &info->vencrypt, &info->has_vencrypt);

-    list = g_new0(VncServerInfo2List, 1);
-    list->value = info;
-    list->next = prev;
-    return list;
+    QAPI_LIST_PREPEND(prev, info);
+    return prev;
 }

 static void qmp_query_auth(int auth, int subauth,
@@ -554,7 +548,7 @@ static void qmp_query_auth(int auth, int subauth,

 VncInfo2List *qmp_query_vnc_servers(Error **errp)
 {
-    VncInfo2List *item, *prev = NULL;
+    VncInfo2List *prev = NULL;
     VncInfo2 *info;
     VncDisplay *vd;
     DeviceState *dev;
@@ -583,10 +577,7 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
                 vd->ws_subauth, info->server);
         }

-        item = g_new0(VncInfo2List, 1);
-        item->value = info;
-        item->next = prev;
-        prev = item;
+        QAPI_LIST_PREPEND(prev, info);
     }
     return prev;
 }
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 725e3d7e4b13..e2a700b28450 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -55,7 +55,7 @@ QemuOpts *qemu_find_opts_singleton(const char *group)

 static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc)
 {
-    CommandLineParameterInfoList *param_list = NULL, *entry;
+    CommandLineParameterInfoList *param_list = NULL;
     CommandLineParameterInfo *info;
     int i;

@@ -87,10 +87,7 @@ static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc)
             info->q_default = g_strdup(desc[i].def_value_str);
         }

-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = param_list;
-        param_list = entry;
+        QAPI_LIST_PREPEND(param_list, info);
     }

     return param_list;
@@ -246,7 +243,7 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
                                                           const char *option,
                                                           Error **errp)
 {
-    CommandLineOptionInfoList *conf_list = NULL, *entry;
+    CommandLineOptionInfoList *conf_list = NULL;
     CommandLineOptionInfo *info;
     int i;

@@ -262,10 +259,7 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
                 info->parameters =
                     query_option_descs(vm_config_groups[i]->desc);
             }
-            entry = g_malloc0(sizeof(*entry));
-            entry->value = info;
-            entry->next = conf_list;
-            conf_list = entry;
+            QAPI_LIST_PREPEND(conf_list, info);
         }
     }

diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index a4d0038828d9..3c05a173437d 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10566,7 +10566,6 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
     ObjectClass *oc = data;
     CpuDefinitionInfoList **first = user_data;
     const char *typename;
-    CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;

     typename = object_class_get_name(oc);
@@ -10574,10 +10573,7 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
     info->name = g_strndup(typename,
                            strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX));

-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
-    entry->next = *first;
-    *first = entry;
+    QAPI_LIST_PREPEND(*first, info);
 }

 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
@@ -10593,7 +10589,6 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
         PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
         ObjectClass *oc;
-        CpuDefinitionInfoList *entry;
         CpuDefinitionInfo *info;

         oc = ppc_cpu_class_by_name(alias->model);
@@ -10605,10 +10600,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
         info->name = g_strdup(alias->alias);
         info->q_typename = g_strdup(object_class_get_name(oc));

-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = cpu_list;
-        cpu_list = entry;
+        QAPI_LIST_PREPEND(cpu_list, info);
     }

     return cpu_list;
-- 
2.29.2



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

* [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
                   ` (3 preceding siblings ...)
  2020-12-23 22:10 ` [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
@ 2020-12-23 22:11 ` Eric Blake
  2020-12-24  6:14   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 13:04   ` Markus Armbruster
  2020-12-23 22:11 ` [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
  2020-12-23 22:11 ` [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
  6 siblings, 2 replies; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Roth, armbru

Similar to the existing QAPI_LIST_PREPEND, but designed for use where
we want to preserve insertion order.  Callers will be added in
upcoming patches.  Note the difference in signature: PREPEND takes
List*, APPEND takes List**.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/util.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 6178e98e97a5..8b4967990c0d 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -37,4 +37,17 @@ int parse_qapi_name(const char *name, bool complete);
     (list) = _tmp; \
 } while (0)

+/*
+ * For any pointer to a GenericList @tail, insert @element at the back and
+ * update the tail.
+ *
+ * Note that this macro evaluates @element exactly once, so it is safe
+ * to have side-effects with that argument.
+ */
+#define QAPI_LIST_APPEND(tail, element) do { \
+    *(tail) = g_malloc0(sizeof(**(tail))); \
+    (*(tail))->value = (element); \
+    (tail) = &(*tail)->next; \
+} while (0)
+
 #endif
-- 
2.29.2



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

* [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
                   ` (4 preceding siblings ...)
  2020-12-23 22:11 ` [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
@ 2020-12-23 22:11 ` Eric Blake
  2020-12-24  9:56   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 13:16   ` Markus Armbruster
  2020-12-23 22:11 ` [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
  6 siblings, 2 replies; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy,
	Daniel P. Berrangé,
	Eduardo Habkost, open list:Dirty Bitmaps, Michael S. Tsirkin,
	open list:Trivial patches, Michael Roth, Richard Henderson,
	armbru, Max Reitz, Michael Tokarev, Paolo Bonzini, Igor Mammedov,
	John Snow, Dr. David Alan Gilbert, Laurent Vivier

The easiest spots to use QAPI_LIST_APPEND are where we already have an
obvious pointer to the tail of a list.  While at it, consistently use
the variable name 'tail' for that purpose.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 backends/hostmem.c                  | 10 ++--
 block/dirty-bitmap.c                |  8 ++-
 block/export/export.c               |  7 +--
 block/qapi.c                        | 23 ++------
 block/qcow2-bitmap.c                | 15 ++----
 block/vmdk.c                        |  9 ++--
 blockdev.c                          | 13 ++---
 crypto/block-luks.c                 |  9 ++--
 hw/acpi/cpu.c                       |  8 +--
 hw/acpi/memory_hotplug.c            |  9 ++--
 iothread.c                          | 12 ++---
 job-qmp.c                           | 13 ++---
 monitor/hmp-cmds.c                  | 10 ++--
 monitor/qmp-cmds-control.c          |  9 ++--
 qemu-img.c                          |  8 +--
 qga/commands-posix.c                | 31 ++++-------
 qga/commands-win32.c                | 11 ++--
 scsi/pr-manager.c                   | 10 +---
 target/i386/cpu.c                   | 23 +++-----
 tests/test-qobject-output-visitor.c | 84 +++++++++--------------------
 tests/test-string-output-visitor.c  |  6 +--
 21 files changed, 99 insertions(+), 229 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4bde00e8e74d..b4c83c1cfa29 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -80,7 +80,7 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
 {
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
     uint16List *host_nodes = NULL;
-    uint16List **node = &host_nodes;
+    uint16List **tail = &host_nodes;
     unsigned long value;

     value = find_first_bit(backend->host_nodes, MAX_NODES);
@@ -88,9 +88,7 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
         goto ret;
     }

-    *node = g_malloc0(sizeof(**node));
-    (*node)->value = value;
-    node = &(*node)->next;
+    QAPI_LIST_APPEND(tail, value);

     do {
         value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
@@ -98,9 +96,7 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
             break;
         }

-        *node = g_malloc0(sizeof(**node));
-        (*node)->value = value;
-        node = &(*node)->next;
+        QAPI_LIST_APPEND(tail, value);
     } while (true);

 ret:
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c01319b188c3..9b9cd712386c 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -572,12 +572,12 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 {
     BdrvDirtyBitmap *bm;
     BlockDirtyInfoList *list = NULL;
-    BlockDirtyInfoList **plist = &list;
+    BlockDirtyInfoList **tail = &list;

     bdrv_dirty_bitmaps_lock(bs);
     QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
         BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
-        BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
+
         info->count = bdrv_get_dirty_count(bm);
         info->granularity = bdrv_dirty_bitmap_granularity(bm);
         info->has_name = !!bm->name;
@@ -588,9 +588,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
         info->persistent = bm->persistent;
         info->has_inconsistent = bm->inconsistent;
         info->inconsistent = bm->inconsistent;
-        entry->value = info;
-        *plist = entry;
-        plist = &entry->next;
+        QAPI_LIST_APPEND(tail, info);
     }
     bdrv_dirty_bitmaps_unlock(bs);

diff --git a/block/export/export.c b/block/export/export.c
index b716c1522c5d..fec7d9f73820 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -342,11 +342,10 @@ void qmp_block_export_del(const char *id,

 BlockExportInfoList *qmp_query_block_exports(Error **errp)
 {
-    BlockExportInfoList *head = NULL, **p_next = &head;
+    BlockExportInfoList *head = NULL, **tail = &head;
     BlockExport *exp;

     QLIST_FOREACH(exp, &block_exports, next) {
-        BlockExportInfoList *entry = g_new0(BlockExportInfoList, 1);
         BlockExportInfo *info = g_new(BlockExportInfo, 1);
         *info = (BlockExportInfo) {
             .id             = g_strdup(exp->id),
@@ -355,9 +354,7 @@ BlockExportInfoList *qmp_query_block_exports(Error **errp)
             .shutting_down  = !exp->user_owned,
         };

-        entry->value = info;
-        *p_next = entry;
-        p_next = &entry->next;
+        QAPI_LIST_APPEND(tail, info);
     }

     return head;
diff --git a/block/qapi.c b/block/qapi.c
index 0ca206f559fe..3a1186fdccf5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -418,17 +418,12 @@ static uint64List *uint64_list(uint64_t *list, int size)
 {
     int i;
     uint64List *out_list = NULL;
-    uint64List **pout_list = &out_list;
+    uint64List **tail = &out_list;

     for (i = 0; i < size; i++) {
-        uint64List *entry = g_new(uint64List, 1);
-        entry->value = list[i];
-        *pout_list = entry;
-        pout_list = &entry->next;
+        QAPI_LIST_APPEND(tail, list[i]);
     }

-    *pout_list = NULL;
-
     return out_list;
 }

@@ -636,26 +631,21 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
                                      bool query_nodes,
                                      Error **errp)
 {
-    BlockStatsList *head = NULL, **p_next = &head;
+    BlockStatsList *head = NULL, **tail = &head;
     BlockBackend *blk;
     BlockDriverState *bs;

     /* Just to be safe if query_nodes is not always initialized */
     if (has_query_nodes && query_nodes) {
         for (bs = bdrv_next_node(NULL); bs; bs = bdrv_next_node(bs)) {
-            BlockStatsList *info = g_malloc0(sizeof(*info));
             AioContext *ctx = bdrv_get_aio_context(bs);

             aio_context_acquire(ctx);
-            info->value = bdrv_query_bds_stats(bs, false);
+            QAPI_LIST_APPEND(tail, bdrv_query_bds_stats(bs, false));
             aio_context_release(ctx);
-
-            *p_next = info;
-            p_next = &info->next;
         }
     } else {
         for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
-            BlockStatsList *info;
             AioContext *ctx = blk_get_aio_context(blk);
             BlockStats *s;
             char *qdev;
@@ -680,10 +670,7 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
             bdrv_query_blk_stats(s->stats, blk);
             aio_context_release(ctx);

-            info = g_malloc0(sizeof(*info));
-            info->value = s;
-            *p_next = info;
-            p_next = &info->next;
+            QAPI_LIST_APPEND(tail, s);
         }
     }

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index d7a31a8ddcdb..5eef82fa55f1 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1061,7 +1061,7 @@ fail:
 static Qcow2BitmapInfoFlagsList *get_bitmap_info_flags(uint32_t flags)
 {
     Qcow2BitmapInfoFlagsList *list = NULL;
-    Qcow2BitmapInfoFlagsList **plist = &list;
+    Qcow2BitmapInfoFlagsList **tail = &list;
     int i;

     static const struct {
@@ -1076,11 +1076,7 @@ static Qcow2BitmapInfoFlagsList *get_bitmap_info_flags(uint32_t flags)

     for (i = 0; i < map_size; ++i) {
         if (flags & map[i].bme) {
-            Qcow2BitmapInfoFlagsList *entry =
-                g_new0(Qcow2BitmapInfoFlagsList, 1);
-            entry->value = map[i].info;
-            *plist = entry;
-            plist = &entry->next;
+            QAPI_LIST_APPEND(tail, map[i].info);
             flags &= ~map[i].bme;
         }
     }
@@ -1105,7 +1101,7 @@ Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
     Qcow2BitmapList *bm_list;
     Qcow2Bitmap *bm;
     Qcow2BitmapInfoList *list = NULL;
-    Qcow2BitmapInfoList **plist = &list;
+    Qcow2BitmapInfoList **tail = &list;

     if (s->nb_bitmaps == 0) {
         return NULL;
@@ -1119,13 +1115,10 @@ Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,

     QSIMPLEQ_FOREACH(bm, bm_list, entry) {
         Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
-        Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
         info->granularity = 1U << bm->granularity_bits;
         info->name = g_strdup(bm->name);
         info->flags = get_bitmap_info_flags(bm->flags & ~BME_RESERVED_FLAGS);
-        obj->value = info;
-        *plist = obj;
-        plist = &obj->next;
+        QAPI_LIST_APPEND(tail, info);
     }

     bitmap_list_free(bm_list);
diff --git a/block/vmdk.c b/block/vmdk.c
index a00dc00eb47a..4499f136bdff 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2928,7 +2928,7 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs,
     int i;
     BDRVVmdkState *s = bs->opaque;
     ImageInfoSpecific *spec_info = g_new0(ImageInfoSpecific, 1);
-    ImageInfoList **next;
+    ImageInfoList **tail;

     *spec_info = (ImageInfoSpecific){
         .type = IMAGE_INFO_SPECIFIC_KIND_VMDK,
@@ -2943,12 +2943,9 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs,
         .parent_cid = s->parent_cid,
     };

-    next = &spec_info->u.vmdk.data->extents;
+    tail = &spec_info->u.vmdk.data->extents;
     for (i = 0; i < s->num_extents; i++) {
-        *next = g_new0(ImageInfoList, 1);
-        (*next)->value = vmdk_get_extent_info(&s->extents[i]);
-        (*next)->next = NULL;
-        next = &(*next)->next;
+        QAPI_LIST_APPEND(tail, vmdk_get_extent_info(&s->extents[i]));
     }

     return spec_info;
diff --git a/blockdev.c b/blockdev.c
index 2431448c5d41..0be8efa64b82 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3676,28 +3676,25 @@ void qmp_x_blockdev_change(const char *parent, bool has_child,

 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 {
-    BlockJobInfoList *head = NULL, **p_next = &head;
+    BlockJobInfoList *head = NULL, **tail = &head;
     BlockJob *job;

     for (job = block_job_next(NULL); job; job = block_job_next(job)) {
-        BlockJobInfoList *elem;
+        BlockJobInfo *value;
         AioContext *aio_context;

         if (block_job_is_internal(job)) {
             continue;
         }
-        elem = g_new0(BlockJobInfoList, 1);
         aio_context = blk_get_aio_context(job->blk);
         aio_context_acquire(aio_context);
-        elem->value = block_job_query(job, errp);
+        value = block_job_query(job, errp);
         aio_context_release(aio_context);
-        if (!elem->value) {
-            g_free(elem);
+        if (!value) {
             qapi_free_BlockJobInfoList(head);
             return NULL;
         }
-        *p_next = elem;
-        p_next = &elem->next;
+        QAPI_LIST_APPEND(tail, value);
     }

     return head;
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 564caa10949b..fe8f04ffb294 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1885,7 +1885,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
 {
     QCryptoBlockLUKS *luks = block->opaque;
     QCryptoBlockInfoLUKSSlot *slot;
-    QCryptoBlockInfoLUKSSlotList *slots = NULL, **prev = &info->u.luks.slots;
+    QCryptoBlockInfoLUKSSlotList **tail = &info->u.luks.slots;
     size_t i;

     info->u.luks.cipher_alg = luks->cipher_alg;
@@ -1902,10 +1902,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
                                   sizeof(luks->header.uuid));

     for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
-        slots = g_new0(QCryptoBlockInfoLUKSSlotList, 1);
-        *prev = slots;
-
-        slots->value = slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
+        slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
         slot->active = luks->header.key_slots[i].active ==
             QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
         slot->key_offset = luks->header.key_slots[i].key_offset_sector
@@ -1917,7 +1914,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
             slot->stripes = luks->header.key_slots[i].stripes;
         }

-        prev = &slots->next;
+        QAPI_LIST_APPEND(tail, slot);
     }

     return 0;
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 6350caa76530..3f3182007b07 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -42,16 +42,12 @@ static ACPIOSTInfo *acpi_cpu_device_status(int idx, AcpiCpuStatus *cdev)
     return info;
 }

-void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
+void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***tail)
 {
     int i;

     for (i = 0; i < cpu_st->dev_count; i++) {
-        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
-        elem->value = acpi_cpu_device_status(i, &cpu_st->devs[i]);
-        elem->next = NULL;
-        **list = elem;
-        *list = &elem->next;
+        QAPI_LIST_APPEND(*tail, acpi_cpu_device_status(i, &cpu_st->devs[i]));
     }
 }

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index f2552b2a4624..e4b836bd5e46 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -51,16 +51,13 @@ static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
     return info;
 }

-void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list)
+void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***tail)
 {
     int i;

     for (i = 0; i < mem_st->dev_count; i++) {
-        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
-        elem->value = acpi_memory_device_status(i, &mem_st->devs[i]);
-        elem->next = NULL;
-        **list = elem;
-        *list = &elem->next;
+        QAPI_LIST_APPEND(*tail,
+                         acpi_memory_device_status(i, &mem_st->devs[i]));
     }
 }

diff --git a/iothread.c b/iothread.c
index 69eff9efbc70..b9f275138257 100644
--- a/iothread.c
+++ b/iothread.c
@@ -1,7 +1,7 @@
 /*
  * Event loop thread
  *
- * Copyright Red Hat Inc., 2013
+ * Copyright Red Hat Inc., 2013, 2020
  *
  * Authors:
  *  Stefan Hajnoczi   <stefanha@redhat.com>
@@ -310,8 +310,7 @@ AioContext *iothread_get_aio_context(IOThread *iothread)

 static int query_one_iothread(Object *object, void *opaque)
 {
-    IOThreadInfoList ***prev = opaque;
-    IOThreadInfoList *elem;
+    IOThreadInfoList ***tail = opaque;
     IOThreadInfo *info;
     IOThread *iothread;

@@ -327,12 +326,7 @@ static int query_one_iothread(Object *object, void *opaque)
     info->poll_grow = iothread->poll_grow;
     info->poll_shrink = iothread->poll_shrink;

-    elem = g_new0(IOThreadInfoList, 1);
-    elem->value = info;
-    elem->next = NULL;
-
-    **prev = elem;
-    *prev = &elem->next;
+    QAPI_LIST_APPEND(*tail, info);
     return 0;
 }

diff --git a/job-qmp.c b/job-qmp.c
index 645601b2ccc1..34c4da094f22 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -164,28 +164,25 @@ static JobInfo *job_query_single(Job *job, Error **errp)

 JobInfoList *qmp_query_jobs(Error **errp)
 {
-    JobInfoList *head = NULL, **p_next = &head;
+    JobInfoList *head = NULL, **tail = &head;
     Job *job;

     for (job = job_next(NULL); job; job = job_next(job)) {
-        JobInfoList *elem;
+        JobInfo *value;
         AioContext *aio_context;

         if (job_is_internal(job)) {
             continue;
         }
-        elem = g_new0(JobInfoList, 1);
         aio_context = job->aio_context;
         aio_context_acquire(aio_context);
-        elem->value = job_query_single(job, errp);
+        value = job_query_single(job, errp);
         aio_context_release(aio_context);
-        if (!elem->value) {
-            g_free(elem);
+        if (!value) {
             qapi_free_JobInfoList(head);
             return NULL;
         }
-        *p_next = elem;
-        p_next = &elem->next;
+        QAPI_LIST_APPEND(tail, value);
     }

     return head;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 73b79df7d875..ed4131efbca6 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -76,20 +76,20 @@ void hmp_handle_error(Monitor *mon, Error *err)
 static strList *strList_from_comma_list(const char *in)
 {
     strList *res = NULL;
-    strList **hook = &res;
+    strList **tail = &res;

     while (in && in[0]) {
         char *comma = strchr(in, ',');
-        *hook = g_new0(strList, 1);
+        char *value;

         if (comma) {
-            (*hook)->value = g_strndup(in, comma - in);
+            value = g_strndup(in, comma - in);
             in = comma + 1; /* skip the , */
         } else {
-            (*hook)->value = g_strdup(in);
+            value = g_strdup(in);
             in = NULL;
         }
-        hook = &(*hook)->next;
+        QAPI_LIST_APPEND(tail, value);
     }

     return res;
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 17514f495965..87ffa70ca28d 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -104,17 +104,16 @@ VersionInfo *qmp_query_version(Error **errp)

 static void query_commands_cb(const QmpCommand *cmd, void *opaque)
 {
-    CommandInfoList *info, **list = opaque;
+    CommandInfo *info;
+    CommandInfoList **tail = opaque;

     if (!cmd->enabled) {
         return;
     }

     info = g_malloc0(sizeof(*info));
-    info->value = g_malloc0(sizeof(*info->value));
-    info->value->name = g_strdup(cmd->name);
-    info->next = *list;
-    *list = info;
+    info->name = g_strdup(cmd->name);
+    QAPI_LIST_APPEND(tail, info);
 }

 CommandInfoList *qmp_query_commands(Error **errp)
diff --git a/qemu-img.c b/qemu-img.c
index d599659c7f29..94310715ace9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2857,7 +2857,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
                                               bool chain, bool force_share)
 {
     ImageInfoList *head = NULL;
-    ImageInfoList **last = &head;
+    ImageInfoList **tail = &head;
     GHashTable *filenames;
     Error *err = NULL;

@@ -2867,7 +2867,6 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
         BlockBackend *blk;
         BlockDriverState *bs;
         ImageInfo *info;
-        ImageInfoList *elem;

         if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL)) {
             error_report("Backing file '%s' creates an infinite loop.",
@@ -2891,10 +2890,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
             goto err;
         }

-        elem = g_new0(ImageInfoList, 1);
-        elem->value = info;
-        *last = elem;
-        last = &elem->next;
+        QAPI_LIST_APPEND(tail, info);

         blk_unref(blk);

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 849923b260d7..a5058a3bd15e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2440,18 +2440,17 @@ static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
 GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
 {
     int64_t current;
-    GuestLogicalProcessorList *head, **link;
+    GuestLogicalProcessorList *head, **tail;
     long sc_max;
     Error *local_err = NULL;

     current = 0;
     head = NULL;
-    link = &head;
+    tail = &head;
     sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);

     while (local_err == NULL && current < sc_max) {
         GuestLogicalProcessor *vcpu;
-        GuestLogicalProcessorList *entry;
         int64_t id = current++;
         char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
                                      id);
@@ -2461,10 +2460,7 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
             vcpu->logical_id = id;
             vcpu->has_can_offline = true; /* lolspeak ftw */
             transfer_vcpu(vcpu, true, path, &local_err);
-            entry = g_malloc0(sizeof *entry);
-            entry->value = vcpu;
-            *link = entry;
-            link = &entry->next;
+            QAPI_LIST_APPEND(tail, vcpu);
         }
         g_free(path);
     }
@@ -2797,13 +2793,13 @@ out1:

 GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
 {
-    GuestMemoryBlockList *head, **link;
+    GuestMemoryBlockList *head, **tail;
     Error *local_err = NULL;
     struct dirent *de;
     DIR *dp;

     head = NULL;
-    link = &head;
+    tail = &head;

     dp = opendir("/sys/devices/system/memory/");
     if (!dp) {
@@ -2825,7 +2821,6 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
      */
     while ((de = readdir(dp)) != NULL) {
         GuestMemoryBlock *mem_blk;
-        GuestMemoryBlockList *entry;

         if ((strncmp(de->d_name, "memory", 6) != 0) ||
             !(de->d_type & DT_DIR)) {
@@ -2841,11 +2836,7 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
             break;
         }

-        entry = g_malloc0(sizeof *entry);
-        entry->value = mem_blk;
-
-        *link = entry;
-        link = &entry->next;
+        QAPI_LIST_APPEND(tail, mem_blk);
     }

     closedir(dp);
@@ -2865,15 +2856,14 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
 GuestMemoryBlockResponseList *
 qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
 {
-    GuestMemoryBlockResponseList *head, **link;
+    GuestMemoryBlockResponseList *head, **tail;
     Error *local_err = NULL;

     head = NULL;
-    link = &head;
+    tail = &head;

     while (mem_blks != NULL) {
         GuestMemoryBlockResponse *result;
-        GuestMemoryBlockResponseList *entry;
         GuestMemoryBlock *current_mem_blk = mem_blks->value;

         result = g_malloc0(sizeof(*result));
@@ -2882,11 +2872,8 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
         if (local_err) { /* should never happen */
             goto err;
         }
-        entry = g_malloc0(sizeof *entry);
-        entry->value = result;

-        *link = entry;
-        link = &entry->next;
+        QAPI_LIST_APPEND(tail, result);
         mem_blks = mem_blks->next;
     }

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 684639bd131e..a6cc481bc356 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1833,7 +1833,7 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
 {
     PSYSTEM_LOGICAL_PROCESSOR_INFORMATION pslpi, ptr;
     DWORD length;
-    GuestLogicalProcessorList *head, **link;
+    GuestLogicalProcessorList *head, **tail;
     Error *local_err = NULL;
     int64_t current;

@@ -1841,7 +1841,7 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
     length = 0;
     current = 0;
     head = NULL;
-    link = &head;
+    tail = &head;

     if ((GetLogicalProcessorInformation(pslpi, &length) == FALSE) &&
         (GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
@@ -1864,18 +1864,13 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
             while (cpu_bits > 0) {
                 if (!!(cpu_bits & 1)) {
                     GuestLogicalProcessor *vcpu;
-                    GuestLogicalProcessorList *entry;

                     vcpu = g_malloc0(sizeof *vcpu);
                     vcpu->logical_id = current++;
                     vcpu->online = true;
                     vcpu->has_can_offline = true;

-                    entry = g_malloc0(sizeof *entry);
-                    entry->value = vcpu;
-
-                    *link = entry;
-                    link = &entry->next;
+                    QAPI_LIST_APPEND(tail, vcpu);
                 }
                 cpu_bits >>= 1;
             }
diff --git a/scsi/pr-manager.c b/scsi/pr-manager.c
index 32b9287e680d..2098d7e759cc 100644
--- a/scsi/pr-manager.c
+++ b/scsi/pr-manager.c
@@ -116,8 +116,7 @@ pr_manager_register_types(void)

 static int query_one_pr_manager(Object *object, void *opaque)
 {
-    PRManagerInfoList ***prev = opaque;
-    PRManagerInfoList *elem;
+    PRManagerInfoList ***tail = opaque;
     PRManagerInfo *info;
     PRManager *pr_mgr;

@@ -126,15 +125,10 @@ static int query_one_pr_manager(Object *object, void *opaque)
         return 0;
     }

-    elem = g_new0(PRManagerInfoList, 1);
     info = g_new0(PRManagerInfo, 1);
     info->id = g_strdup(object_get_canonical_path_component(object));
     info->connected = pr_manager_is_connected(pr_mgr);
-    elem->value = info;
-    elem->next = NULL;
-
-    **prev = elem;
-    *prev = &elem->next;
+    QAPI_LIST_APPEND(*tail, info);
     return 0;
 }

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 35459a38bb1c..95326285b76d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4817,20 +4817,17 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose);

 /* Build a list with the name of all features on a feature word array */
 static void x86_cpu_list_feature_names(FeatureWordArray features,
-                                       strList **feat_names)
+                                       strList **tail)
 {
     FeatureWord w;
-    strList **next = feat_names;

     for (w = 0; w < FEATURE_WORDS; w++) {
         uint64_t filtered = features[w];
         int i;
         for (i = 0; i < 64; i++) {
             if (filtered & (1ULL << i)) {
-                strList *new = g_new0(strList, 1);
-                new->value = g_strdup(x86_cpu_feature_name(w, i));
-                *next = new;
-                next = &new->next;
+                QAPI_LIST_APPEND(tail,
+                                 g_strdup(x86_cpu_feature_name(w, i)));
             }
         }
     }
@@ -4851,16 +4848,13 @@ static void x86_cpu_get_unavailable_features(Object *obj, Visitor *v,
  * running using the current machine and accelerator.
  */
 static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
-                                                 strList **missing_feats)
+                                                 strList **tail)
 {
     X86CPU *xc;
     Error *err = NULL;
-    strList **next = missing_feats;

     if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
-        strList *new = g_new0(strList, 1);
-        new->value = g_strdup("kvm");
-        *missing_feats = new;
+        QAPI_LIST_APPEND(tail, g_strdup("kvm"));
         return;
     }

@@ -4872,16 +4866,13 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
          * but in case it does, just report the model as not
          * runnable at all using the "type" property.
          */
-        strList *new = g_new0(strList, 1);
-        new->value = g_strdup("type");
-        *next = new;
-        next = &new->next;
+        QAPI_LIST_APPEND(tail, g_strdup("type"));
         error_free(err);
     }

     x86_cpu_filter_features(xc, false);

-    x86_cpu_list_feature_names(xc->filtered_features, next);
+    x86_cpu_list_feature_names(xc->filtered_features, tail);

     object_unref(OBJECT(xc));
 }
diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index b20ab8b29b85..9dc1e075e7d4 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -442,122 +442,86 @@ static void init_list_union(UserDefListUnion *cvalue)
     int i;
     switch (cvalue->type) {
     case USER_DEF_LIST_UNION_KIND_INTEGER: {
-        intList **list = &cvalue->u.integer.data;
+        intList **tail = &cvalue->u.integer.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(intList, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_S8: {
-        int8List **list = &cvalue->u.s8.data;
+        int8List **tail = &cvalue->u.s8.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(int8List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_S16: {
-        int16List **list = &cvalue->u.s16.data;
+        int16List **tail = &cvalue->u.s16.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(int16List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_S32: {
-        int32List **list = &cvalue->u.s32.data;
+        int32List **tail = &cvalue->u.s32.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(int32List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_S64: {
-        int64List **list = &cvalue->u.s64.data;
+        int64List **tail = &cvalue->u.s64.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(int64List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_U8: {
-        uint8List **list = &cvalue->u.u8.data;
+        uint8List **tail = &cvalue->u.u8.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(uint8List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_U16: {
-        uint16List **list = &cvalue->u.u16.data;
+        uint16List **tail = &cvalue->u.u16.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(uint16List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_U32: {
-        uint32List **list = &cvalue->u.u32.data;
+        uint32List **tail = &cvalue->u.u32.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(uint32List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_U64: {
-        uint64List **list = &cvalue->u.u64.data;
+        uint64List **tail = &cvalue->u.u64.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(uint64List, 1);
-            (*list)->value = i;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, i);
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_BOOLEAN: {
-        boolList **list = &cvalue->u.boolean.data;
+        boolList **tail = &cvalue->u.boolean.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(boolList, 1);
-            (*list)->value = QEMU_IS_ALIGNED(i, 3);
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, QEMU_IS_ALIGNED(i, 3));
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_STRING: {
-        strList **list = &cvalue->u.string.data;
+        strList **tail = &cvalue->u.string.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(strList, 1);
-            (*list)->value = g_strdup_printf("%d", i);
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, g_strdup_printf("%d", i));
         }
         break;
     }
     case USER_DEF_LIST_UNION_KIND_NUMBER: {
-        numberList **list = &cvalue->u.number.data;
+        numberList **tail = &cvalue->u.number.data;
         for (i = 0; i < 32; i++) {
-            *list = g_new0(numberList, 1);
-            (*list)->value = (double)i / 3;
-            (*list)->next = NULL;
-            list = &(*list)->next;
+            QAPI_LIST_APPEND(tail, (double)i / 3);
         }
         break;
     }
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 9f6581439ade..6771495ca8ab 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -88,15 +88,13 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
 {
     int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
         3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
-    intList *list = NULL, **tmp = &list;
+    intList *list = NULL, **tail = &list;
     int i;
     Error *err = NULL;
     char *str;

     for (i = 0; i < ARRAY_SIZE(value); i++) {
-        *tmp = g_malloc0(sizeof(**tmp));
-        (*tmp)->value = value[i];
-        tmp = &(*tmp)->next;
+        QAPI_LIST_APPEND(tail, value[i]);
     }

     visit_type_intList(data->ov, NULL, &list, &err);
-- 
2.29.2



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

* [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND
  2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
                   ` (5 preceding siblings ...)
  2020-12-23 22:11 ` [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
@ 2020-12-23 22:11 ` Eric Blake
  2020-12-24 11:35   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 13:31   ` Markus Armbruster
  6 siblings, 2 replies; 21+ messages in thread
From: Eric Blake @ 2020-12-23 22:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
	open list:GLUSTER, Michael S. Tsirkin, Michael Roth, Jason Wang,
	Juan Quintela, armbru, Max Reitz, Gerd Hoffmann, Igor Mammedov,
	Marc-André Lureau, Dr. David Alan Gilbert

These cases require a bit more thought to review; in each case, the
code was appending to a list, but not with a FOOList **tail variable.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/gluster.c            | 13 ++----
 block/qapi.c               | 14 +-----
 dump/dump.c                | 22 +++------
 hw/core/machine-qmp-cmds.c | 93 ++++++++++++++++---------------------
 hw/mem/memory-device.c     | 12 +----
 hw/pci/pci.c               | 60 ++++++++----------------
 migration/migration.c      | 20 +++-----
 monitor/hmp-cmds.c         | 25 ++++------
 net/net.c                  | 13 +-----
 qga/commands-posix.c       | 94 ++++++++++++++------------------------
 qga/commands-win32.c       | 88 ++++++++++++-----------------------
 softmmu/tpm.c              | 38 +++------------
 ui/spice-core.c            | 27 ++++-------
 13 files changed, 170 insertions(+), 349 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 1f8699b93835..e8ee14c8e9bf 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -514,7 +514,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
 {
     QemuOpts *opts;
     SocketAddress *gsconf = NULL;
-    SocketAddressList *curr = NULL;
+    SocketAddressList **tail;
     QDict *backing_options = NULL;
     Error *local_err = NULL;
     char *str = NULL;
@@ -547,6 +547,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
     }
     gconf->path = g_strdup(ptr);
     qemu_opts_del(opts);
+    tail = &gconf->server;

     for (i = 0; i < num_servers; i++) {
         str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i);
@@ -655,15 +656,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
             qemu_opts_del(opts);
         }

-        if (gconf->server == NULL) {
-            gconf->server = g_new0(SocketAddressList, 1);
-            gconf->server->value = gsconf;
-            curr = gconf->server;
-        } else {
-            curr->next = g_new0(SocketAddressList, 1);
-            curr->next->value = gsconf;
-            curr = curr->next;
-        }
+        QAPI_LIST_APPEND(tail, gsconf);
         gsconf = NULL;

         qobject_unref(backing_options);
diff --git a/block/qapi.c b/block/qapi.c
index 3a1186fdccf5..0a96099e36e2 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -198,7 +198,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 {
     int i, sn_count;
     QEMUSnapshotInfo *sn_tab = NULL;
-    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
+    SnapshotInfoList *head = NULL, **tail = &head;
     SnapshotInfo *info;

     sn_count = bdrv_snapshot_list(bs, &sn_tab);
@@ -233,17 +233,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
         info->icount        = sn_tab[i].icount;
         info->has_icount    = sn_tab[i].icount != -1ULL;

-        info_list = g_new0(SnapshotInfoList, 1);
-        info_list->value = info;
-
-        /* XXX: waiting for the qapi to support qemu-queue.h types */
-        if (!cur_item) {
-            head = cur_item = info_list;
-        } else {
-            cur_item->next = info_list;
-            cur_item = info_list;
-        }
-
+        QAPI_LIST_APPEND(tail, info);
     }

     g_free(sn_tab);
diff --git a/dump/dump.c b/dump/dump.c
index dec32468d98c..929138e91d08 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -2030,39 +2030,29 @@ void qmp_dump_guest_memory(bool paging, const char *file,

 DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
 {
-    DumpGuestMemoryFormatList *item;
     DumpGuestMemoryCapability *cap =
                                   g_malloc0(sizeof(DumpGuestMemoryCapability));
+    DumpGuestMemoryFormatList **tail = &cap->formats;

     /* elf is always available */
-    item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
-    cap->formats = item;
-    item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
+    QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_ELF);

     /* kdump-zlib is always available */
-    item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
-    item = item->next;
-    item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
+    QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB);

     /* add new item if kdump-lzo is available */
 #ifdef CONFIG_LZO
-    item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
-    item = item->next;
-    item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
+    QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO);
 #endif

     /* add new item if kdump-snappy is available */
 #ifdef CONFIG_SNAPPY
-    item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
-    item = item->next;
-    item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
+    QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY);
 #endif

     /* Windows dump is available only if target is x86_64 */
 #ifdef TARGET_X86_64
-    item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
-    item = item->next;
-    item->value = DUMP_GUEST_MEMORY_FORMAT_WIN_DMP;
+    QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_WIN_DMP);
 #endif

     return cap;
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 156223a344ed..44e979e503b1 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -28,11 +28,11 @@ CpuInfoList *qmp_query_cpus(Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
     MachineClass *mc = MACHINE_GET_CLASS(ms);
-    CpuInfoList *head = NULL, *cur_item = NULL;
+    CpuInfoList *head = NULL, **tail = &head;
     CPUState *cpu;

     CPU_FOREACH(cpu) {
-        CpuInfoList *info;
+        CpuInfo *value;
 #if defined(TARGET_I386)
         X86CPU *x86_cpu = X86_CPU(cpu);
         CPUX86State *env = &x86_cpu->env;
@@ -58,53 +58,46 @@ CpuInfoList *qmp_query_cpus(Error **errp)

         cpu_synchronize_state(cpu);

-        info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->CPU = cpu->cpu_index;
-        info->value->current = (cpu == first_cpu);
-        info->value->halted = cpu->halted;
-        info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
-        info->value->thread_id = cpu->thread_id;
+        value = g_malloc0(sizeof(*value));
+        value->CPU = cpu->cpu_index;
+        value->current = (cpu == first_cpu);
+        value->halted = cpu->halted;
+        value->qom_path = object_get_canonical_path(OBJECT(cpu));
+        value->thread_id = cpu->thread_id;
 #if defined(TARGET_I386)
-        info->value->arch = CPU_INFO_ARCH_X86;
-        info->value->u.x86.pc = env->eip + env->segs[R_CS].base;
+        value->arch = CPU_INFO_ARCH_X86;
+        value->u.x86.pc = env->eip + env->segs[R_CS].base;
 #elif defined(TARGET_PPC)
-        info->value->arch = CPU_INFO_ARCH_PPC;
-        info->value->u.ppc.nip = env->nip;
+        value->arch = CPU_INFO_ARCH_PPC;
+        value->u.ppc.nip = env->nip;
 #elif defined(TARGET_SPARC)
-        info->value->arch = CPU_INFO_ARCH_SPARC;
-        info->value->u.q_sparc.pc = env->pc;
-        info->value->u.q_sparc.npc = env->npc;
+        value->arch = CPU_INFO_ARCH_SPARC;
+        value->u.q_sparc.pc = env->pc;
+        value->u.q_sparc.npc = env->npc;
 #elif defined(TARGET_MIPS)
-        info->value->arch = CPU_INFO_ARCH_MIPS;
-        info->value->u.q_mips.PC = env->active_tc.PC;
+        value->arch = CPU_INFO_ARCH_MIPS;
+        value->u.q_mips.PC = env->active_tc.PC;
 #elif defined(TARGET_TRICORE)
-        info->value->arch = CPU_INFO_ARCH_TRICORE;
-        info->value->u.tricore.PC = env->PC;
+        value->arch = CPU_INFO_ARCH_TRICORE;
+        value->u.tricore.PC = env->PC;
 #elif defined(TARGET_S390X)
-        info->value->arch = CPU_INFO_ARCH_S390;
-        info->value->u.s390.cpu_state = env->cpu_state;
+        value->arch = CPU_INFO_ARCH_S390;
+        value->u.s390.cpu_state = env->cpu_state;
 #elif defined(TARGET_RISCV)
-        info->value->arch = CPU_INFO_ARCH_RISCV;
-        info->value->u.riscv.pc = env->pc;
+        value->arch = CPU_INFO_ARCH_RISCV;
+        value->u.riscv.pc = env->pc;
 #else
-        info->value->arch = CPU_INFO_ARCH_OTHER;
+        value->arch = CPU_INFO_ARCH_OTHER;
 #endif
-        info->value->has_props = !!mc->cpu_index_to_instance_props;
-        if (info->value->has_props) {
+        value->has_props = !!mc->cpu_index_to_instance_props;
+        if (value->has_props) {
             CpuInstanceProperties *props;
             props = g_malloc0(sizeof(*props));
             *props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
-            info->value->props = props;
+            value->props = props;
         }

-        /* XXX: waiting for the qapi to support GSList */
-        if (!cur_item) {
-            head = cur_item = info;
-        } else {
-            cur_item->next = info;
-            cur_item = info;
-        }
+        QAPI_LIST_APPEND(tail, value);
     }

     return head;
@@ -170,39 +163,33 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
     MachineClass *mc = MACHINE_GET_CLASS(ms);
-    CpuInfoFastList *head = NULL, *cur_item = NULL;
+    CpuInfoFastList *head = NULL, **tail = &head;
     SysEmuTarget target = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME,
                                           -1, &error_abort);
     CPUState *cpu;

     CPU_FOREACH(cpu) {
-        CpuInfoFastList *info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
+        CpuInfoFast *value = g_malloc0(sizeof(*value));

-        info->value->cpu_index = cpu->cpu_index;
-        info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
-        info->value->thread_id = cpu->thread_id;
+        value->cpu_index = cpu->cpu_index;
+        value->qom_path = object_get_canonical_path(OBJECT(cpu));
+        value->thread_id = cpu->thread_id;

-        info->value->has_props = !!mc->cpu_index_to_instance_props;
-        if (info->value->has_props) {
+        value->has_props = !!mc->cpu_index_to_instance_props;
+        if (value->has_props) {
             CpuInstanceProperties *props;
             props = g_malloc0(sizeof(*props));
             *props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
-            info->value->props = props;
+            value->props = props;
         }

-        info->value->arch = sysemu_target_to_cpuinfo_arch(target);
-        info->value->target = target;
+        value->arch = sysemu_target_to_cpuinfo_arch(target);
+        value->target = target;
         if (target == SYS_EMU_TARGET_S390X) {
-            cpustate_to_cpuinfo_s390(&info->value->u.s390x, cpu);
+            cpustate_to_cpuinfo_s390(&value->u.s390x, cpu);
         }

-        if (!cur_item) {
-            head = cur_item = info;
-        } else {
-            cur_item->next = info;
-            cur_item = info;
-        }
+        QAPI_LIST_APPEND(tail, value);
     }

     return head;
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index cf0627fd01c1..d9f8301711e2 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -199,7 +199,7 @@ out:
 MemoryDeviceInfoList *qmp_memory_device_list(void)
 {
     GSList *devices = NULL, *item;
-    MemoryDeviceInfoList *list = NULL, *prev = NULL;
+    MemoryDeviceInfoList *list = NULL, **tail = &list;

     object_child_foreach(qdev_get_machine(), memory_device_build_list,
                          &devices);
@@ -207,19 +207,11 @@ MemoryDeviceInfoList *qmp_memory_device_list(void)
     for (item = devices; item; item = g_slist_next(item)) {
         const MemoryDeviceState *md = MEMORY_DEVICE(item->data);
         const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(item->data);
-        MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
         MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);

         mdc->fill_device_info(md, info);

-        elem->value = info;
-        elem->next = NULL;
-        if (prev) {
-            prev->next = elem;
-        } else {
-            list = elem;
-        }
-        prev = elem;
+        QAPI_LIST_APPEND(tail, info);
     }

     g_slist_free(devices);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d4349ea57765..0d99fe03b98b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1682,41 +1682,34 @@ static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num);

 static PciMemoryRegionList *qmp_query_pci_regions(const PCIDevice *dev)
 {
-    PciMemoryRegionList *head = NULL, *cur_item = NULL;
+    PciMemoryRegionList *head = NULL, **tail = &head;
     int i;

     for (i = 0; i < PCI_NUM_REGIONS; i++) {
         const PCIIORegion *r = &dev->io_regions[i];
-        PciMemoryRegionList *region;
+        PciMemoryRegion *region;

         if (!r->size) {
             continue;
         }

         region = g_malloc0(sizeof(*region));
-        region->value = g_malloc0(sizeof(*region->value));

         if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
-            region->value->type = g_strdup("io");
+            region->type = g_strdup("io");
         } else {
-            region->value->type = g_strdup("memory");
-            region->value->has_prefetch = true;
-            region->value->prefetch = !!(r->type & PCI_BASE_ADDRESS_MEM_PREFETCH);
-            region->value->has_mem_type_64 = true;
-            region->value->mem_type_64 = !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
+            region->type = g_strdup("memory");
+            region->has_prefetch = true;
+            region->prefetch = !!(r->type & PCI_BASE_ADDRESS_MEM_PREFETCH);
+            region->has_mem_type_64 = true;
+            region->mem_type_64 = !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
         }

-        region->value->bar = i;
-        region->value->address = r->addr;
-        region->value->size = r->size;
+        region->bar = i;
+        region->address = r->addr;
+        region->size = r->size;

-        /* XXX: waiting for the qapi to support GSList */
-        if (!cur_item) {
-            head = cur_item = region;
-        } else {
-            cur_item->next = region;
-            cur_item = region;
-        }
+        QAPI_LIST_APPEND(tail, region);
     }

     return head;
@@ -1813,23 +1806,14 @@ static PciDeviceInfo *qmp_query_pci_device(PCIDevice *dev, PCIBus *bus,

 static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num)
 {
-    PciDeviceInfoList *info, *head = NULL, *cur_item = NULL;
+    PciDeviceInfoList *head = NULL, **tail = &head;
     PCIDevice *dev;
     int devfn;

     for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
         dev = bus->devices[devfn];
         if (dev) {
-            info = g_malloc0(sizeof(*info));
-            info->value = qmp_query_pci_device(dev, bus, bus_num);
-
-            /* XXX: waiting for the qapi to support GSList */
-            if (!cur_item) {
-                head = cur_item = info;
-            } else {
-                cur_item->next = info;
-                cur_item = info;
-            }
+            QAPI_LIST_APPEND(tail, qmp_query_pci_device(dev, bus, bus_num));
         }
     }

@@ -1852,21 +1836,13 @@ static PciInfo *qmp_query_pci_bus(PCIBus *bus, int bus_num)

 PciInfoList *qmp_query_pci(Error **errp)
 {
-    PciInfoList *info, *head = NULL, *cur_item = NULL;
+    PciInfoList *head = NULL, **tail = &head;
     PCIHostState *host_bridge;

     QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
-        info = g_malloc0(sizeof(*info));
-        info->value = qmp_query_pci_bus(host_bridge->bus,
-                                        pci_bus_num(host_bridge->bus));
-
-        /* XXX: waiting for the qapi to support GSList */
-        if (!cur_item) {
-            head = cur_item = info;
-        } else {
-            cur_item->next = info;
-            cur_item = info;
-        }
+        QAPI_LIST_APPEND(tail,
+                         qmp_query_pci_bus(host_bridge->bus,
+                                           pci_bus_num(host_bridge->bus)));
     }

     return head;
diff --git a/migration/migration.c b/migration/migration.c
index 805712488e4d..a676405019d1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -784,29 +784,21 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value)

 MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
 {
-    MigrationCapabilityStatusList *head = NULL;
-    MigrationCapabilityStatusList *caps;
+    MigrationCapabilityStatusList *head = NULL, **tail = &head;
+    MigrationCapabilityStatus *caps;
     MigrationState *s = migrate_get_current();
     int i;

-    caps = NULL; /* silence compiler warning */
     for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
 #ifndef CONFIG_LIVE_BLOCK_MIGRATION
         if (i == MIGRATION_CAPABILITY_BLOCK) {
             continue;
         }
 #endif
-        if (head == NULL) {
-            head = g_malloc0(sizeof(*caps));
-            caps = head;
-        } else {
-            caps->next = g_malloc0(sizeof(*caps));
-            caps = caps->next;
-        }
-        caps->value =
-            g_malloc(sizeof(*caps->value));
-        caps->value->capability = i;
-        caps->value->state = s->enabled_capabilities[i];
+        caps = g_malloc(sizeof(*caps));
+        caps->capability = i;
+        caps->state = s->enabled_capabilities[i];
+        QAPI_LIST_APPEND(tail, caps);
     }

     return head;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ed4131efbca6..a9643ff41961 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1705,7 +1705,8 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
 void hmp_sendkey(Monitor *mon, const QDict *qdict)
 {
     const char *keys = qdict_get_str(qdict, "keys");
-    KeyValueList *keylist, *head = NULL, *tmp = NULL;
+    KeyValue *v = NULL;
+    KeyValueList *head = NULL, **tail = &head;
     int has_hold_time = qdict_haskey(qdict, "hold-time");
     int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
     Error *err = NULL;
@@ -1722,16 +1723,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
             keyname_len = 4;
         }

-        keylist = g_malloc0(sizeof(*keylist));
-        keylist->value = g_malloc0(sizeof(*keylist->value));
-
-        if (!head) {
-            head = keylist;
-        }
-        if (tmp) {
-            tmp->next = keylist;
-        }
-        tmp = keylist;
+        v = g_malloc0(sizeof(*v));

         if (strstart(keys, "0x", NULL)) {
             char *endp;
@@ -1740,16 +1732,18 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
             if (endp != keys + keyname_len) {
                 goto err_out;
             }
-            keylist->value->type = KEY_VALUE_KIND_NUMBER;
-            keylist->value->u.number.data = value;
+            v->type = KEY_VALUE_KIND_NUMBER;
+            v->u.number.data = value;
         } else {
             int idx = index_from_key(keys, keyname_len);
             if (idx == Q_KEY_CODE__MAX) {
                 goto err_out;
             }
-            keylist->value->type = KEY_VALUE_KIND_QCODE;
-            keylist->value->u.qcode.data = idx;
+            v->type = KEY_VALUE_KIND_QCODE;
+            v->u.qcode.data = idx;
         }
+        QAPI_LIST_APPEND(tail, v);
+        v = NULL;

         if (!*separator) {
             break;
@@ -1761,6 +1755,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);

 out:
+    qapi_free_KeyValue(v);
     qapi_free_KeyValueList(head);
     return;

diff --git a/net/net.c b/net/net.c
index e581c8a26868..c7e8f4bc403c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1197,10 +1197,9 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
                                       Error **errp)
 {
     NetClientState *nc;
-    RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
+    RxFilterInfoList *filter_list = NULL, **tail = &filter_list;

     QTAILQ_FOREACH(nc, &net_clients, next) {
-        RxFilterInfoList *entry;
         RxFilterInfo *info;

         if (has_name && strcmp(nc->name, name) != 0) {
@@ -1225,15 +1224,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,

         if (nc->info->query_rx_filter) {
             info = nc->info->query_rx_filter(nc);
-            entry = g_malloc0(sizeof(*entry));
-            entry->value = info;
-
-            if (!filter_list) {
-                filter_list = entry;
-            } else {
-                last_entry->next = entry;
-            }
-            last_entry = entry;
+            QAPI_LIST_APPEND(tail, info);
         } else if (has_name) {
             error_setg(errp, "net client(%s) doesn't support"
                        " rx-filter querying", name);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index a5058a3bd15e..10ee740eee1b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2119,17 +2119,17 @@ void qmp_guest_suspend_hybrid(Error **errp)
     guest_suspend(SUSPEND_MODE_HYBRID, errp);
 }

-static GuestNetworkInterfaceList *
+static GuestNetworkInterface *
 guest_find_interface(GuestNetworkInterfaceList *head,
                      const char *name)
 {
     for (; head; head = head->next) {
         if (strcmp(head->value->name, name) == 0) {
-            break;
+            return head->value;
         }
     }

-    return head;
+    return NULL;
 }

 static int guest_get_network_stats(const char *name,
@@ -2198,7 +2198,7 @@ static int guest_get_network_stats(const char *name,
  */
 GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
 {
-    GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
+    GuestNetworkInterfaceList *head = NULL, **tail = &head;
     struct ifaddrs *ifap, *ifa;

     if (getifaddrs(&ifap) < 0) {
@@ -2207,9 +2207,10 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
     }

     for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
-        GuestNetworkInterfaceList *info;
-        GuestIpAddressList **address_list = NULL, *address_item = NULL;
-        GuestNetworkInterfaceStat  *interface_stat = NULL;
+        GuestNetworkInterface *info;
+        GuestIpAddressList **address_tail;
+        GuestIpAddress *address_item = NULL;
+        GuestNetworkInterfaceStat *interface_stat = NULL;
         char addr4[INET_ADDRSTRLEN];
         char addr6[INET6_ADDRSTRLEN];
         int sock;
@@ -2223,19 +2224,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)

         if (!info) {
             info = g_malloc0(sizeof(*info));
-            info->value = g_malloc0(sizeof(*info->value));
-            info->value->name = g_strdup(ifa->ifa_name);
+            info->name = g_strdup(ifa->ifa_name);

-            if (!cur_item) {
-                head = cur_item = info;
-            } else {
-                cur_item->next = info;
-                cur_item = info;
-            }
+            QAPI_LIST_APPEND(tail, info);
         }

-        if (!info->value->has_hardware_address &&
-            ifa->ifa_flags & SIOCGIFHWADDR) {
+        address_tail = &info->ip_addresses;
+
+        if (!info->has_hardware_address && ifa->ifa_flags & SIOCGIFHWADDR) {
             /* we haven't obtained HW address yet */
             sock = socket(PF_INET, SOCK_STREAM, 0);
             if (sock == -1) {
@@ -2244,7 +2240,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             }

             memset(&ifr, 0, sizeof(ifr));
-            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->value->name);
+            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
             if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
                 error_setg_errno(errp, errno,
                                  "failed to get MAC address of %s",
@@ -2256,13 +2252,13 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             close(sock);
             mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;

-            info->value->hardware_address =
+            info->hardware_address =
                 g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
                                 (int) mac_addr[0], (int) mac_addr[1],
                                 (int) mac_addr[2], (int) mac_addr[3],
                                 (int) mac_addr[4], (int) mac_addr[5]);

-            info->value->has_hardware_address = true;
+            info->has_hardware_address = true;
         }

         if (ifa->ifa_addr &&
@@ -2275,15 +2271,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             }

             address_item = g_malloc0(sizeof(*address_item));
-            address_item->value = g_malloc0(sizeof(*address_item->value));
-            address_item->value->ip_address = g_strdup(addr4);
-            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
+            address_item->ip_address = g_strdup(addr4);
+            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;

             if (ifa->ifa_netmask) {
                 /* Count the number of set bits in netmask.
                  * This is safe as '1' and '0' cannot be shuffled in netmask. */
                 p = &((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr;
-                address_item->value->prefix = ctpop32(((uint32_t *) p)[0]);
+                address_item->prefix = ctpop32(((uint32_t *) p)[0]);
             }
         } else if (ifa->ifa_addr &&
                    ifa->ifa_addr->sa_family == AF_INET6) {
@@ -2295,15 +2290,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             }

             address_item = g_malloc0(sizeof(*address_item));
-            address_item->value = g_malloc0(sizeof(*address_item->value));
-            address_item->value->ip_address = g_strdup(addr6);
-            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
+            address_item->ip_address = g_strdup(addr6);
+            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;

             if (ifa->ifa_netmask) {
                 /* Count the number of set bits in netmask.
                  * This is safe as '1' and '0' cannot be shuffled in netmask. */
                 p = &((struct sockaddr_in6 *)ifa->ifa_netmask)->sin6_addr;
-                address_item->value->prefix =
+                address_item->prefix =
                     ctpop32(((uint32_t *) p)[0]) +
                     ctpop32(((uint32_t *) p)[1]) +
                     ctpop32(((uint32_t *) p)[2]) +
@@ -2315,29 +2309,18 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             continue;
         }

-        address_list = &info->value->ip_addresses;
+        QAPI_LIST_APPEND(address_tail, address_item);

-        while (*address_list && (*address_list)->next) {
-            address_list = &(*address_list)->next;
-        }
+        info->has_ip_addresses = true;

-        if (!*address_list) {
-            *address_list = address_item;
-        } else {
-            (*address_list)->next = address_item;
-        }
-
-        info->value->has_ip_addresses = true;
-
-        if (!info->value->has_statistics) {
+        if (!info->has_statistics) {
             interface_stat = g_malloc0(sizeof(*interface_stat));
-            if (guest_get_network_stats(info->value->name,
-                interface_stat) == -1) {
-                info->value->has_statistics = false;
+            if (guest_get_network_stats(info->name, interface_stat) == -1) {
+                info->has_statistics = false;
                 g_free(interface_stat);
             } else {
-                info->value->statistics = interface_stat;
-                info->value->has_statistics = true;
+                info->statistics = interface_stat;
+                info->has_statistics = true;
             }
         }
     }
@@ -3104,11 +3087,10 @@ static double ga_get_login_time(struct utmpx *user_info)
 GuestUserList *qmp_guest_get_users(Error **errp)
 {
     GHashTable *cache = NULL;
-    GuestUserList *head = NULL, *cur_item = NULL;
+    GuestUserList *head = NULL, **tail = &head;
     struct utmpx *user_info = NULL;
     gpointer value = NULL;
     GuestUser *user = NULL;
-    GuestUserList *item = NULL;
     double login_time = 0;

     cache = g_hash_table_new(g_str_hash, g_str_equal);
@@ -3131,19 +3113,13 @@ GuestUserList *qmp_guest_get_users(Error **errp)
             continue;
         }

-        item = g_new0(GuestUserList, 1);
-        item->value = g_new0(GuestUser, 1);
-        item->value->user = g_strdup(user_info->ut_user);
-        item->value->login_time = ga_get_login_time(user_info);
+        user = g_new0(GuestUser, 1);
+        user->user = g_strdup(user_info->ut_user);
+        user->login_time = ga_get_login_time(user_info);

-        g_hash_table_insert(cache, item->value->user, item->value);
+        g_hash_table_insert(cache, user->user, user);

-        if (!cur_item) {
-            head = cur_item = item;
-        } else {
-            cur_item->next = item;
-            cur_item = item;
-        }
+        QAPI_LIST_APPEND(tail, user);
     }
     endutxent();
     g_hash_table_destroy(cache);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index a6cc481bc356..a00e6cb16557 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1624,11 +1624,11 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
 {
     IP_ADAPTER_ADDRESSES *adptr_addrs, *addr;
     IP_ADAPTER_UNICAST_ADDRESS *ip_addr = NULL;
-    GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
-    GuestIpAddressList *head_addr, *cur_addr;
-    GuestNetworkInterfaceList *info;
+    GuestNetworkInterfaceList *head = NULL, **tail = &head;
+    GuestIpAddressList *head_addr, **tail_addr;
+    GuestNetworkInterface *info;
     GuestNetworkInterfaceStat *interface_stat = NULL;
-    GuestIpAddressList *address_item = NULL;
+    GuestIpAddress *address_item = NULL;
     unsigned char *mac_addr;
     char *addr_str;
     WORD wsa_version;
@@ -1651,30 +1651,24 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
     for (addr = adptr_addrs; addr; addr = addr->Next) {
         info = g_malloc0(sizeof(*info));

-        if (cur_item == NULL) {
-            head = cur_item = info;
-        } else {
-            cur_item->next = info;
-            cur_item = info;
-        }
+        QAPI_LIST_APPEND(tail, info);

-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->name = guest_wctomb_dup(addr->FriendlyName);
+        info->name = guest_wctomb_dup(addr->FriendlyName);

         if (addr->PhysicalAddressLength != 0) {
             mac_addr = addr->PhysicalAddress;

-            info->value->hardware_address =
+            info->hardware_address =
                 g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
                                 (int) mac_addr[0], (int) mac_addr[1],
                                 (int) mac_addr[2], (int) mac_addr[3],
                                 (int) mac_addr[4], (int) mac_addr[5]);

-            info->value->has_hardware_address = true;
+            info->has_hardware_address = true;
         }

         head_addr = NULL;
-        cur_addr = NULL;
+        tail_addr = &head_addr;
         for (ip_addr = addr->FirstUnicastAddress;
                 ip_addr;
                 ip_addr = ip_addr->Next) {
@@ -1685,37 +1679,29 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)

             address_item = g_malloc0(sizeof(*address_item));

-            if (!cur_addr) {
-                head_addr = cur_addr = address_item;
-            } else {
-                cur_addr->next = address_item;
-                cur_addr = address_item;
-            }
+            QAPI_LIST_APPEND(tail_addr, address_item);

-            address_item->value = g_malloc0(sizeof(*address_item->value));
-            address_item->value->ip_address = addr_str;
-            address_item->value->prefix = guest_ip_prefix(ip_addr);
+            address_item->ip_address = addr_str;
+            address_item->prefix = guest_ip_prefix(ip_addr);
             if (ip_addr->Address.lpSockaddr->sa_family == AF_INET) {
-                address_item->value->ip_address_type =
-                    GUEST_IP_ADDRESS_TYPE_IPV4;
+                address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
             } else if (ip_addr->Address.lpSockaddr->sa_family == AF_INET6) {
-                address_item->value->ip_address_type =
-                    GUEST_IP_ADDRESS_TYPE_IPV6;
+                address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
             }
         }
         if (head_addr) {
-            info->value->has_ip_addresses = true;
-            info->value->ip_addresses = head_addr;
+            info->has_ip_addresses = true;
+            info->ip_addresses = head_addr;
         }
-        if (!info->value->has_statistics) {
+        if (!info->has_statistics) {
             interface_stat = g_malloc0(sizeof(*interface_stat));
             if (guest_get_network_stats(addr->AdapterName,
                 interface_stat) == -1) {
-                info->value->has_statistics = false;
+                info->has_statistics = false;
                 g_free(interface_stat);
             } else {
-                info->value->statistics = interface_stat;
-                info->value->has_statistics = true;
+                info->statistics = interface_stat;
+                info->has_statistics = true;
             }
         }
     }
@@ -2082,12 +2068,11 @@ GuestUserList *qmp_guest_get_users(Error **errp)
 #define QGA_NANOSECONDS 10000000

     GHashTable *cache = NULL;
-    GuestUserList *head = NULL, *cur_item = NULL;
+    GuestUserList *head = NULL, **tail = &head;

     DWORD buffer_size = 0, count = 0, i = 0;
     GA_WTSINFOA *info = NULL;
     WTS_SESSION_INFOA *entries = NULL;
-    GuestUserList *item = NULL;
     GuestUser *user = NULL;
     gpointer value = NULL;
     INT64 login = 0;
@@ -2123,23 +2108,17 @@ GuestUserList *qmp_guest_get_users(Error **errp)
                         user->login_time = login_time;
                     }
                 } else {
-                    item = g_new0(GuestUserList, 1);
-                    item->value = g_new0(GuestUser, 1);
+                    user = g_new0(GuestUser, 1);

-                    item->value->user = g_strdup(info->UserName);
-                    item->value->domain = g_strdup(info->Domain);
-                    item->value->has_domain = true;
+                    user->user = g_strdup(info->UserName);
+                    user->domain = g_strdup(info->Domain);
+                    user->has_domain = true;

-                    item->value->login_time = login_time;
+                    user->login_time = login_time;

-                    g_hash_table_add(cache, item->value->user);
+                    g_hash_table_add(cache, user->user);

-                    if (!cur_item) {
-                        head = cur_item = item;
-                    } else {
-                        cur_item->next = item;
-                        cur_item = item;
-                    }
+                    QAPI_LIST_APPEND(tail, user);
                 }
             }
             WTSFreeMemory(info);
@@ -2424,7 +2403,7 @@ static GStrv ga_get_hardware_ids(DEVINST devInstance)

 GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
 {
-    GuestDeviceInfoList *head = NULL, *cur_item = NULL, *item = NULL;
+    GuestDeviceInfoList *head = NULL, **tail = &head;
     HDEVINFO dev_info = INVALID_HANDLE_VALUE;
     SP_DEVINFO_DATA dev_info_data;
     int i, j;
@@ -2522,14 +2501,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
         slog("driver: %s\ndriver version: %" PRId64 ",%s\n",
              device->driver_name, device->driver_date,
              device->driver_version);
-        item = g_new0(GuestDeviceInfoList, 1);
-        item->value = g_steal_pointer(&device);
-        if (!cur_item) {
-            head = cur_item = item;
-        } else {
-            cur_item->next = item;
-            cur_item = item;
-        }
+        QAPI_LIST_APPEND(tail, g_steal_pointer(&device));
     }

     if (dev_info != INVALID_HANDLE_VALUE) {
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index cab206355afd..578563f05a7b 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -196,22 +196,14 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
 TPMInfoList *qmp_query_tpm(Error **errp)
 {
     TPMBackend *drv;
-    TPMInfoList *info, *head = NULL, *cur_item = NULL;
+    TPMInfoList *head = NULL, **tail = &head;

     QLIST_FOREACH(drv, &tpm_backends, list) {
         if (!drv->tpmif) {
             continue;
         }

-        info = g_new0(TPMInfoList, 1);
-        info->value = tpm_backend_query_tpm(drv);
-
-        if (!cur_item) {
-            head = cur_item = info;
-        } else {
-            cur_item->next = info;
-            cur_item = info;
-        }
+        QAPI_LIST_APPEND(tail, tpm_backend_query_tpm(drv));
     }

     return head;
@@ -220,44 +212,26 @@ TPMInfoList *qmp_query_tpm(Error **errp)
 TpmTypeList *qmp_query_tpm_types(Error **errp)
 {
     unsigned int i = 0;
-    TpmTypeList *head = NULL, *prev = NULL, *cur_item;
+    TpmTypeList *head = NULL, **tail = &head;

     for (i = 0; i < TPM_TYPE__MAX; i++) {
         if (!tpm_be_find_by_type(i)) {
             continue;
         }
-        cur_item = g_new0(TpmTypeList, 1);
-        cur_item->value = i;
-
-        if (prev) {
-            prev->next = cur_item;
-        }
-        if (!head) {
-            head = cur_item;
-        }
-        prev = cur_item;
+        QAPI_LIST_APPEND(tail, i);
     }

     return head;
 }
 TpmModelList *qmp_query_tpm_models(Error **errp)
 {
-    TpmModelList *head = NULL, *prev = NULL, *cur_item;
+    TpmModelList *head = NULL, **tail = &head;
     GSList *e, *l = object_class_get_list(TYPE_TPM_IF, false);

     for (e = l; e; e = e->next) {
         TPMIfClass *c = TPM_IF_CLASS(e->data);

-        cur_item = g_new0(TpmModelList, 1);
-        cur_item->value = c->model;
-
-        if (prev) {
-            prev->next = cur_item;
-        }
-        if (!head) {
-            head = cur_item;
-        }
-        prev = cur_item;
+        QAPI_LIST_APPEND(tail, c->model);
     }
     g_slist_free(l);

diff --git a/ui/spice-core.c b/ui/spice-core.c
index eea52f538999..58232b649e60 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -355,11 +355,11 @@ static const char *wan_compression_names[] = {

 static SpiceChannelList *qmp_query_spice_channels(void)
 {
-    SpiceChannelList *cur_item = NULL, *head = NULL;
+    SpiceChannelList *head = NULL, **tail = &head;
     ChannelList *item;

     QTAILQ_FOREACH(item, &channel_list, link) {
-        SpiceChannelList *chan;
+        SpiceChannel *chan;
         char host[NI_MAXHOST], port[NI_MAXSERV];
         struct sockaddr *paddr;
         socklen_t plen;
@@ -367,29 +367,22 @@ static SpiceChannelList *qmp_query_spice_channels(void)
         assert(item->info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT);

         chan = g_malloc0(sizeof(*chan));
-        chan->value = g_malloc0(sizeof(*chan->value));

         paddr = (struct sockaddr *)&item->info->paddr_ext;
         plen = item->info->plen_ext;
         getnameinfo(paddr, plen,
                     host, sizeof(host), port, sizeof(port),
                     NI_NUMERICHOST | NI_NUMERICSERV);
-        chan->value->host = g_strdup(host);
-        chan->value->port = g_strdup(port);
-        chan->value->family = inet_netfamily(paddr->sa_family);
+        chan->host = g_strdup(host);
+        chan->port = g_strdup(port);
+        chan->family = inet_netfamily(paddr->sa_family);

-        chan->value->connection_id = item->info->connection_id;
-        chan->value->channel_type = item->info->type;
-        chan->value->channel_id = item->info->id;
-        chan->value->tls = item->info->flags & SPICE_CHANNEL_EVENT_FLAG_TLS;
+        chan->connection_id = item->info->connection_id;
+        chan->channel_type = item->info->type;
+        chan->channel_id = item->info->id;
+        chan->tls = item->info->flags & SPICE_CHANNEL_EVENT_FLAG_TLS;

-       /* XXX: waiting for the qapi to support GSList */
-        if (!cur_item) {
-            head = cur_item = chan;
-        } else {
-            cur_item->next = chan;
-            cur_item = chan;
-        }
+        QAPI_LIST_APPEND(tail, chan);
     }

     return head;
-- 
2.29.2



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

* Re: [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND
  2020-12-23 22:11 ` [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
@ 2020-12-24  6:14   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 13:04   ` Markus Armbruster
  1 sibling, 0 replies; 21+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-12-24  6:14 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: Michael Roth, armbru

24.12.2020 01:11, Eric Blake wrote:
> Similar to the existing QAPI_LIST_PREPEND, but designed for use where
> we want to preserve insertion order.  Callers will be added in
> upcoming patches.  Note the difference in signature: PREPEND takes
> List*, APPEND takes List**.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   include/qapi/util.h | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 6178e98e97a5..8b4967990c0d 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -37,4 +37,17 @@ int parse_qapi_name(const char *name, bool complete);
>       (list) = _tmp; \
>   } while (0)
> 
> +/*
> + * For any pointer to a GenericList @tail, insert @element at the back and
> + * update the tail.

Would be more obvious for me, if directly mention that tail is a pointer to 'next' field in the last element of the list.

> + *
> + * Note that this macro evaluates @element exactly once, so it is safe
> + * to have side-effects with that argument.
> + */
> +#define QAPI_LIST_APPEND(tail, element) do { \
> +    *(tail) = g_malloc0(sizeof(**(tail))); \
> +    (*(tail))->value = (element); \
> +    (tail) = &(*tail)->next; \
> +} while (0)
> +
>   #endif
> 

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

-- 
Best regards,
Vladimir


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

* Re: [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
  2020-12-23 22:11 ` [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
@ 2020-12-24  9:56   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 14:10     ` Eric Blake
  2021-01-13 13:16   ` Markus Armbruster
  1 sibling, 1 reply; 21+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-12-24  9:56 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, open list:Dirty Bitmaps, Michael S. Tsirkin,
	open list:Trivial patches, Michael Roth, Richard Henderson,
	armbru, Max Reitz, Michael Tokarev, Paolo Bonzini, Igor Mammedov,
	John Snow, Dr. David Alan Gilbert, Laurent Vivier

24.12.2020 01:11, Eric Blake wrote:
> The easiest spots to use QAPI_LIST_APPEND are where we already have an
> obvious pointer to the tail of a list.  While at it, consistently use
> the variable name 'tail' for that purpose.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---

[..]

> --- a/monitor/qmp-cmds-control.c
> +++ b/monitor/qmp-cmds-control.c
> @@ -104,17 +104,16 @@ VersionInfo *qmp_query_version(Error **errp)
> 
>   static void query_commands_cb(const QmpCommand *cmd, void *opaque)
>   {
> -    CommandInfoList *info, **list = opaque;
> +    CommandInfo *info;
> +    CommandInfoList **tail = opaque;
> 
>       if (!cmd->enabled) {
>           return;
>       }
> 
>       info = g_malloc0(sizeof(*info));
> -    info->value = g_malloc0(sizeof(*info->value));
> -    info->value->name = g_strdup(cmd->name);
> -    info->next = *list;
> -    *list = info;
> +    info->name = g_strdup(cmd->name);
> +    QAPI_LIST_APPEND(tail, info);

Original logic is prepend in this hunk.

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

>   }
> 
>   CommandInfoList *qmp_query_commands(Error **errp)

[..]

> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4817,20 +4817,17 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose);
> 
>   /* Build a list with the name of all features on a feature word array */
>   static void x86_cpu_list_feature_names(FeatureWordArray features,
> -                                       strList **feat_names)
> +                                       strList **tail)
>   {
>       FeatureWord w;
> -    strList **next = feat_names;
> 
>       for (w = 0; w < FEATURE_WORDS; w++) {
>           uint64_t filtered = features[w];
>           int i;
>           for (i = 0; i < 64; i++) {
>               if (filtered & (1ULL << i)) {
> -                strList *new = g_new0(strList, 1);
> -                new->value = g_strdup(x86_cpu_feature_name(w, i));
> -                *next = new;
> -                next = &new->next;
> +                QAPI_LIST_APPEND(tail,
> +                                 g_strdup(x86_cpu_feature_name(w, i)));

actually, fit in one line...

>               }
>           }
>       }

[..]

-- 
Best regards,
Vladimir


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

* Re: [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND
  2020-12-23 22:11 ` [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
@ 2020-12-24 11:35   ` Vladimir Sementsov-Ogievskiy
  2021-01-13 13:31   ` Markus Armbruster
  1 sibling, 0 replies; 21+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-12-24 11:35 UTC (permalink / raw)
  To: Eric Blake, qemu-devel
  Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
	open list:GLUSTER, Michael S. Tsirkin, Michael Roth, Jason Wang,
	Juan Quintela, armbru, Max Reitz, Gerd Hoffmann,
	Marc-André Lureau, Igor Mammedov, Dr. David Alan Gilbert

24.12.2020 01:11, Eric Blake wrote:
> These cases require a bit more thought to review; in each case, the
> code was appending to a list, but not with a FOOList **tail variable.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---

[..]

> diff --git a/migration/migration.c b/migration/migration.c
> index 805712488e4d..a676405019d1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -784,29 +784,21 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value)
> 
>   MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
>   {
> -    MigrationCapabilityStatusList *head = NULL;
> -    MigrationCapabilityStatusList *caps;
> +    MigrationCapabilityStatusList *head = NULL, **tail = &head;
> +    MigrationCapabilityStatus *caps;
>       MigrationState *s = migrate_get_current();
>       int i;
> 
> -    caps = NULL; /* silence compiler warning */
>       for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
>   #ifndef CONFIG_LIVE_BLOCK_MIGRATION
>           if (i == MIGRATION_CAPABILITY_BLOCK) {
>               continue;
>           }
>   #endif
> -        if (head == NULL) {
> -            head = g_malloc0(sizeof(*caps));
> -            caps = head;
> -        } else {
> -            caps->next = g_malloc0(sizeof(*caps));
> -            caps = caps->next;
> -        }
> -        caps->value =
> -            g_malloc(sizeof(*caps->value));
> -        caps->value->capability = i;
> -        caps->value->state = s->enabled_capabilities[i];
> +        caps = g_malloc(sizeof(*caps));

While being here, probably better use g_malloc0, for safety in future

> +        caps->capability = i;
> +        caps->state = s->enabled_capabilities[i];
> +        QAPI_LIST_APPEND(tail, caps);
>       }
> 
>       return head;
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ed4131efbca6..a9643ff41961 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1705,7 +1705,8 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
>   void hmp_sendkey(Monitor *mon, const QDict *qdict)
>   {
>       const char *keys = qdict_get_str(qdict, "keys");
> -    KeyValueList *keylist, *head = NULL, *tmp = NULL;
> +    KeyValue *v = NULL;
> +    KeyValueList *head = NULL, **tail = &head;
>       int has_hold_time = qdict_haskey(qdict, "hold-time");
>       int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
>       Error *err = NULL;
> @@ -1722,16 +1723,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>               keyname_len = 4;
>           }
> 
> -        keylist = g_malloc0(sizeof(*keylist));
> -        keylist->value = g_malloc0(sizeof(*keylist->value));
> -
> -        if (!head) {
> -            head = keylist;
> -        }
> -        if (tmp) {
> -            tmp->next = keylist;
> -        }
> -        tmp = keylist;
> +        v = g_malloc0(sizeof(*v));
> 
>           if (strstart(keys, "0x", NULL)) {
>               char *endp;
> @@ -1740,16 +1732,18 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>               if (endp != keys + keyname_len) {
>                   goto err_out;
>               }
> -            keylist->value->type = KEY_VALUE_KIND_NUMBER;
> -            keylist->value->u.number.data = value;
> +            v->type = KEY_VALUE_KIND_NUMBER;
> +            v->u.number.data = value;
>           } else {
>               int idx = index_from_key(keys, keyname_len);
>               if (idx == Q_KEY_CODE__MAX) {
>                   goto err_out;
>               }
> -            keylist->value->type = KEY_VALUE_KIND_QCODE;
> -            keylist->value->u.qcode.data = idx;
> +            v->type = KEY_VALUE_KIND_QCODE;
> +            v->u.qcode.data = idx;
>           }
> +        QAPI_LIST_APPEND(tail, v);
> +        v = NULL;
> 
>           if (!*separator) {
>               break;
> @@ -1761,6 +1755,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>       hmp_handle_error(mon, err);
> 
>   out:
> +    qapi_free_KeyValue(v);

alternative would be to define v as g_autoptr inside while-loop body and use g_steal_pointer() for QAPI_LIST_APPEND().

>       qapi_free_KeyValueList(head);
>       return;

[..]

> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index a5058a3bd15e..10ee740eee1b 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2119,17 +2119,17 @@ void qmp_guest_suspend_hybrid(Error **errp)
>       guest_suspend(SUSPEND_MODE_HYBRID, errp);
>   }
> 
> -static GuestNetworkInterfaceList *
> +static GuestNetworkInterface *
>   guest_find_interface(GuestNetworkInterfaceList *head,
>                        const char *name)
>   {
>       for (; head; head = head->next) {
>           if (strcmp(head->value->name, name) == 0) {
> -            break;
> +            return head->value;
>           }
>       }
> 
> -    return head;
> +    return NULL;
>   }
> 
>   static int guest_get_network_stats(const char *name,
> @@ -2198,7 +2198,7 @@ static int guest_get_network_stats(const char *name,
>    */
>   GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>   {
> -    GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
> +    GuestNetworkInterfaceList *head = NULL, **tail = &head;
>       struct ifaddrs *ifap, *ifa;
> 
>       if (getifaddrs(&ifap) < 0) {
> @@ -2207,9 +2207,10 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>       }
> 
>       for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
> -        GuestNetworkInterfaceList *info;
> -        GuestIpAddressList **address_list = NULL, *address_item = NULL;
> -        GuestNetworkInterfaceStat  *interface_stat = NULL;
> +        GuestNetworkInterface *info;
> +        GuestIpAddressList **address_tail;
> +        GuestIpAddress *address_item = NULL;
> +        GuestNetworkInterfaceStat *interface_stat = NULL;
>           char addr4[INET_ADDRSTRLEN];
>           char addr6[INET6_ADDRSTRLEN];
>           int sock;
> @@ -2223,19 +2224,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
> 
>           if (!info) {
>               info = g_malloc0(sizeof(*info));
> -            info->value = g_malloc0(sizeof(*info->value));
> -            info->value->name = g_strdup(ifa->ifa_name);
> +            info->name = g_strdup(ifa->ifa_name);
> 
> -            if (!cur_item) {
> -                head = cur_item = info;
> -            } else {
> -                cur_item->next = info;
> -                cur_item = info;
> -            }
> +            QAPI_LIST_APPEND(tail, info);
>           }
> 
> -        if (!info->value->has_hardware_address &&
> -            ifa->ifa_flags & SIOCGIFHWADDR) {
> +        address_tail = &info->ip_addresses;
> +
> +        if (!info->has_hardware_address && ifa->ifa_flags & SIOCGIFHWADDR) {
>               /* we haven't obtained HW address yet */
>               sock = socket(PF_INET, SOCK_STREAM, 0);
>               if (sock == -1) {
> @@ -2244,7 +2240,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>               }
> 
>               memset(&ifr, 0, sizeof(ifr));
> -            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->value->name);
> +            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
>               if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
>                   error_setg_errno(errp, errno,
>                                    "failed to get MAC address of %s",
> @@ -2256,13 +2252,13 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>               close(sock);
>               mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
> 
> -            info->value->hardware_address =
> +            info->hardware_address =
>                   g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
>                                   (int) mac_addr[0], (int) mac_addr[1],
>                                   (int) mac_addr[2], (int) mac_addr[3],
>                                   (int) mac_addr[4], (int) mac_addr[5]);
> 
> -            info->value->has_hardware_address = true;
> +            info->has_hardware_address = true;
>           }
> 
>           if (ifa->ifa_addr &&
> @@ -2275,15 +2271,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>               }
> 
>               address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
> -            address_item->value->ip_address = g_strdup(addr4);
> -            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
> +            address_item->ip_address = g_strdup(addr4);
> +            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
> 
>               if (ifa->ifa_netmask) {
>                   /* Count the number of set bits in netmask.
>                    * This is safe as '1' and '0' cannot be shuffled in netmask. */
>                   p = &((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr;
> -                address_item->value->prefix = ctpop32(((uint32_t *) p)[0]);
> +                address_item->prefix = ctpop32(((uint32_t *) p)[0]);
>               }
>           } else if (ifa->ifa_addr &&
>                      ifa->ifa_addr->sa_family == AF_INET6) {
> @@ -2295,15 +2290,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>               }
> 
>               address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
> -            address_item->value->ip_address = g_strdup(addr6);
> -            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
> +            address_item->ip_address = g_strdup(addr6);
> +            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
> 
>               if (ifa->ifa_netmask) {
>                   /* Count the number of set bits in netmask.
>                    * This is safe as '1' and '0' cannot be shuffled in netmask. */
>                   p = &((struct sockaddr_in6 *)ifa->ifa_netmask)->sin6_addr;
> -                address_item->value->prefix =
> +                address_item->prefix =
>                       ctpop32(((uint32_t *) p)[0]) +
>                       ctpop32(((uint32_t *) p)[1]) +
>                       ctpop32(((uint32_t *) p)[2]) +
> @@ -2315,29 +2309,18 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>               continue;
>           }
> 
> -        address_list = &info->value->ip_addresses;

address_tail is used only here, I'd prefere it to be initialized here.

> +        QAPI_LIST_APPEND(address_tail, address_item);
> 
> -        while (*address_list && (*address_list)->next) {
> -            address_list = &(*address_list)->next;
> -        }

Hmm. It's wrong. Original code searches for the end of the list, but with the patch we just APPEND to the head of non-empty list.

As I understand, list may be non-empty if info comes from guest_find_interface, that's why this loop is here.

> +        info->has_ip_addresses = true;
> 
> -        if (!*address_list) {
> -            *address_list = address_item;
> -        } else {
> -            (*address_list)->next = address_item;
> -        }
> -
> -        info->value->has_ip_addresses = true;
> -
> -        if (!info->value->has_statistics) {
> +        if (!info->has_statistics) {


So, with squashed-in:



diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 10ee740eee..c4815d4b0d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2229,8 +2229,6 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
              QAPI_LIST_APPEND(tail, info);
          }
  
-        address_tail = &info->ip_addresses;
-
          if (!info->has_hardware_address && ifa->ifa_flags & SIOCGIFHWADDR) {
              /* we haven't obtained HW address yet */
              sock = socket(PF_INET, SOCK_STREAM, 0);
@@ -2309,6 +2307,10 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
              continue;
          }
  
+        address_tail = &info->ip_addresses;
+        while (!*address_tail) {
+            address_tail = &(*address_tail)->next;
+        }
          QAPI_LIST_APPEND(address_tail, address_item);
  
          info->has_ip_addresses = true;





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


-- 
Best regards,
Vladimir


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

* Re: [PATCH v3 1/7] net: Clarify early exit condition
  2020-12-23 22:10 ` [PATCH v3 1/7] net: Clarify early exit condition Eric Blake
@ 2021-01-13 12:57   ` Markus Armbruster
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 12:57 UTC (permalink / raw)
  To: Eric Blake; +Cc: Jason Wang, qemu-devel

Eric Blake <eblake@redhat.com> writes:

> On first glance, the loop in qmp_query_rx_filter() has early return
> paths that could leak any allocation of filter_list from a previous
> iteration.  But on closer inspection, it is obvious that all of the
> early exits are guarded by has_name, and that the bulk of the loop
> body can be executed at most once if the user is filtering by name,
> thus, any early exit coincides with an empty list.  Add asserts to
> make this obvious.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  net/net.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index e1035f21d183..e581c8a26868 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1211,6 +1211,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
>          if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
>              if (has_name) {
>                  error_setg(errp, "net client(%s) isn't a NIC", name);
> +                assert(!filter_list);
>                  return NULL;
>              }
>              continue;
> @@ -1236,6 +1237,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
>          } else if (has_name) {
>              error_setg(errp, "net client(%s) doesn't support"
>                         " rx-filter querying", name);
> +            assert(!filter_list);
>              return NULL;
>          }

Reviewed-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v3 2/7] rocker: Revamp fp_port_get_info
  2020-12-23 22:10 ` [PATCH v3 2/7] rocker: Revamp fp_port_get_info Eric Blake
@ 2021-01-13 12:57   ` Markus Armbruster
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 12:57 UTC (permalink / raw)
  To: Eric Blake; +Cc: Jason Wang, Jiri Pirko, qemu-devel

Eric Blake <eblake@redhat.com> writes:

> Instead of modifying the value member of a list element passed as a
> parameter, and open-coding the manipulation of that list, it's nicer
> to just return a freshly allocated value to be prepended to a list
> using QAPI_LIST_PREPEND.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Alread in master as commit fe4d7e338f.



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

* Re: [PATCH v3 3/7] migration: Refactor migrate_cap_add
  2020-12-23 22:10 ` [PATCH v3 3/7] migration: Refactor migrate_cap_add Eric Blake
@ 2021-01-13 12:58   ` Markus Armbruster
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 12:58 UTC (permalink / raw)
  To: Eric Blake; +Cc: Juan Quintela, qemu-devel, Dr. David Alan Gilbert

Eric Blake <eblake@redhat.com> writes:

> Instead of taking a list parameter and returning a new head at a
> distance, just return the new item for the caller to insert into a
> list via QAPI_LIST_PREPEND.  Update some variable names to avoid long
> lines, and drop a useless comment.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Already in master as commit eaedde5255.



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

* Re: [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible
  2020-12-23 22:10 ` [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
@ 2021-01-13 13:00   ` Markus Armbruster
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 13:00 UTC (permalink / raw)
  To: Eric Blake
  Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, qemu-devel,
	Gerd Hoffmann, open list:GLUSTER, Juan Quintela,
	David Hildenbrand, Halil Pasic, Christian Borntraeger,
	Marc-André Lureau, Thomas Huth, Jiri Pirko, Eduardo Habkost,
	Michael Roth, Richard Henderson, Dr. David Alan Gilbert,
	Greg Kurz, open list:S390 KVM CPUs, open list:ARM TCG CPUs,
	Stefan Hajnoczi, David Gibson, Kevin Wolf, open list:GLUSTER,
	Daniel P. Berrangé,
	Cornelia Huck, Philippe Mathieu-Daudé,
	Max Reitz, open list:PowerPC TCG CPUs, Paolo Bonzini,
	Aleksandar Rikalo, Aurelien Jarno

Eric Blake <eblake@redhat.com> writes:

> Anywhere we create a list of just one item or by prepending items
> (typically because order doesn't matter), we can use the
> QAPI_LIST_PREPEND macro.  But places where we must keep the list in
> order by appending remain open-coded until later patches.
>
> Note that as a side effect, this also performs a cleanup of two minor
> issues in qga/commands-posix.c: the old code was performing
>  new = g_malloc0(sizeof(*ret));
> which 1) is confusing because you have to verify whether 'new' and
> 'ret' are variables with the same type, and 2) would conflict with C++
> compilation (not an actual problem for this file, but makes
> copy-and-paste harder).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

Already in master as commit 54aa3de72e, except for:

> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 87f14140a381..156223a344ed 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
[...]
> @@ -297,41 +293,31 @@ void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>  static int query_memdev(Object *obj, void *opaque)
>  {
>      MemdevList **list = opaque;
> -    MemdevList *m = NULL;
> +    Memdev *m;
>      QObject *host_nodes;
>      Visitor *v;
>
>      if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
>          m = g_malloc0(sizeof(*m));
>
> -        m->value = g_malloc0(sizeof(*m->value));
> +        m->id = g_strdup(object_get_canonical_path_component(obj));
> +        m->has_id = !!m->id;
>
> -        m->value->id = g_strdup(object_get_canonical_path_component(obj));
> -        m->value->has_id = !!m->value->id;
> -
> -        m->value->size = object_property_get_uint(obj, "size",
> -                                                  &error_abort);
> -        m->value->merge = object_property_get_bool(obj, "merge",
> -                                                   &error_abort);
> -        m->value->dump = object_property_get_bool(obj, "dump",
> -                                                  &error_abort);
> -        m->value->prealloc = object_property_get_bool(obj,
> -                                                      "prealloc",
> -                                                      &error_abort);
> -        m->value->policy = object_property_get_enum(obj,
> -                                                    "policy",
> -                                                    "HostMemPolicy",
> -                                                    &error_abort);
> +        m->size = object_property_get_uint(obj, "size", &error_abort);
> +        m->merge = object_property_get_bool(obj, "merge", &error_abort);
> +        m->dump = object_property_get_bool(obj, "dump", &error_abort);
> +        m->prealloc = object_property_get_bool(obj, "prealloc", &error_abort);
> +        m->policy = object_property_get_enum(obj, "policy", "HostMemPolicy",
> +                                             &error_abort);
>          host_nodes = object_property_get_qobject(obj,
>                                                   "host-nodes",
>                                                   &error_abort);
>          v = qobject_input_visitor_new(host_nodes);
> -        visit_type_uint16List(v, NULL, &m->value->host_nodes, &error_abort);
> +        visit_type_uint16List(v, NULL, &m->host_nodes, &error_abort);
>          visit_free(v);
>          qobject_unref(host_nodes);
>
> -        m->next = *list;
> -        *list = m;
> +        QAPI_LIST_PREPEND(*list, m);
>      }
>
>      return 0;
[...]



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

* Re: [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND
  2020-12-23 22:11 ` [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
  2020-12-24  6:14   ` Vladimir Sementsov-Ogievskiy
@ 2021-01-13 13:04   ` Markus Armbruster
  2021-01-13 14:08     ` Eric Blake
  1 sibling, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 13:04 UTC (permalink / raw)
  To: Eric Blake; +Cc: Michael Roth, qemu-devel

Eric Blake <eblake@redhat.com> writes:

> Similar to the existing QAPI_LIST_PREPEND, but designed for use where
> we want to preserve insertion order.  Callers will be added in
> upcoming patches.  Note the difference in signature: PREPEND takes
> List*, APPEND takes List**.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  include/qapi/util.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 6178e98e97a5..8b4967990c0d 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -37,4 +37,17 @@ int parse_qapi_name(const char *name, bool complete);
>      (list) = _tmp; \
>  } while (0)
>
> +/*
> + * For any pointer to a GenericList @tail, insert @element at the back and
> + * update the tail.
> + *
> + * Note that this macro evaluates @element exactly once, so it is safe
> + * to have side-effects with that argument.
> + */
> +#define QAPI_LIST_APPEND(tail, element) do { \
> +    *(tail) = g_malloc0(sizeof(**(tail))); \
> +    (*(tail))->value = (element); \
> +    (tail) = &(*tail)->next; \
> +} while (0)
> +
>  #endif

You mentioned parenthesizing the lone unparenthesized occurence of
@tail, like

  +    (tail) = &(*(tail))->next; \

Did you decide not to?



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

* Re: [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
  2020-12-23 22:11 ` [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
  2020-12-24  9:56   ` Vladimir Sementsov-Ogievskiy
@ 2021-01-13 13:16   ` Markus Armbruster
  2021-01-13 14:11     ` Eric Blake
  1 sibling, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 13:16 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy,
	Daniel P. Berrangé,
	Eduardo Habkost, open list:Dirty Bitmaps, Michael S. Tsirkin,
	open list:Trivial patches, Michael Roth, Richard Henderson,
	qemu-devel, Max Reitz, Michael Tokarev, Igor Mammedov,
	Paolo Bonzini, John Snow, Dr. David Alan Gilbert, Laurent Vivier

Eric Blake <eblake@redhat.com> writes:

> The easiest spots to use QAPI_LIST_APPEND are where we already have an
> obvious pointer to the tail of a list.  While at it, consistently use
> the variable name 'tail' for that purpose.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
[...]
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 6350caa76530..3f3182007b07 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -42,16 +42,12 @@ static ACPIOSTInfo *acpi_cpu_device_status(int idx, AcpiCpuStatus *cdev)
>      return info;
>  }
>
> -void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
> +void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***tail)
>  {
>      int i;

Sure you want to rename the parameter?  What about:

   void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
   {
  +    ACPIOSTInfoList ***tail = list;
       int i;

>
>      for (i = 0; i < cpu_st->dev_count; i++) {
> -        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
> -        elem->value = acpi_cpu_device_status(i, &cpu_st->devs[i]);
> -        elem->next = NULL;
> -        **list = elem;
> -        *list = &elem->next;
> +        QAPI_LIST_APPEND(*tail, acpi_cpu_device_status(i, &cpu_st->devs[i]));
>      }
>  }
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index f2552b2a4624..e4b836bd5e46 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -51,16 +51,13 @@ static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
>      return info;
>  }
>
> -void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list)
> +void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***tail)

Likewise.

>  {
>      int i;
>
>      for (i = 0; i < mem_st->dev_count; i++) {
> -        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
> -        elem->value = acpi_memory_device_status(i, &mem_st->devs[i]);
> -        elem->next = NULL;
> -        **list = elem;
> -        *list = &elem->next;
> +        QAPI_LIST_APPEND(*tail,
> +                         acpi_memory_device_status(i, &mem_st->devs[i]));
>      }
>  }
>
[...]
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 35459a38bb1c..95326285b76d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4817,20 +4817,17 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose);
>
>  /* Build a list with the name of all features on a feature word array */
>  static void x86_cpu_list_feature_names(FeatureWordArray features,
> -                                       strList **feat_names)
> +                                       strList **tail)

Likewise.

>  {
>      FeatureWord w;
> -    strList **next = feat_names;
>
>      for (w = 0; w < FEATURE_WORDS; w++) {
>          uint64_t filtered = features[w];
>          int i;
>          for (i = 0; i < 64; i++) {
>              if (filtered & (1ULL << i)) {
> -                strList *new = g_new0(strList, 1);
> -                new->value = g_strdup(x86_cpu_feature_name(w, i));
> -                *next = new;
> -                next = &new->next;
> +                QAPI_LIST_APPEND(tail,
> +                                 g_strdup(x86_cpu_feature_name(w, i)));
>              }
>          }
>      }
> @@ -4851,16 +4848,13 @@ static void x86_cpu_get_unavailable_features(Object *obj, Visitor *v,
>   * running using the current machine and accelerator.
>   */
>  static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> -                                                 strList **missing_feats)
> +                                                 strList **tail)

Likewise.

>  {
>      X86CPU *xc;
>      Error *err = NULL;
> -    strList **next = missing_feats;
>
>      if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
> -        strList *new = g_new0(strList, 1);
> -        new->value = g_strdup("kvm");
> -        *missing_feats = new;
> +        QAPI_LIST_APPEND(tail, g_strdup("kvm"));
>          return;
>      }
>
> @@ -4872,16 +4866,13 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>           * but in case it does, just report the model as not
>           * runnable at all using the "type" property.
>           */
> -        strList *new = g_new0(strList, 1);
> -        new->value = g_strdup("type");
> -        *next = new;
> -        next = &new->next;
> +        QAPI_LIST_APPEND(tail, g_strdup("type"));
>          error_free(err);
>      }
>
>      x86_cpu_filter_features(xc, false);
>
> -    x86_cpu_list_feature_names(xc->filtered_features, next);
> +    x86_cpu_list_feature_names(xc->filtered_features, tail);
>
>      object_unref(OBJECT(xc));
>  }
[...]

Other than that:
Reviewed-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND
  2020-12-23 22:11 ` [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
  2020-12-24 11:35   ` Vladimir Sementsov-Ogievskiy
@ 2021-01-13 13:31   ` Markus Armbruster
  1 sibling, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2021-01-13 13:31 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
	open list:GLUSTER, Michael S. Tsirkin, Michael Roth, Jason Wang,
	Juan Quintela, qemu-devel, Max Reitz, Gerd Hoffmann,
	Marc-André Lureau, Igor Mammedov, Dr. David Alan Gilbert

Eric Blake <eblake@redhat.com> writes:

> These cases require a bit more thought to review; in each case, the
> code was appending to a list, but not with a FOOList **tail variable.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
[...]
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index a5058a3bd15e..10ee740eee1b 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2119,17 +2119,17 @@ void qmp_guest_suspend_hybrid(Error **errp)
>      guest_suspend(SUSPEND_MODE_HYBRID, errp);
>  }
>
> -static GuestNetworkInterfaceList *
> +static GuestNetworkInterface *
>  guest_find_interface(GuestNetworkInterfaceList *head,
>                       const char *name)
>  {
>      for (; head; head = head->next) {
>          if (strcmp(head->value->name, name) == 0) {
> -            break;
> +            return head->value;
>          }
>      }
>
> -    return head;
> +    return NULL;
>  }
>
>  static int guest_get_network_stats(const char *name,
> @@ -2198,7 +2198,7 @@ static int guest_get_network_stats(const char *name,
>   */
>  GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>  {
> -    GuestNetworkInterfaceList *head = NULL, *cur_item = NULL;
> +    GuestNetworkInterfaceList *head = NULL, **tail = &head;
>      struct ifaddrs *ifap, *ifa;
>
>      if (getifaddrs(&ifap) < 0) {
> @@ -2207,9 +2207,10 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>      }
>
>      for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
> -        GuestNetworkInterfaceList *info;
> -        GuestIpAddressList **address_list = NULL, *address_item = NULL;
> -        GuestNetworkInterfaceStat  *interface_stat = NULL;
> +        GuestNetworkInterface *info;
> +        GuestIpAddressList **address_tail;
> +        GuestIpAddress *address_item = NULL;
> +        GuestNetworkInterfaceStat *interface_stat = NULL;
>          char addr4[INET_ADDRSTRLEN];
>          char addr6[INET6_ADDRSTRLEN];
>          int sock;
> @@ -2223,19 +2224,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>
>          if (!info) {
>              info = g_malloc0(sizeof(*info));
> -            info->value = g_malloc0(sizeof(*info->value));
> -            info->value->name = g_strdup(ifa->ifa_name);
> +            info->name = g_strdup(ifa->ifa_name);
>
> -            if (!cur_item) {
> -                head = cur_item = info;
> -            } else {
> -                cur_item->next = info;
> -                cur_item = info;
> -            }
> +            QAPI_LIST_APPEND(tail, info);
>          }
>
> -        if (!info->value->has_hardware_address &&
> -            ifa->ifa_flags & SIOCGIFHWADDR) {
> +        address_tail = &info->ip_addresses;
> +
> +        if (!info->has_hardware_address && ifa->ifa_flags & SIOCGIFHWADDR) {

Unrelated line break removal.  I don't mind.

>              /* we haven't obtained HW address yet */
>              sock = socket(PF_INET, SOCK_STREAM, 0);
>              if (sock == -1) {
> @@ -2244,7 +2240,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              }
>
>              memset(&ifr, 0, sizeof(ifr));
> -            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->value->name);
> +            pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
>              if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
>                  error_setg_errno(errp, errno,
>                                   "failed to get MAC address of %s",
> @@ -2256,13 +2252,13 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              close(sock);
>              mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
>
> -            info->value->hardware_address =
> +            info->hardware_address =
>                  g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
>                                  (int) mac_addr[0], (int) mac_addr[1],
>                                  (int) mac_addr[2], (int) mac_addr[3],
>                                  (int) mac_addr[4], (int) mac_addr[5]);
>
> -            info->value->has_hardware_address = true;
> +            info->has_hardware_address = true;
>          }
>
>          if (ifa->ifa_addr &&
> @@ -2275,15 +2271,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              }
>
>              address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
> -            address_item->value->ip_address = g_strdup(addr4);
> -            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
> +            address_item->ip_address = g_strdup(addr4);
> +            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
>
>              if (ifa->ifa_netmask) {
>                  /* Count the number of set bits in netmask.
>                   * This is safe as '1' and '0' cannot be shuffled in netmask. */
>                  p = &((struct sockaddr_in *)ifa->ifa_netmask)->sin_addr;
> -                address_item->value->prefix = ctpop32(((uint32_t *) p)[0]);
> +                address_item->prefix = ctpop32(((uint32_t *) p)[0]);
>              }
>          } else if (ifa->ifa_addr &&
>                     ifa->ifa_addr->sa_family == AF_INET6) {
> @@ -2295,15 +2290,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              }
>
>              address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
> -            address_item->value->ip_address = g_strdup(addr6);
> -            address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
> +            address_item->ip_address = g_strdup(addr6);
> +            address_item->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
>
>              if (ifa->ifa_netmask) {
>                  /* Count the number of set bits in netmask.
>                   * This is safe as '1' and '0' cannot be shuffled in netmask. */
>                  p = &((struct sockaddr_in6 *)ifa->ifa_netmask)->sin6_addr;
> -                address_item->value->prefix =
> +                address_item->prefix =
>                      ctpop32(((uint32_t *) p)[0]) +
>                      ctpop32(((uint32_t *) p)[1]) +
>                      ctpop32(((uint32_t *) p)[2]) +
> @@ -2315,29 +2309,18 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              continue;
>          }
>
> -        address_list = &info->value->ip_addresses;
> +        QAPI_LIST_APPEND(address_tail, address_item);
>
> -        while (*address_list && (*address_list)->next) {
> -            address_list = &(*address_list)->next;
> -        }
> +        info->has_ip_addresses = true;
>
> -        if (!*address_list) {
> -            *address_list = address_item;
> -        } else {
> -            (*address_list)->next = address_item;
> -        }
> -
> -        info->value->has_ip_addresses = true;

Before the patch:

           address_list = &info->value->ip_addresses;

           while (*address_list && (*address_list)->next) {
               address_list = &(*address_list)->next;
           }

           if (!*address_list) {
               *address_list = address_item;
           } else {
               (*address_list)->next = address_item;
           }

Note the loop to advance address list to the tail.

Afterwards (info->value has become info):

           address_tail = &info->ip_addresses;
           [...]
           QAPI_LIST_APPEND(address_tail, address_item);

Not the same, I'm afraid: QAPI_LIST_APPEND() blindly overwrites
info->ip_addresses.

> -
> -        if (!info->value->has_statistics) {
> +        if (!info->has_statistics) {
>              interface_stat = g_malloc0(sizeof(*interface_stat));
> -            if (guest_get_network_stats(info->value->name,
> -                interface_stat) == -1) {
> -                info->value->has_statistics = false;
> +            if (guest_get_network_stats(info->name, interface_stat) == -1) {
> +                info->has_statistics = false;
>                  g_free(interface_stat);
>              } else {
> -                info->value->statistics = interface_stat;
> -                info->value->has_statistics = true;
> +                info->statistics = interface_stat;
> +                info->has_statistics = true;
>              }
>          }
>      }
[...]



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

* Re: [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND
  2021-01-13 13:04   ` Markus Armbruster
@ 2021-01-13 14:08     ` Eric Blake
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2021-01-13 14:08 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Michael Roth, qemu-devel

On 1/13/21 7:04 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Similar to the existing QAPI_LIST_PREPEND, but designed for use where
>> we want to preserve insertion order.  Callers will be added in
>> upcoming patches.  Note the difference in signature: PREPEND takes
>> List*, APPEND takes List**.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  include/qapi/util.h | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/include/qapi/util.h b/include/qapi/util.h
>> index 6178e98e97a5..8b4967990c0d 100644
>> --- a/include/qapi/util.h
>> +++ b/include/qapi/util.h
>> @@ -37,4 +37,17 @@ int parse_qapi_name(const char *name, bool complete);
>>      (list) = _tmp; \
>>  } while (0)
>>
>> +/*
>> + * For any pointer to a GenericList @tail, insert @element at the back and
>> + * update the tail.
>> + *
>> + * Note that this macro evaluates @element exactly once, so it is safe
>> + * to have side-effects with that argument.
>> + */
>> +#define QAPI_LIST_APPEND(tail, element) do { \
>> +    *(tail) = g_malloc0(sizeof(**(tail))); \
>> +    (*(tail))->value = (element); \
>> +    (tail) = &(*tail)->next; \
>> +} while (0)
>> +
>>  #endif
> 
> You mentioned parenthesizing the lone unparenthesized occurence of
> @tail, like
> 
>   +    (tail) = &(*(tail))->next; \
> 
> Did you decide not to?

Hmm; not sure what happened. I still want the () added.  I'll respin
anyways, since my v3 crossed paths with what you already checked in.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
  2020-12-24  9:56   ` Vladimir Sementsov-Ogievskiy
@ 2021-01-13 14:10     ` Eric Blake
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2021-01-13 14:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, open list:Dirty Bitmaps, Michael S. Tsirkin,
	open list:Trivial patches, Michael Roth, Richard Henderson,
	armbru, Max Reitz, Michael Tokarev, Paolo Bonzini, Igor Mammedov,
	John Snow, Dr. David Alan Gilbert, Laurent Vivier

On 12/24/20 3:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 24.12.2020 01:11, Eric Blake wrote:
>> The easiest spots to use QAPI_LIST_APPEND are where we already have an
>> obvious pointer to the tail of a list.  While at it, consistently use
>> the variable name 'tail' for that purpose.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
> 
> [..]
> 
>> --- a/monitor/qmp-cmds-control.c
>> +++ b/monitor/qmp-cmds-control.c
>> @@ -104,17 +104,16 @@ VersionInfo *qmp_query_version(Error **errp)
>>
>>   static void query_commands_cb(const QmpCommand *cmd, void *opaque)
>>   {
>> -    CommandInfoList *info, **list = opaque;
>> +    CommandInfo *info;
>> +    CommandInfoList **tail = opaque;
>>
>>       if (!cmd->enabled) {
>>           return;
>>       }
>>
>>       info = g_malloc0(sizeof(*info));
>> -    info->value = g_malloc0(sizeof(*info->value));
>> -    info->value->name = g_strdup(cmd->name);
>> -    info->next = *list;
>> -    *list = info;
>> +    info->name = g_strdup(cmd->name);
>> +    QAPI_LIST_APPEND(tail, info);
> 
> Original logic is prepend in this hunk.
> 

Good catch; looks like it should be folded in with the remainder of
patch 4/7 on the respin.

> Without this hunk:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
>>   }
>>
>>   CommandInfoList *qmp_query_commands(Error **errp)
> 
> [..]
> 
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -4817,20 +4817,17 @@ static void x86_cpu_filter_features(X86CPU
>> *cpu, bool verbose);
>>
>>   /* Build a list with the name of all features on a feature word
>> array */
>>   static void x86_cpu_list_feature_names(FeatureWordArray features,
>> -                                       strList **feat_names)
>> +                                       strList **tail)
>>   {
>>       FeatureWord w;
>> -    strList **next = feat_names;
>>
>>       for (w = 0; w < FEATURE_WORDS; w++) {
>>           uint64_t filtered = features[w];
>>           int i;
>>           for (i = 0; i < 64; i++) {
>>               if (filtered & (1ULL << i)) {
>> -                strList *new = g_new0(strList, 1);
>> -                new->value = g_strdup(x86_cpu_feature_name(w, i));
>> -                *next = new;
>> -                next = &new->next;
>> +                QAPI_LIST_APPEND(tail,
>> +                                 g_strdup(x86_cpu_feature_name(w, i)));
> 
> actually, fit in one line...
> 
>>               }
>>           }
>>       }
> 
> [..]
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

* Re: [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
  2021-01-13 13:16   ` Markus Armbruster
@ 2021-01-13 14:11     ` Eric Blake
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Blake @ 2021-01-13 14:11 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Kevin Wolf, Fam Zheng, Vladimir Sementsov-Ogievskiy,
	Daniel P. Berrangé,
	Eduardo Habkost, open list:Dirty Bitmaps, Michael S. Tsirkin,
	open list:Trivial patches, Michael Roth, Richard Henderson,
	qemu-devel, Max Reitz, Michael Tokarev, Igor Mammedov,
	Paolo Bonzini, John Snow, Dr. David Alan Gilbert, Laurent Vivier

On 1/13/21 7:16 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> The easiest spots to use QAPI_LIST_APPEND are where we already have an
>> obvious pointer to the tail of a list.  While at it, consistently use
>> the variable name 'tail' for that purpose.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>

>> -void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>> +void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***tail)
>>  {
>>      int i;
> 
> Sure you want to rename the parameter?  What about:
> 
>    void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>    {
>   +    ACPIOSTInfoList ***tail = list;
>        int i;

Reasonable.  I'll pick it up for v4.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

end of thread, other threads:[~2021-01-13 14:28 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-23 22:10 [PATCH v3 0/7] Common macros for QAPI list growth Eric Blake
2020-12-23 22:10 ` [PATCH v3 1/7] net: Clarify early exit condition Eric Blake
2021-01-13 12:57   ` Markus Armbruster
2020-12-23 22:10 ` [PATCH v3 2/7] rocker: Revamp fp_port_get_info Eric Blake
2021-01-13 12:57   ` Markus Armbruster
2020-12-23 22:10 ` [PATCH v3 3/7] migration: Refactor migrate_cap_add Eric Blake
2021-01-13 12:58   ` Markus Armbruster
2020-12-23 22:10 ` [PATCH v3 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
2021-01-13 13:00   ` Markus Armbruster
2020-12-23 22:11 ` [PATCH v3 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
2020-12-24  6:14   ` Vladimir Sementsov-Ogievskiy
2021-01-13 13:04   ` Markus Armbruster
2021-01-13 14:08     ` Eric Blake
2020-12-23 22:11 ` [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
2020-12-24  9:56   ` Vladimir Sementsov-Ogievskiy
2021-01-13 14:10     ` Eric Blake
2021-01-13 13:16   ` Markus Armbruster
2021-01-13 14:11     ` Eric Blake
2020-12-23 22:11 ` [PATCH v3 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
2020-12-24 11:35   ` Vladimir Sementsov-Ogievskiy
2021-01-13 13:31   ` Markus Armbruster

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.