* [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-16 14:22 ` Markus Armbruster
2020-11-13 1:13 ` [PATCH v2 2/7] rocker: Revamp fp_port_get_info Eric Blake
` (6 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, armbru
If qmp_query_rx_filter() encounters an error on a second iteration, it
leaks the memory from the first.
Fixes: 9083da1d4c
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 794c652282cb..eb65e110871a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1213,6 +1213,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);
+ qapi_free_RxFilterInfoList(filter_list);
return NULL;
}
continue;
@@ -1238,6 +1239,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);
+ qapi_free_RxFilterInfoList(filter_list);
return NULL;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error
2020-11-13 1:13 ` [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error Eric Blake
@ 2020-11-16 14:22 ` Markus Armbruster
2020-11-16 14:41 ` Eric Blake
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2020-11-16 14:22 UTC (permalink / raw)
To: Eric Blake; +Cc: Jason Wang, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> If qmp_query_rx_filter() encounters an error on a second iteration, it
> leaks the memory from the first.
>
> Fixes: 9083da1d4c
> 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 794c652282cb..eb65e110871a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1213,6 +1213,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
NetClientState *nc;
RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
QTAILQ_FOREACH(nc, &net_clients, next) {
RxFilterInfoList *entry;
RxFilterInfo *info;
if (has_name && strcmp(nc->name, name) != 0) {
continue;
}
If @has_name and we get here more than once, then multiple @net_clients
have the same name. How can that be? We are not supposed to return
multiple replies with the same @name, are we?
/* only query rx-filter information of NIC */
> if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
> if (has_name) {
> error_setg(errp, "net client(%s) isn't a NIC", name);
> + qapi_free_RxFilterInfoList(filter_list);
Unless multiple @net_clients are named @name, @filter_list is null,
isn't it?
> return NULL;
> }
> continue;
}
/* only query information on queue 0 since the info is per nic,
* not per queue
*/
if (nc->queue_index != 0)
continue;
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;
From now on, we must either return or free @filter_list.
} else {
last_entry->next = entry;
}
last_entry = entry;
> @@ -1238,6 +1239,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);
> + qapi_free_RxFilterInfoList(filter_list);
Unless multiple @net_clients are named @name, @filter_list is null,
isn't it?
> return NULL;
> }
if (has_name) {
break;
}
}
I dislike this loop.
if (filter_list == NULL && has_name) {
error_setg(errp, "invalid net client name: %s", name);
}
return filter_list;
I should've strangled the optional @name parameter in the crib.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error
2020-11-16 14:22 ` Markus Armbruster
@ 2020-11-16 14:41 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2020-11-16 14:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Jason Wang, qemu-devel
On 11/16/20 8:22 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> If qmp_query_rx_filter() encounters an error on a second iteration, it
>> leaks the memory from the first.
>>
>> Fixes: 9083da1d4c
>> 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 794c652282cb..eb65e110871a 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1213,6 +1213,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
> NetClientState *nc;
> RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
>
> QTAILQ_FOREACH(nc, &net_clients, next) {
> RxFilterInfoList *entry;
> RxFilterInfo *info;
>
> if (has_name && strcmp(nc->name, name) != 0) {
> continue;
> }
>
> If @has_name and we get here more than once, then multiple @net_clients
> have the same name. How can that be? We are not supposed to return
> multiple replies with the same @name, are we?
Oh, good spot. I was going off of the fact that the loop had an early
exit, but did not further note that early exit is only possible in the
case where the bulk of the loop executes exactly once.
>
> /* only query rx-filter information of NIC */
>> if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
>> if (has_name) {
>> error_setg(errp, "net client(%s) isn't a NIC", name);
>> + qapi_free_RxFilterInfoList(filter_list);
>
> Unless multiple @net_clients are named @name, @filter_list is null,
> isn't it?
Correct.
>
>> return NULL;
>> }
>> continue;
> }
>
> /* only query information on queue 0 since the info is per nic,
> * not per queue
> */
> if (nc->queue_index != 0)
> continue;
>
> 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;
>
>>From now on, we must either return or free @filter_list.
>
> } else {
> last_entry->next = entry;
> }
> last_entry = entry;
>> @@ -1238,6 +1239,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);
>> + qapi_free_RxFilterInfoList(filter_list);
>
> Unless multiple @net_clients are named @name, @filter_list is null,
> isn't it?
>
>> return NULL;
>> }
>
> if (has_name) {
> break;
> }
> }
>
> I dislike this loop.
No joke. But you've confirmed that this patch is unnecessary; and the
rest of my series is 6.0 material, so there's nothing left to worry
about for 5.2 from this series.
>
> if (filter_list == NULL && has_name) {
> error_setg(errp, "invalid net client name: %s", name);
> }
>
> return filter_list;
>
> I should've strangled the optional @name parameter in the crib.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/7] rocker: Revamp fp_port_get_info
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
2020-11-13 1:13 ` [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-17 9:27 ` Markus Armbruster
2020-11-13 1:13 ` [PATCH v2 3/7] migration: Refactor migrate_cap_add Eric Blake
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 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>
---
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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/7] rocker: Revamp fp_port_get_info
2020-11-13 1:13 ` [PATCH v2 2/7] rocker: Revamp fp_port_get_info Eric Blake
@ 2020-11-17 9:27 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2020-11-17 9:27 UTC (permalink / raw)
To: Eric Blake; +Cc: Jason Wang, Jiri Pirko, qemu-devel, armbru
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>
> ---
> 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]));
Relies on QAPI_LIST_PREPEND() evaluating its second argument exactly
once. Part of its written contract; okay.
> }
>
> 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)
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 3/7] migration: Refactor migrate_cap_add
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
2020-11-13 1:13 ` [PATCH v2 1/7 for-5.2?] net: Fix memory leak on error Eric Blake
2020-11-13 1:13 ` [PATCH v2 2/7] rocker: Revamp fp_port_get_info Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-17 9:45 ` Markus Armbruster
2020-11-13 1:13 ` [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 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.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
migration/migration.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 3263aa55a9da..e8c414a7b8f0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1667,27 +1667,23 @@ 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 *cap = NULL;
- cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
+ QAPI_LIST_PREPEND(cap, migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value));
qmp_migrate_set_capabilities(cap, errp);
qapi_free_MigrationCapabilityStatusList(cap);
}
@@ -3874,7 +3870,7 @@ 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));
}
}
--
2.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 3/7] migration: Refactor migrate_cap_add
2020-11-13 1:13 ` [PATCH v2 3/7] migration: Refactor migrate_cap_add Eric Blake
@ 2020-11-17 9:45 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2020-11-17 9:45 UTC (permalink / raw)
To: Eric Blake; +Cc: Juan Quintela, qemu-devel, Dr. David Alan Gilbert, armbru
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.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> migration/migration.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 3263aa55a9da..e8c414a7b8f0 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1667,27 +1667,23 @@ 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 *cap = NULL;
>
> - cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
> + QAPI_LIST_PREPEND(cap, migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value));
Line is too long for my taste. Please consider
MigrationCapabilityStatusList *caps = NULL;
MigrationCapabilityStatus *cap;
cap = migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value);
QAPI_LIST_PREPEND(caps, cap);
qmp_migrate_set_capabilities(caps, errp);
qapi_free_MigrationCapabilityStatusList(caps);
> qmp_migrate_set_capabilities(cap, errp);
> qapi_free_MigrationCapabilityStatusList(cap);
> }
> @@ -3874,7 +3870,7 @@ 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 */
Unrelated: this comment is not worth its keep.
qapi_free_MigrationCapabilityStatusList(head);
return ret;
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
` (2 preceding siblings ...)
2020-11-13 1:13 ` [PATCH v2 3/7] migration: Refactor migrate_cap_add Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-17 10:20 ` Markus Armbruster
2020-11-17 11:45 ` Stefan Hajnoczi
2020-11-13 1:13 ` [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
` (3 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Thomas Huth,
Michael Roth, Gerd Hoffmann, open list:GLUSTER, Juan Quintela,
David Hildenbrand, armbru, Halil Pasic, Christian Borntraeger,
Marc-André Lureau, Richard Henderson, Aleksandar Rikalo,
Jiri Pirko, Eduardo Habkost, Dr. David Alan Gilbert,
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 now-public
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>
---
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 | 6 +-
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 | 46 +++--------
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/helper.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, 158 insertions(+), 420 deletions(-)
diff --git a/docs/devel/writing-qmp-commands.txt b/docs/devel/writing-qmp-commands.txt
index 46a6c48683f5..2349ff8da69d 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 aa4282164aca..f8d4c8f8f1e8 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 5362c80a1873..ca39d15d93a2 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);
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d0408049b53c..32d50ad74d07 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -494,11 +494,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)
@@ -559,7 +555,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);
@@ -572,9 +567,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 9179013ac402..26b575324081 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 e8c414a7b8f0..84e5f4982fb2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -417,12 +417,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));
}
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 a6a6684df1c6..1c643de4ca30 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1254,7 +1254,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);
@@ -1262,14 +1263,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 32e6a8c13d07..9d2fbc4b06be 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1421,33 +1421,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 12c1ba5ef79d..2f7a91d46746 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);
}
}
@@ -1287,7 +1282,6 @@ static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
}
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 */
@@ -1295,10 +1289,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->dependents;
- disk->dependents = dep_item;
+ QAPI_LIST_PREPEND(disk->dependents, dev_name);
}
}
g_dir_close(dp_deps);
@@ -1317,7 +1308,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);
@@ -1351,14 +1342,9 @@ static GuestDiskInfoList *get_disk_partitions(
partition->name = dev_name;
partition->partition = true;
/* Add parent disk as dependent for easier tracking of hierarchy */
- partition->dependents = g_new0(strList, 1);
- partition->dependents->value = g_strdup(disk_dev);
-
- item = g_new0(GuestDiskInfoList, 1);
- item->value = partition;
- item->next = ret;
- ret = item;
+ QAPI_LIST_PREPEND(partition->dependents, g_strdup(disk_dev));
+ QAPI_LIST_PREPEND(ret, partition);
}
closedir(dp_disk);
@@ -1367,7 +1353,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;
@@ -1413,10 +1399,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);
@@ -1493,7 +1476,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);
@@ -1506,10 +1489,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);
@@ -1775,7 +1755,6 @@ GuestFilesystemTrimResponse *
qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
{
GuestFilesystemTrimResponse *response;
- GuestFilesystemTrimResultList *list;
GuestFilesystemTrimResult *result;
int ret = 0;
FsMountList mounts;
@@ -1799,10 +1778,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 300b87c859af..1696e50d54a7 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 310ab2d0481d..d035377e9f70 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,
@@ -155,7 +150,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 ||
@@ -181,10 +175,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);
@@ -222,7 +213,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);
@@ -230,10 +220,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 11b0803df722..bbaafddf76d7 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 0d8606958e9e..562e9632caf2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4984,7 +4984,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));
@@ -5009,10 +5008,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/helper.c b/target/mips/helper.c
index 063b65c0528d..3482331adec6 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -1502,7 +1502,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;
@@ -1512,10 +1511,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 49235056f7a8..6e61d2ad5fd5 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 660f47b0050f..7b4fec837bca 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 dc68da3cfd69..2740d4a53756 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10623,7 +10623,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);
@@ -10631,10 +10630,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)
@@ -10650,7 +10646,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);
@@ -10662,10 +10657,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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible
2020-11-13 1:13 ` [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
@ 2020-11-17 10:20 ` Markus Armbruster
2020-11-17 11:45 ` Stefan Hajnoczi
1 sibling, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2020-11-17 10:20 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, Michael Roth, Halil Pasic,
Christian Borntraeger, Marc-André Lureau, Richard Henderson,
Thomas Huth, Jiri Pirko, Eduardo Habkost, Dr. David Alan Gilbert,
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 now-public
> macro. But places where we must keep the list in order by appending
> remain open-coded until later patches.
"now-public" suggests a patch in this series made it public. Used to be
the case, but no more. Suggest "we can use QAPI_LIST_PREPEND()".
> 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).
I consider 2) a complete non-issue :)
> Signed-off-by: Eric Blake <eblake@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 | 6 +-
> 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 | 46 +++--------
> 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/helper.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, 158 insertions(+), 420 deletions(-)
Quite a few more instances as in v1. Some of the more "creative" ones
were bothersome to review. I figure they were just as bothersome to
clean up. Thanks for that!
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible
2020-11-13 1:13 ` [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
2020-11-17 10:20 ` Markus Armbruster
@ 2020-11-17 11:45 ` Stefan Hajnoczi
1 sibling, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2020-11-17 11:45 UTC (permalink / raw)
To: Eric Blake
Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, qemu-devel,
Michael Roth, Gerd Hoffmann, open list:GLUSTER, Juan Quintela,
David Hildenbrand, armbru, Halil Pasic, Christian Borntraeger,
Thomas Huth, Marc-André Lureau, Richard Henderson,
Aleksandar Rikalo, Jiri Pirko, Eduardo Habkost,
Dr. David Alan Gilbert, open list:S390 KVM CPUs,
open list:ARM TCG CPUs, 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
[-- Attachment #1: Type: text/plain, Size: 2551 bytes --]
On Thu, Nov 12, 2020 at 07:13:37PM -0600, Eric Blake wrote:
> Anywhere we create a list of just one item or by prepending items
> (typically because order doesn't matter), we can use the now-public
> 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>
> ---
> 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 | 6 +-
> 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 | 46 +++--------
> 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/helper.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, 158 insertions(+), 420 deletions(-)
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
` (3 preceding siblings ...)
2020-11-13 1:13 ` [PATCH v2 4/7] qapi: Use QAPI_LIST_PREPEND() where possible Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-17 12:51 ` Markus Armbruster
2020-11-13 1:13 ` [PATCH v2 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
2020-11-13 1:13 ` [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
@ 2020-11-17 12:51 ` Markus Armbruster
2020-11-18 0:41 ` Eric Blake
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2020-11-17 12:51 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
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
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
2020-11-17 12:51 ` Markus Armbruster
@ 2020-11-18 0:41 ` Eric Blake
2020-11-18 6:21 ` Markus Armbruster
0 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2020-11-18 0:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
On 11/17/20 6:51 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>
>> +#define QAPI_LIST_APPEND(tail, element) do { \
>> + *(tail) = g_malloc0(sizeof(**(tail))); \
>> + (*(tail))->value = (element); \
>> + (tail) = &(*tail)->next; \
Hmm; I'm inconsistent on whether to spell things '*tail' or '*(tail)'.
I don't think any of the callers converted in patches 6 or 7 care about
the difference, but for maximal copy-paste portability, the use of the
macro parameter should be surrounded by () anywhere that could otherwise
cause a mis-parse on some arbitrary expression with an operator at
higher precedence than unary * (hmm, the only such operators are all
suffix operators; so maybe the *(tail) is overkill...)
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND
2020-11-18 0:41 ` Eric Blake
@ 2020-11-18 6:21 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2020-11-18 6:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 11/17/20 6:51 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>
>
>>> +#define QAPI_LIST_APPEND(tail, element) do { \
>>> + *(tail) = g_malloc0(sizeof(**(tail))); \
>>> + (*(tail))->value = (element); \
>>> + (tail) = &(*tail)->next; \
>
> Hmm; I'm inconsistent on whether to spell things '*tail' or '*(tail)'.
> I don't think any of the callers converted in patches 6 or 7 care about
> the difference, but for maximal copy-paste portability, the use of the
> macro parameter should be surrounded by () anywhere that could otherwise
> cause a mis-parse on some arbitrary expression with an operator at
> higher precedence than unary * (hmm, the only such operators are all
> suffix operators; so maybe the *(tail) is overkill...)
Good habit: enclose macro parameter in parenthesis unless there is a
reason not to. Let's do it here, too.
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
` (4 preceding siblings ...)
2020-11-13 1:13 ` [PATCH v2 5/7] qapi: Introduce QAPI_LIST_APPEND Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-13 1:13 ` [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
2020-11-19 9:28 ` [PATCH v2 0/7] Common macros for QAPI list growth Markus Armbruster
7 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 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 Tokarev, armbru, Max Reitz,
Laurent Vivier, Paolo Bonzini, Igor Mammedov, John Snow,
Michael Roth, Dr. David Alan Gilbert, Richard Henderson
The easiest spots to use QAPI_LIST_APPEND are where we already have an
obvious pointer to the tail of a list.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
backends/hostmem.c | 8 +---
block/dirty-bitmap.c | 6 +--
block/export/export.c | 5 +--
block/qapi.c | 19 ++-------
block/qcow2-bitmap.c | 11 +-----
block/vmdk.c | 5 +--
blockdev.c | 11 ++----
crypto/block-luks.c | 9 ++---
hw/acpi/cpu.c | 6 +--
hw/acpi/memory_hotplug.c | 7 +---
iothread.c | 8 +---
job-qmp.c | 11 ++----
monitor/hmp-cmds.c | 8 ++--
monitor/qmp-cmds-control.c | 9 ++---
qemu-img.c | 6 +--
qga/commands-posix.c | 13 +------
qga/commands-win32.c | 7 +---
scsi/pr-manager.c | 8 +---
target/i386/cpu.c | 19 +++------
tests/test-qobject-output-visitor.c | 60 ++++++-----------------------
tests/test-string-output-visitor.c | 4 +-
21 files changed, 57 insertions(+), 183 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4bde00e8e74d..5d390c0c91d2 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -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(node, 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(node, value);
} while (true);
ret:
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c01319b188c3..e6d09d2118de 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -577,7 +577,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
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(plist, info);
}
bdrv_dirty_bitmaps_unlock(bs);
diff --git a/block/export/export.c b/block/export/export.c
index bad6f21b1c15..b0743a7addad 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -342,7 +342,6 @@ BlockExportInfoList *qmp_query_block_exports(Error **errp)
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),
@@ -351,9 +350,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(p_next, info);
}
return head;
diff --git a/block/qapi.c b/block/qapi.c
index 0ca206f559fe..f6d00b0909a1 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -421,14 +421,9 @@ static uint64List *uint64_list(uint64_t *list, int size)
uint64List **pout_list = &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(pout_list, list[i]);
}
- *pout_list = NULL;
-
return out_list;
}
@@ -643,19 +638,14 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
/* 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(p_next, 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(p_next, s);
}
}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index d7a31a8ddcdb..465dd8f9b3a5 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -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(plist, map[i].info);
flags &= ~map[i].bme;
}
}
@@ -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(plist, info);
}
bitmap_list_free(bm_list);
diff --git a/block/vmdk.c b/block/vmdk.c
index a00dc00eb47a..a38ac09d4472 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2945,10 +2945,7 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs,
next = &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(next, vmdk_get_extent_info(&s->extents[i]));
}
return spec_info;
diff --git a/blockdev.c b/blockdev.c
index fe6fb5dc1d19..ba158c45d479 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3674,24 +3674,21 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
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(p_next, value);
}
return head;
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 564caa10949b..8d01e482778f 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 **prev = &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(prev, slot);
}
return 0;
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index f099b5092730..5465e948e5dc 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -47,11 +47,7 @@ void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***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(*list, 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..1e5369f08443 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -56,11 +56,8 @@ void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list)
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(*list,
+ acpi_memory_device_status(i, &mem_st->devs[i]));
}
}
diff --git a/iothread.c b/iothread.c
index 69eff9efbc70..951b29566813 100644
--- a/iothread.c
+++ b/iothread.c
@@ -311,7 +311,6 @@ AioContext *iothread_get_aio_context(IOThread *iothread)
static int query_one_iothread(Object *object, void *opaque)
{
IOThreadInfoList ***prev = opaque;
- IOThreadInfoList *elem;
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(*prev, info);
return 0;
}
diff --git a/job-qmp.c b/job-qmp.c
index 645601b2ccc1..3d33921d171a 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -168,24 +168,21 @@ JobInfoList *qmp_query_jobs(Error **errp)
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(p_next, value);
}
return head;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 1c643de4ca30..01a7d317c3c9 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -79,16 +79,16 @@ static strList *strList_from_comma_list(const char *in)
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(hook, value);
}
return res;
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 17514f495965..a78cf2bcaf4f 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 **list = 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(list, info);
}
CommandInfoList *qmp_query_commands(Error **errp)
diff --git a/qemu-img.c b/qemu-img.c
index d599659c7f29..bba5b6ffecc8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -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(last, info);
blk_unref(blk);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 2f7a91d46746..d8bc40ea9f6e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2450,7 +2450,6 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
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);
@@ -2460,10 +2459,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(link, vcpu);
}
g_free(path);
}
@@ -2824,7 +2820,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)) {
@@ -2840,11 +2835,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(link, mem_blk);
}
closedir(dp);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1696e50d54a7..031bbe223ecf 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1865,18 +1865,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(link, vcpu);
}
cpu_bits >>= 1;
}
diff --git a/scsi/pr-manager.c b/scsi/pr-manager.c
index 32b9287e680d..9f38e8424c99 100644
--- a/scsi/pr-manager.c
+++ b/scsi/pr-manager.c
@@ -117,7 +117,6 @@ pr_manager_register_types(void)
static int query_one_pr_manager(Object *object, void *opaque)
{
PRManagerInfoList ***prev = opaque;
- PRManagerInfoList *elem;
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(*prev, info);
return 0;
}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 562e9632caf2..56a35a95a98e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4790,17 +4790,14 @@ static void x86_cpu_list_feature_names(FeatureWordArray features,
strList **feat_names)
{
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(feat_names,
+ g_strdup(x86_cpu_feature_name(w, i)));
}
}
}
@@ -4825,12 +4822,9 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
{
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(missing_feats, g_strdup("kvm"));
return;
}
@@ -4842,16 +4836,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(missing_feats, 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, missing_feats);
object_unref(OBJECT(xc));
}
diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index b20ab8b29b85..1ba35938fe4b 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -444,120 +444,84 @@ static void init_list_union(UserDefListUnion *cvalue)
case USER_DEF_LIST_UNION_KIND_INTEGER: {
intList **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_S8: {
int8List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_S16: {
int16List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_S32: {
int32List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_S64: {
int64List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_U8: {
uint8List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_U16: {
uint16List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_U32: {
uint32List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_U64: {
uint64List **list = &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(list, i);
}
break;
}
case USER_DEF_LIST_UNION_KIND_BOOLEAN: {
boolList **list = &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(list, QEMU_IS_ALIGNED(i, 3));
}
break;
}
case USER_DEF_LIST_UNION_KIND_STRING: {
strList **list = &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(list, g_strdup_printf("%d", i));
}
break;
}
case USER_DEF_LIST_UNION_KIND_NUMBER: {
numberList **list = &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(list, (double)i / 3);
}
break;
}
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 9f6581439ade..7cee4ed52ee0 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -94,9 +94,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
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(tmp, value[i]);
}
visit_type_intList(data->ov, NULL, &list, &err);
--
2.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
` (5 preceding siblings ...)
2020-11-13 1:13 ` [PATCH v2 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases Eric Blake
@ 2020-11-13 1:13 ` Eric Blake
2020-11-13 19:39 ` Dr. David Alan Gilbert
2020-11-19 8:50 ` Markus Armbruster
2020-11-19 9:28 ` [PATCH v2 0/7] Common macros for QAPI list growth Markus Armbruster
7 siblings, 2 replies; 23+ messages in thread
From: Eric Blake @ 2020-11-13 1:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
open list:GLUSTER, Michael S. Tsirkin, Jason Wang, Juan Quintela,
armbru, Max Reitz, Gerd Hoffmann, Igor Mammedov,
Marc-André Lureau, Michael Roth, 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 | 125 +++++++++++++++----------------------
hw/mem/memory-device.c | 12 +---
hw/pci/pci.c | 60 ++++++------------
migration/migration.c | 20 ++----
monitor/hmp-cmds.c | 23 +++----
net/net.c | 13 +---
qga/commands-posix.c | 101 +++++++++++-------------------
qga/commands-win32.c | 88 +++++++++-----------------
softmmu/tpm.c | 38 ++---------
ui/spice-core.c | 27 +++-----
13 files changed, 180 insertions(+), 376 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 1f8699b93835..4963642d6e6b 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 **curr;
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);
+ curr = &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(curr, gsconf);
gsconf = NULL;
qobject_unref(backing_options);
diff --git a/block/qapi.c b/block/qapi.c
index f6d00b0909a1..0218462c5a77 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 ca39d15d93a2..711814be2312 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, **last = &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(last, 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, **last = &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(last, value);
}
return head;
@@ -294,41 +281,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_APPEND(list, m);
}
return 0;
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index cf0627fd01c1..1afcc29a0649 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, **prev = &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(prev, info);
}
g_slist_free(devices);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 0131d9d02c16..43f19e4ab219 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1681,41 +1681,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;
@@ -1812,23 +1805,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));
}
}
@@ -1851,21 +1835,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 84e5f4982fb2..b97eb2d0494e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -797,29 +797,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 01a7d317c3c9..678f388d2e1f 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1699,7 +1699,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;
+ 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;
@@ -1716,16 +1717,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;
@@ -1734,16 +1726,17 @@ 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);
if (!*separator) {
break;
diff --git a/net/net.c b/net/net.c
index eb65e110871a..453865db6f10 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1199,10 +1199,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, **last = &filter_list;
QTAILQ_FOREACH(nc, &net_clients, next) {
- RxFilterInfoList *entry;
RxFilterInfo *info;
if (has_name && strcmp(nc->name, name) != 0) {
@@ -1227,15 +1226,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(last, 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 d8bc40ea9f6e..55087e268cda 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2118,17 +2118,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,
@@ -2197,7 +2197,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) {
@@ -2206,9 +2206,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;
@@ -2222,19 +2223,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) {
@@ -2243,7 +2239,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",
@@ -2255,13 +2251,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 &&
@@ -2274,15 +2270,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) {
@@ -2294,15 +2289,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]) +
@@ -2314,29 +2308,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;
}
}
}
@@ -2863,7 +2846,6 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
while (mem_blks != NULL) {
GuestMemoryBlockResponse *result;
- GuestMemoryBlockResponseList *entry;
GuestMemoryBlock *current_mem_blk = mem_blks->value;
result = g_malloc0(sizeof(*result));
@@ -2872,11 +2854,7 @@ 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(link, result);
mem_blks = mem_blks->next;
}
@@ -3107,11 +3085,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);
@@ -3134,19 +3111,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 031bbe223ecf..cfc530975170 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1625,11 +1625,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;
@@ -1652,30 +1652,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) {
@@ -1686,37 +1680,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;
}
}
}
@@ -2083,12 +2069,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;
@@ -2124,23 +2109,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);
@@ -2425,7 +2404,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;
@@ -2523,14 +2502,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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND
2020-11-13 1:13 ` [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
@ 2020-11-13 19:39 ` Dr. David Alan Gilbert
2020-11-16 13:27 ` Eric Blake
2020-11-19 8:50 ` Markus Armbruster
1 sibling, 1 reply; 23+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-13 19:39 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
open list:GLUSTER, Michael S. Tsirkin, armbru, Jason Wang,
Juan Quintela, qemu-devel, Max Reitz, Gerd Hoffmann,
Igor Mammedov, Marc-André Lureau, Michael Roth
* Eric Blake (eblake@redhat.com) 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>
> ---
> block/gluster.c | 13 +---
> block/qapi.c | 14 +----
> dump/dump.c | 22 ++-----
> hw/core/machine-qmp-cmds.c | 125 +++++++++++++++----------------------
> hw/mem/memory-device.c | 12 +---
> hw/pci/pci.c | 60 ++++++------------
> migration/migration.c | 20 ++----
> monitor/hmp-cmds.c | 23 +++----
> net/net.c | 13 +---
> qga/commands-posix.c | 101 +++++++++++-------------------
> qga/commands-win32.c | 88 +++++++++-----------------
> softmmu/tpm.c | 38 ++---------
> ui/spice-core.c | 27 +++-----
> 13 files changed, 180 insertions(+), 376 deletions(-)
>
<snip>
> diff --git a/migration/migration.c b/migration/migration.c
> index 84e5f4982fb2..b97eb2d0494e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -797,29 +797,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 01a7d317c3c9..678f388d2e1f 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1699,7 +1699,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;
> + 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;
> @@ -1716,16 +1717,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;
> @@ -1734,16 +1726,17 @@ 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);
>
> if (!*separator) {
> break;
Don't you need to arrange for 'v' to be free'd in the err_out case?
Dave
> diff --git a/net/net.c b/net/net.c
> index eb65e110871a..453865db6f10 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1199,10 +1199,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, **last = &filter_list;
>
> QTAILQ_FOREACH(nc, &net_clients, next) {
> - RxFilterInfoList *entry;
> RxFilterInfo *info;
>
> if (has_name && strcmp(nc->name, name) != 0) {
> @@ -1227,15 +1226,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(last, 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 d8bc40ea9f6e..55087e268cda 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2118,17 +2118,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,
> @@ -2197,7 +2197,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) {
> @@ -2206,9 +2206,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;
> @@ -2222,19 +2223,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) {
> @@ -2243,7 +2239,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",
> @@ -2255,13 +2251,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 &&
> @@ -2274,15 +2270,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) {
> @@ -2294,15 +2289,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]) +
> @@ -2314,29 +2308,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;
> }
> }
> }
> @@ -2863,7 +2846,6 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
>
> while (mem_blks != NULL) {
> GuestMemoryBlockResponse *result;
> - GuestMemoryBlockResponseList *entry;
> GuestMemoryBlock *current_mem_blk = mem_blks->value;
>
> result = g_malloc0(sizeof(*result));
> @@ -2872,11 +2854,7 @@ 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(link, result);
> mem_blks = mem_blks->next;
> }
>
> @@ -3107,11 +3085,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);
> @@ -3134,19 +3111,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 031bbe223ecf..cfc530975170 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1625,11 +1625,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;
> @@ -1652,30 +1652,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) {
> @@ -1686,37 +1680,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;
> }
> }
> }
> @@ -2083,12 +2069,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;
> @@ -2124,23 +2109,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);
> @@ -2425,7 +2404,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;
> @@ -2523,14 +2502,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.28.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND
2020-11-13 19:39 ` Dr. David Alan Gilbert
@ 2020-11-16 13:27 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2020-11-16 13:27 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
open list:GLUSTER, Michael S. Tsirkin, armbru, Jason Wang,
Juan Quintela, qemu-devel, Max Reitz, Gerd Hoffmann,
Igor Mammedov, Marc-André Lureau, Michael Roth
On 11/13/20 1:39 PM, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) 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>
>> ---
>
> <snip>
>
>> +++ b/monitor/hmp-cmds.c
>> @@ -1699,7 +1699,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;
>> + 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;
>> @@ -1716,16 +1717,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;
>> @@ -1734,16 +1726,17 @@ 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);
>>
>> if (!*separator) {
>> break;
>
> Don't you need to arrange for 'v' to be free'd in the err_out case?
Good catch. Pre-patch, the allocation was appended to the list before
it was possible to reach 'goto err_out', but post-patch, the use of a
separate variable and delayed addition to the list matters. Will fix.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND
2020-11-13 1:13 ` [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
2020-11-13 19:39 ` Dr. David Alan Gilbert
@ 2020-11-19 8:50 ` Markus Armbruster
2020-12-04 22:54 ` Eric Blake
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2020-11-19 8:50 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
open list:GLUSTER, Michael S. Tsirkin, Jason Wang, Juan Quintela,
qemu-devel, armbru, Gerd Hoffmann, Marc-André Lureau,
Igor Mammedov, Max Reitz, Michael Roth, 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>
> ---
> block/gluster.c | 13 +---
> block/qapi.c | 14 +----
> dump/dump.c | 22 ++-----
> hw/core/machine-qmp-cmds.c | 125 +++++++++++++++----------------------
> hw/mem/memory-device.c | 12 +---
> hw/pci/pci.c | 60 ++++++------------
> migration/migration.c | 20 ++----
> monitor/hmp-cmds.c | 23 +++----
> net/net.c | 13 +---
> qga/commands-posix.c | 101 +++++++++++-------------------
> qga/commands-win32.c | 88 +++++++++-----------------
> softmmu/tpm.c | 38 ++---------
> ui/spice-core.c | 27 +++-----
> 13 files changed, 180 insertions(+), 376 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 1f8699b93835..4963642d6e6b 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 **curr;
Looks like "a FOOList **tail variable" to me. Hmm, unlike the ones in
PATCH 6, its initializer is garbage, and ...
> 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);
> + curr = &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(curr, gsconf);
... it is used only from the second list element on. Okay, I see why
this is not in PATCH 6.
> gsconf = NULL;
>
> qobject_unref(backing_options);
[...]
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index ca39d15d93a2..711814be2312 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
[...]
> @@ -294,41 +281,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_APPEND(list, m);
The old code prepends, doesn't it?
> }
>
> return 0;
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index cf0627fd01c1..1afcc29a0649 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, **prev = &list;
Here, you reuse the old name for the new variable.
>
> 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(prev, info);
> }
>
> g_slist_free(devices);
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 0131d9d02c16..43f19e4ab219 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1681,41 +1681,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;
Here, you use a new and better name.
I'd like to encourage you to name tail pointer variables @tail
elsewhere, too.
> 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;
[...]
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index d8bc40ea9f6e..55087e268cda 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2118,17 +2118,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,
> @@ -2197,7 +2197,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) {
> @@ -2206,9 +2206,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;
> @@ -2222,19 +2223,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.
> /* we haven't obtained HW address yet */
> sock = socket(PF_INET, SOCK_STREAM, 0);
> if (sock == -1) {
> @@ -2243,7 +2239,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",
> @@ -2255,13 +2251,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 &&
> @@ -2274,15 +2270,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) {
> @@ -2294,15 +2289,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]) +
> @@ -2314,29 +2308,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;
> }
> }
> }
> @@ -2863,7 +2846,6 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
>
> while (mem_blks != NULL) {
> GuestMemoryBlockResponse *result;
> - GuestMemoryBlockResponseList *entry;
> GuestMemoryBlock *current_mem_blk = mem_blks->value;
>
> result = g_malloc0(sizeof(*result));
> @@ -2872,11 +2854,7 @@ 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(link, result);
> mem_blks = mem_blks->next;
> }
>
This one looks like a candidate for PATCH 6.
> @@ -3107,11 +3085,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);
> @@ -3134,19 +3111,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);
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND
2020-11-19 8:50 ` Markus Armbruster
@ 2020-12-04 22:54 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2020-12-04 22:54 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, open list:GLUSTER, Eduardo Habkost,
open list:GLUSTER, Michael S. Tsirkin, Jason Wang, Juan Quintela,
qemu-devel, Max Reitz, Gerd Hoffmann, Marc-André Lureau,
Igor Mammedov, Michael Roth, Dr. David Alan Gilbert
On 11/19/20 2:50 AM, Markus Armbruster wrote:
> 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.
>> +++ b/hw/core/machine-qmp-cmds.c
> [...]
>> @@ -294,41 +281,31 @@ void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>> static int query_memdev(Object *obj, void *opaque)
>> 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_APPEND(list, m);
>
> The old code prepends, doesn't it?
Good catch, will correct and hoist this into 4/7 for v3.
>
>> }
>>
>> return 0;
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index cf0627fd01c1..1afcc29a0649 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, **prev = &list;
>
> Here, you reuse the old name for the new variable.
>> +++ b/hw/pci/pci.c
>> @@ -1681,41 +1681,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;
>
> Here, you use a new and better name.
>
> I'd like to encourage you to name tail pointer variables @tail
> elsewhere, too.
In v3, I will consistently rename the FOOList ** variable 'tail'.
>> @@ -2863,7 +2846,6 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
>>
>> while (mem_blks != NULL) {
>> GuestMemoryBlockResponse *result;
>> - GuestMemoryBlockResponseList *entry;
>> GuestMemoryBlock *current_mem_blk = mem_blks->value;
>>
>> result = g_malloc0(sizeof(*result));
>> @@ -2872,11 +2854,7 @@ 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(link, result);
>> mem_blks = mem_blks->next;
>> }
>>
>
> This one looks like a candidate for PATCH 6.
Yes. Will hoist.
v3 will be posted soon.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/7] Common macros for QAPI list growth
2020-11-13 1:13 [PATCH v2 0/7] Common macros for QAPI list growth Eric Blake
` (6 preceding siblings ...)
2020-11-13 1:13 ` [PATCH v2 7/7] qapi: More complex uses of QAPI_LIST_APPEND Eric Blake
@ 2020-11-19 9:28 ` Markus Armbruster
2020-12-19 9:43 ` Markus Armbruster
7 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2020-11-19 9:28 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
Eric Blake <eblake@redhat.com> writes:
> v1, as such, was here:
> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg08003.html
> (v6 11/11 qapi: Use QAPI_LIST_ADD() where possible)
>
> since then, I've rebased that patch (upstream went with PREPEND
> instead of ADD), split things out for easier review, added
> QAPI_LIST_APPEND, caught a lot more places that can use PREPEND, and
> even fixed a years-old memory leak that might be worth having in 5.2.
> But patches 2-7 are 6.0 material.
[...]
> 55 files changed, 431 insertions(+), 1007 deletions(-)
Lovely. These two macros are obviously overdue.
Series needs a rebase.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 0/7] Common macros for QAPI list growth
2020-11-19 9:28 ` [PATCH v2 0/7] Common macros for QAPI list growth Markus Armbruster
@ 2020-12-19 9:43 ` Markus Armbruster
0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2020-12-19 9:43 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> Eric Blake <eblake@redhat.com> writes:
>
>> v1, as such, was here:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg08003.html
>> (v6 11/11 qapi: Use QAPI_LIST_ADD() where possible)
>>
>> since then, I've rebased that patch (upstream went with PREPEND
>> instead of ADD), split things out for easier review, added
>> QAPI_LIST_APPEND, caught a lot more places that can use PREPEND, and
>> even fixed a years-old memory leak that might be worth having in 5.2.
>> But patches 2-7 are 6.0 material.
> [...]
>> 55 files changed, 431 insertions(+), 1007 deletions(-)
>
> Lovely. These two macros are obviously overdue.
I'm queuing PATCH 2-4. We concluced PATCH 1 isn't necessary. PATCH 7
needs work, and I gather you'd like to improve PATCH 5 (parenthesis in
macro expansion) and 6 (use the opportunity to improve variable naming).
> Series needs a rebase.
The conflicts are due to
a8aa94b5f8 qga: update schema for guest-get-disks 'dependents' field
a10b453a52 target/mips: Move mips_cpu_add_definition() from helper.c to cpu.c
and easy enough to resolve for me.
Thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread