From fe4d7e338faf7809024017e373b15dd8ac82094d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 12 Nov 2020 19:13:35 -0600 Subject: [PATCH 01/33] rocker: Revamp fp_port_get_info 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 Message-Id: <20201113011340.463563-3-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- hw/net/rocker/rocker.c | 8 +------- hw/net/rocker/rocker_fp.c | 17 ++++++++++------- hw/net/rocker/rocker_fp.h | 2 +- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c index 1af1e6fa2f..c53a02315e 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 4aa7da79b8..cbeed65bd5 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) diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h index dbe1dd329a..7ff57aac01 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); From eaedde5255842e8add96bec127567e2a8b3be9a8 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 12 Nov 2020 19:13:36 -0600 Subject: [PATCH 02/33] migration: Refactor migrate_cap_add 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 Message-Id: <20201113011340.463563-4-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- migration/migration.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index e0dbde4091..ccf969a62e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1654,27 +1654,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); } @@ -3863,7 +3859,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)); } } From 54aa3de72ea2aaa2e903e7e879a4f3dda515a00e Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 12 Nov 2020 19:13:37 -0600 Subject: [PATCH 03/33] qapi: Use QAPI_LIST_PREPEND() where possible Anywhere we create a list of just one item or by prepending items (typically because order doesn't matter), we can use QAPI_LIST_PREPEND(). 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 Message-Id: <20201113011340.463563-5-eblake@redhat.com> Reviewed-by: Markus Armbruster Acked-by: Stefan Hajnoczi [Straightforward conflicts due to commit a8aa94b5f8 "qga: update schema for guest-get-disks 'dependents' field" and commit a10b453a52 "target/mips: Move mips_cpu_add_definition() from helper.c to cpu.c" resolved. Commit message tweaked.] Signed-off-by: Markus Armbruster --- block/gluster.c | 4 +- block/qapi.c | 7 +- chardev/char.c | 20 ++--- docs/devel/writing-qmp-commands.txt | 12 +-- 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 | 47 +++--------- qga/commands-win32.c | 32 ++------ qga/commands.c | 6 +- qom/qom-qmp-cmds.c | 29 ++----- target/arm/helper.c | 6 +- target/arm/monitor.c | 13 +--- target/i386/cpu.c | 6 +- target/mips/cpu.c | 6 +- target/ppc/translate_init.c.inc | 12 +-- 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 +--- 32 files changed, 158 insertions(+), 421 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index 4f1448e2bc..1f8699b938 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 036da085ee..0ca206f559 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 a9b8c5a9aa..288efebd12 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/docs/devel/writing-qmp-commands.txt b/docs/devel/writing-qmp-commands.txt index 28984686c9..258e63bff5 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/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index 87f14140a3..affffe0c4a 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 05dcaf09c9..de3b8f1b31 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -504,11 +504,7 @@ static void machine_set_nvdimm_persistence(Object *obj, const char *value, void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type) { - strList *item = g_new0(strList, 1); - - item->value = g_strdup(type); - item->next = mc->allowed_dynamic_sysbus_devices; - mc->allowed_dynamic_sysbus_devices = item; + QAPI_LIST_PREPEND(mc->allowed_dynamic_sysbus_devices, g_strdup(type)); } static void validate_sysbus_device(SysBusDevice *sbdev, void *opaque) @@ -569,7 +565,6 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine) for (i = 0; i < machine->possible_cpus->len; i++) { Object *cpu; - HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1); HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1); cpu_item->type = g_strdup(machine->possible_cpus->cpus[i].type); @@ -582,9 +577,7 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine) cpu_item->has_qom_path = true; cpu_item->qom_path = object_get_canonical_path(cpu); } - list_item->value = cpu_item; - list_item->next = head; - head = list_item; + QAPI_LIST_PREPEND(head, cpu_item); } return head; } diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c index 8e347d1ee4..b3b8c5bb6d 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 044ac95f6f..8356eeec13 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 ccf969a62e..3f4191a9c7 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -406,12 +406,9 @@ int migration_incoming_enable_colo(void) void migrate_add_address(SocketAddress *address) { MigrationIncomingState *mis = migration_incoming_get_current(); - SocketAddressList *addrs; - addrs = g_new0(SocketAddressList, 1); - addrs->next = mis->socket_address_list; - mis->socket_address_list = addrs; - addrs->value = QAPI_CLONE(SocketAddress, address); + QAPI_LIST_PREPEND(mis->socket_address_list, + QAPI_CLONE(SocketAddress, address)); } static void qemu_start_incoming_migration(const char *uri, Error **errp) diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index d99842eb1b..ab482adef1 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 65d8ff4849..73b79df7d8 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1255,7 +1255,8 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict) const char *cap = qdict_get_str(qdict, "capability"); bool state = qdict_get_bool(qdict, "state"); Error *err = NULL; - MigrationCapabilityStatusList *caps = g_malloc0(sizeof(*caps)); + MigrationCapabilityStatusList *caps = NULL; + MigrationCapabilityStatus *value; int val; val = qapi_enum_parse(&MigrationCapability_lookup, cap, -1, &err); @@ -1263,14 +1264,14 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict) goto end; } - caps->value = g_malloc0(sizeof(*caps->value)); - caps->value->capability = val; - caps->value->state = state; - caps->next = NULL; + value = g_malloc0(sizeof(*value)); + value->capability = val; + value->state = state; + QAPI_LIST_PREPEND(caps, value); qmp_migrate_set_capabilities(caps, &err); + qapi_free_MigrationCapabilityStatusList(caps); end: - qapi_free_MigrationCapabilityStatusList(caps); hmp_handle_error(mon, err); } diff --git a/monitor/misc.c b/monitor/misc.c index fde6e36a0b..f2ee7cd77a 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -1430,33 +1430,26 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) QEMU_LOCK_GUARD(&mon_fdsets_lock); QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { - FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info)); - FdsetFdInfoList *fdsetfd_list = NULL; + FdsetInfo *fdset_info = g_malloc0(sizeof(*fdset_info)); - fdset_info->value = g_malloc0(sizeof(*fdset_info->value)); - fdset_info->value->fdset_id = mon_fdset->id; + fdset_info->fdset_id = mon_fdset->id; QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { - FdsetFdInfoList *fdsetfd_info; + FdsetFdInfo *fdsetfd_info; fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info)); - fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value)); - fdsetfd_info->value->fd = mon_fdset_fd->fd; + fdsetfd_info->fd = mon_fdset_fd->fd; if (mon_fdset_fd->opaque) { - fdsetfd_info->value->has_opaque = true; - fdsetfd_info->value->opaque = g_strdup(mon_fdset_fd->opaque); + fdsetfd_info->has_opaque = true; + fdsetfd_info->opaque = g_strdup(mon_fdset_fd->opaque); } else { - fdsetfd_info->value->has_opaque = false; + fdsetfd_info->has_opaque = false; } - fdsetfd_info->next = fdsetfd_list; - fdsetfd_list = fdsetfd_info; + QAPI_LIST_PREPEND(fdset_info->fds, fdsetfd_info); } - fdset_info->value->fds = fdsetfd_list; - - fdset_info->next = fdset_list; - fdset_list = fdset_info; + QAPI_LIST_PREPEND(fdset_list, fdset_info); } return fdset_list; diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c index a456762f6a..17514f4959 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 8bdea40b58..d599659c7f 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 749167e82d..2dda136d64 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 c089e38120..849923b260 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1036,7 +1036,6 @@ static void build_guest_fsinfo_for_real_device(char const *syspath, { GuestDiskAddress *disk; GuestPCIAddress *pciaddr; - GuestDiskAddressList *list = NULL; bool has_hwinf; #ifdef CONFIG_LIBUDEV struct udev *udev = NULL; @@ -1053,9 +1052,6 @@ static void build_guest_fsinfo_for_real_device(char const *syspath, disk->pci_controller = pciaddr; disk->bus_type = GUEST_DISK_BUS_TYPE_UNKNOWN; - list = g_new0(GuestDiskAddressList, 1); - list->value = disk; - #ifdef CONFIG_LIBUDEV udev = udev_new(); udevice = udev_device_new_from_syspath(udev, syspath); @@ -1089,10 +1085,9 @@ static void build_guest_fsinfo_for_real_device(char const *syspath, } if (has_hwinf || disk->has_dev || disk->has_serial) { - list->next = fs->disk; - fs->disk = list; + QAPI_LIST_PREPEND(fs->disk, disk); } else { - qapi_free_GuestDiskAddressList(list); + qapi_free_GuestDiskAddress(disk); } } @@ -1288,7 +1283,6 @@ static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk) disk->has_dependencies = true; while ((dep = g_dir_read_name(dp_deps)) != NULL) { g_autofree char *dep_dir = NULL; - strList *dep_item = NULL; char *dev_name; /* Add dependent disks */ @@ -1296,10 +1290,7 @@ static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk) dev_name = get_device_for_syspath(dep_dir); if (dev_name != NULL) { g_debug(" adding dependent device: %s", dev_name); - dep_item = g_new0(strList, 1); - dep_item->value = dev_name; - dep_item->next = disk->dependencies; - disk->dependencies = dep_item; + QAPI_LIST_PREPEND(disk->dependencies, dev_name); } } g_dir_close(dp_deps); @@ -1318,7 +1309,7 @@ static GuestDiskInfoList *get_disk_partitions( const char *disk_name, const char *disk_dir, const char *disk_dev) { - GuestDiskInfoList *item, *ret = list; + GuestDiskInfoList *ret = list; struct dirent *de_disk; DIR *dp_disk = NULL; size_t len = strlen(disk_name); @@ -1352,15 +1343,9 @@ static GuestDiskInfoList *get_disk_partitions( partition->name = dev_name; partition->partition = true; /* Add parent disk as dependent for easier tracking of hierarchy */ - partition->dependencies = g_new0(strList, 1); - partition->dependencies->value = g_strdup(disk_dev); - partition->has_dependencies = true; - - item = g_new0(GuestDiskInfoList, 1); - item->value = partition; - item->next = ret; - ret = item; + QAPI_LIST_PREPEND(partition->dependencies, g_strdup(disk_dev)); + QAPI_LIST_PREPEND(ret, partition); } closedir(dp_disk); @@ -1369,7 +1354,7 @@ static GuestDiskInfoList *get_disk_partitions( GuestDiskInfoList *qmp_guest_get_disks(Error **errp) { - GuestDiskInfoList *item, *ret = NULL; + GuestDiskInfoList *ret = NULL; GuestDiskInfo *disk; DIR *dp = NULL; struct dirent *de = NULL; @@ -1415,10 +1400,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp) disk->partition = false; disk->alias = get_alias_for_syspath(disk_dir); disk->has_alias = (disk->alias != NULL); - item = g_new0(GuestDiskInfoList, 1); - item->value = disk; - item->next = ret; - ret = item; + QAPI_LIST_PREPEND(ret, disk); /* Get address for non-virtual devices */ bool is_virtual = is_disk_virtual(disk_dir, &local_err); @@ -1495,7 +1477,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp) { FsMountList mounts; struct FsMount *mount; - GuestFilesystemInfoList *new, *ret = NULL; + GuestFilesystemInfoList *ret = NULL; Error *local_err = NULL; QTAILQ_INIT(&mounts); @@ -1508,10 +1490,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp) QTAILQ_FOREACH(mount, &mounts, next) { g_debug("Building guest fsinfo for '%s'", mount->dirname); - new = g_malloc0(sizeof(*ret)); - new->value = build_guest_fsinfo(mount, &local_err); - new->next = ret; - ret = new; + QAPI_LIST_PREPEND(ret, build_guest_fsinfo(mount, &local_err)); if (local_err) { error_propagate(errp, local_err); qapi_free_GuestFilesystemInfoList(ret); @@ -1777,7 +1756,6 @@ GuestFilesystemTrimResponse * qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) { GuestFilesystemTrimResponse *response; - GuestFilesystemTrimResultList *list; GuestFilesystemTrimResult *result; int ret = 0; FsMountList mounts; @@ -1801,10 +1779,7 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) result = g_malloc0(sizeof(*result)); result->path = g_strdup(mount->dirname); - list = g_malloc0(sizeof(*list)); - list->value = result; - list->next = response->paths; - response->paths = list; + QAPI_LIST_PREPEND(response->paths, result); fd = qemu_open_old(mount->dirname, O_RDONLY); if (fd == -1) { diff --git a/qga/commands-win32.c b/qga/commands-win32.c index ba1fd07d06..684639bd13 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 3dcd5fbe5c..e866fc7081 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 2dd233f293..b40ac39f30 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -46,14 +46,12 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) object_property_iter_init(&iter, obj); while ((prop = object_property_iter_next(&iter))) { - ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry)); + ObjectPropertyInfo *value = g_malloc0(sizeof(ObjectPropertyInfo)); - entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); - entry->next = props; - props = entry; + QAPI_LIST_PREPEND(props, value); - entry->value->name = g_strdup(prop->name); - entry->value->type = g_strdup(prop->type); + value->name = g_strdup(prop->name); + value->type = g_strdup(prop->type); } return props; @@ -90,7 +88,7 @@ QObject *qmp_qom_get(const char *path, const char *property, Error **errp) static void qom_list_types_tramp(ObjectClass *klass, void *data) { - ObjectTypeInfoList *e, **pret = data; + ObjectTypeInfoList **pret = data; ObjectTypeInfo *info; ObjectClass *parent = object_class_get_parent(klass); @@ -102,10 +100,7 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data) info->parent = g_strdup(object_class_get_name(parent)); } - e = g_malloc0(sizeof(*e)); - e->value = info; - e->next = *pret; - *pret = e; + QAPI_LIST_PREPEND(*pret, info); } ObjectTypeInfoList *qmp_qom_list_types(bool has_implements, @@ -150,7 +145,6 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename, object_property_iter_init(&iter, obj); while ((prop = object_property_iter_next(&iter))) { ObjectPropertyInfo *info; - ObjectPropertyInfoList *entry; /* Skip Object and DeviceState properties */ if (strcmp(prop->name, "type") == 0 || @@ -176,10 +170,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename, info->default_value = qobject_ref(prop->defval); info->has_default_value = !!info->default_value; - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = prop_list; - prop_list = entry; + QAPI_LIST_PREPEND(prop_list, info); } object_unref(obj); @@ -217,7 +208,6 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename, } while ((prop = object_property_iter_next(&iter))) { ObjectPropertyInfo *info; - ObjectPropertyInfoList *entry; info = g_malloc0(sizeof(*info)); info->name = g_strdup(prop->name); @@ -225,10 +215,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename, info->has_description = !!prop->description; info->description = g_strdup(prop->description); - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = prop_list; - prop_list = entry; + QAPI_LIST_PREPEND(prop_list, info); } object_unref(obj); diff --git a/target/arm/helper.c b/target/arm/helper.c index 7b8bcd6903..2d0d4cd1e1 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 169d8a64b6..198b14e95e 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 0d20e156f2..35459a38bb 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5014,7 +5014,6 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) ObjectClass *oc = data; X86CPUClass *cc = X86_CPU_CLASS(oc); CpuDefinitionInfoList **cpu_list = user_data; - CpuDefinitionInfoList *entry; CpuDefinitionInfo *info; info = g_malloc0(sizeof(*info)); @@ -5039,10 +5038,7 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) info->has_alias_of = !!info->alias_of; } - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = *cpu_list; - *cpu_list = entry; + QAPI_LIST_PREPEND(*cpu_list, info); } CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) diff --git a/target/mips/cpu.c b/target/mips/cpu.c index aadc6f8e74..b2cd69ff7f 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -543,7 +543,6 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data) { ObjectClass *oc = data; CpuDefinitionInfoList **cpu_list = user_data; - CpuDefinitionInfoList *entry; CpuDefinitionInfo *info; const char *typename; @@ -553,10 +552,7 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data) strlen(typename) - strlen("-" TYPE_MIPS_CPU)); info->q_typename = g_strdup(typename); - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = *cpu_list; - *cpu_list = entry; + QAPI_LIST_PREPEND(*cpu_list, info); } CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc index a4d0038828..3c05a17343 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/translate_init.c.inc @@ -10566,7 +10566,6 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data) ObjectClass *oc = data; CpuDefinitionInfoList **first = user_data; const char *typename; - CpuDefinitionInfoList *entry; CpuDefinitionInfo *info; typename = object_class_get_name(oc); @@ -10574,10 +10573,7 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data) info->name = g_strndup(typename, strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX)); - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = *first; - *first = entry; + QAPI_LIST_PREPEND(*first, info); } CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) @@ -10593,7 +10589,6 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) { PowerPCCPUAlias *alias = &ppc_cpu_aliases[i]; ObjectClass *oc; - CpuDefinitionInfoList *entry; CpuDefinitionInfo *info; oc = ppc_cpu_class_by_name(alias->model); @@ -10605,10 +10600,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) info->name = g_strdup(alias->alias); info->q_typename = g_strdup(object_class_get_name(oc)); - entry = g_malloc0(sizeof(*entry)); - entry->value = info; - entry->next = cpu_list; - cpu_list = entry; + QAPI_LIST_PREPEND(cpu_list, info); } return cpu_list; diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index b5abff8bef..a23fd3e32b 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 5e1e8b2f5e..4944b3d857 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 1c856d9bd2..b20ab8b29b 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 1c5a8b94ea..12275e56d8 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 38246e1aa6..85f81e47cc 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 4791b089c7..8ac407dec4 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 34e6dc437c..7452ac7df2 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 725e3d7e4b..e2a700b284 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); } } From 3953f826a3ff09a6b71b0365c05d1d3f9fdf49f2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:43 +0100 Subject: [PATCH 04/33] tests/check-qjson: Don't skip funny QNumber to JSON conversions simple_number() and float_number() convert from JSON to QNumber and back. simple_number() tests "-0", but skips the conversion back to JSON, because it yields "0", not "-0". Works as intended, so better cover it: don't skip, but expect the funny result. float_number() tests "-32.20e-10", but skips the conversion back to JSON, because it yields "-0". This is a known bug in qnum_to_string(), marked FIXME there. Cover the bug: don't skip, but expect the funny result. While there, switch from g_assert() to g_assert_cmpstr() & friends for friendlier test failures. Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-2-armbru@redhat.com> --- tests/check-qjson.c | 55 +++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 9a02079099..2a5852904a 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -793,37 +793,35 @@ static void utf8_string(void) static void simple_number(void) { - int i; struct { const char *encoded; int64_t decoded; - int skip; + const char *reencoded; } test_cases[] = { { "0", 0 }, { "1234", 1234 }, { "1", 1 }, { "-32", -32 }, - { "-0", 0, .skip = 1 }, - { }, + { "-0", 0, "0" }, + {}, }; + int i; + QNum *qnum; + int64_t val; + QString *str; for (i = 0; test_cases[i].encoded; i++) { - QNum *qnum; - int64_t val; - qnum = qobject_to(QNum, qobject_from_json(test_cases[i].encoded, &error_abort)); g_assert(qnum); g_assert(qnum_get_try_int(qnum, &val)); g_assert_cmpint(val, ==, test_cases[i].decoded); - if (test_cases[i].skip == 0) { - QString *str; - str = qobject_to_json(QOBJECT(qnum)); - g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0); - qobject_unref(str); - } + str = qobject_to_json(QOBJECT(qnum)); + g_assert_cmpstr(qstring_get_str(str), ==, + test_cases[i].reencoded ?: test_cases[i].encoded); + qobject_unref(str); qobject_unref(qnum); } @@ -874,35 +872,32 @@ static void large_number(void) static void float_number(void) { - int i; struct { const char *encoded; double decoded; - int skip; + const char *reencoded; } test_cases[] = { { "32.43", 32.43 }, { "0.222", 0.222 }, { "-32.12313", -32.12313 }, - { "-32.20e-10", -32.20e-10, .skip = 1 }, - { }, + { "-32.20e-10", -32.20e-10, "-0" /* BUG */ }, + {}, }; + int i; + QNum *qnum; + QString *str; for (i = 0; test_cases[i].encoded; i++) { - QObject *obj; - QNum *qnum; - - obj = qobject_from_json(test_cases[i].encoded, &error_abort); - qnum = qobject_to(QNum, obj); + qnum = qobject_to(QNum, + qobject_from_json(test_cases[i].encoded, + &error_abort)); g_assert(qnum); - g_assert(qnum_get_double(qnum) == test_cases[i].decoded); + g_assert_cmpfloat(qnum_get_double(qnum), ==, test_cases[i].decoded); - if (test_cases[i].skip == 0) { - QString *str; - - str = qobject_to_json(obj); - g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0); - qobject_unref(str); - } + str = qobject_to_json(QOBJECT(qnum)); + g_assert_cmpstr(qstring_get_str(str), ==, + test_cases[i].reencoded ?: test_cases[i].encoded); + qobject_unref(str); qobject_unref(qnum); } From 1a68eb8c186d81a2836bd1e9abd03c0b39f252e9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:44 +0100 Subject: [PATCH 05/33] tests/check-qjson: Examine QNum more thoroughly simple_number() checks only qnum_get_try_int(). Also check qnum_get_try_uint() and qnum_get_double(). float_number() checks only qnum_get_double(). Also check qnum_get_try_int() and qnum_get_try_uint(). Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-3-armbru@redhat.com> --- tests/check-qjson.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 2a5852904a..6ab6b111b0 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -807,7 +807,8 @@ static void simple_number(void) }; int i; QNum *qnum; - int64_t val; + int64_t ival; + uint64_t uval; QString *str; for (i = 0; test_cases[i].encoded; i++) { @@ -815,8 +816,16 @@ static void simple_number(void) qobject_from_json(test_cases[i].encoded, &error_abort)); g_assert(qnum); - g_assert(qnum_get_try_int(qnum, &val)); - g_assert_cmpint(val, ==, test_cases[i].decoded); + g_assert(qnum_get_try_int(qnum, &ival)); + g_assert_cmpint(ival, ==, test_cases[i].decoded); + if (test_cases[i].decoded >= 0) { + g_assert(qnum_get_try_uint(qnum, &uval)); + g_assert_cmpuint(uval, ==, (uint64_t)test_cases[i].decoded); + } else { + g_assert(!qnum_get_try_uint(qnum, &uval)); + } + g_assert_cmpfloat(qnum_get_double(qnum), ==, + (double)test_cases[i].decoded); str = qobject_to_json(QOBJECT(qnum)); g_assert_cmpstr(qstring_get_str(str), ==, @@ -885,6 +894,8 @@ static void float_number(void) }; int i; QNum *qnum; + int64_t ival; + uint64_t uval; QString *str; for (i = 0; test_cases[i].encoded; i++) { @@ -893,6 +904,8 @@ static void float_number(void) &error_abort)); g_assert(qnum); g_assert_cmpfloat(qnum_get_double(qnum), ==, test_cases[i].decoded); + g_assert(!qnum_get_try_int(qnum, &ival)); + g_assert(!qnum_get_try_uint(qnum, &uval)); str = qobject_to_json(QOBJECT(qnum)); g_assert_cmpstr(qstring_get_str(str), ==, From 4aea88335d5c824b7b63b73c12830b5db558d463 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:45 +0100 Subject: [PATCH 06/33] tests/check-qjson: Cover number 2^63 Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-4-armbru@redhat.com> --- tests/check-qjson.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 6ab6b111b0..8cb8a40524 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -791,7 +791,7 @@ static void utf8_string(void) } } -static void simple_number(void) +static void int_number(void) { struct { const char *encoded; @@ -836,6 +836,42 @@ static void simple_number(void) } } +static void uint_number(void) +{ + struct { + const char *encoded; + uint64_t decoded; + const char *reencoded; + } test_cases[] = { + { "9223372036854775808", (uint64_t)1 << 63 }, + {}, + }; + int i; + QNum *qnum; + int64_t ival; + uint64_t uval; + QString *str; + + for (i = 0; test_cases[i].encoded; i++) { + qnum = qobject_to(QNum, + qobject_from_json(test_cases[i].encoded, + &error_abort)); + g_assert(qnum); + g_assert(qnum_get_try_uint(qnum, &uval)); + g_assert_cmpuint(uval, ==, test_cases[i].decoded); + g_assert(!qnum_get_try_int(qnum, &ival)); + g_assert_cmpfloat(qnum_get_double(qnum), ==, + (double)test_cases[i].decoded); + + str = qobject_to_json(QOBJECT(qnum)); + g_assert_cmpstr(qstring_get_str(str), ==, + test_cases[i].reencoded ?: test_cases[i].encoded); + qobject_unref(str); + + qobject_unref(qnum); + } +} + static void large_number(void) { const char *maxu64 = "18446744073709551615"; /* 2^64-1 */ @@ -1487,7 +1523,8 @@ int main(int argc, char **argv) g_test_add_func("/literals/string/quotes", string_with_quotes); g_test_add_func("/literals/string/utf8", utf8_string); - g_test_add_func("/literals/number/simple", simple_number); + g_test_add_func("/literals/number/int", int_number); + g_test_add_func("/literals/number/uint", uint_number); g_test_add_func("/literals/number/large", large_number); g_test_add_func("/literals/number/float", float_number); From 780df5d42befde9293cd667e1e237f26bcf37e94 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:46 +0100 Subject: [PATCH 07/33] tests/check-qjson: Replace redundant large_number() Move one of large_number()'s three checks to uint_number(), and the other two to float_number(). Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-5-armbru@redhat.com> --- tests/check-qjson.c | 47 +++------------------------------------------ 1 file changed, 3 insertions(+), 44 deletions(-) diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 8cb8a40524..98515b1fd6 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -844,6 +844,7 @@ static void uint_number(void) const char *reencoded; } test_cases[] = { { "9223372036854775808", (uint64_t)1 << 63 }, + { "18446744073709551615", UINT64_MAX }, {}, }; int i; @@ -872,49 +873,6 @@ static void uint_number(void) } } -static void large_number(void) -{ - const char *maxu64 = "18446744073709551615"; /* 2^64-1 */ - const char *gtu64 = "18446744073709551616"; /* 2^64 */ - const char *lti64 = "-9223372036854775809"; /* -2^63 - 1 */ - QNum *qnum; - QString *str; - uint64_t val; - int64_t ival; - - qnum = qobject_to(QNum, qobject_from_json(maxu64, &error_abort)); - g_assert(qnum); - g_assert_cmpuint(qnum_get_uint(qnum), ==, 18446744073709551615U); - g_assert(!qnum_get_try_int(qnum, &ival)); - - str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, maxu64); - qobject_unref(str); - qobject_unref(qnum); - - qnum = qobject_to(QNum, qobject_from_json(gtu64, &error_abort)); - g_assert(qnum); - g_assert_cmpfloat(qnum_get_double(qnum), ==, 18446744073709552e3); - g_assert(!qnum_get_try_uint(qnum, &val)); - g_assert(!qnum_get_try_int(qnum, &ival)); - - str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, gtu64); - qobject_unref(str); - qobject_unref(qnum); - - qnum = qobject_to(QNum, qobject_from_json(lti64, &error_abort)); - g_assert(qnum); - g_assert_cmpfloat(qnum_get_double(qnum), ==, -92233720368547758e2); - g_assert(!qnum_get_try_uint(qnum, &val)); - g_assert(!qnum_get_try_int(qnum, &ival)); - - str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, "-9223372036854775808"); - qobject_unref(str); - qobject_unref(qnum); -} - static void float_number(void) { struct { @@ -926,6 +884,8 @@ static void float_number(void) { "0.222", 0.222 }, { "-32.12313", -32.12313 }, { "-32.20e-10", -32.20e-10, "-0" /* BUG */ }, + { "18446744073709551616", 0x1p64 }, + { "-9223372036854775809", -0x1p63, "-9223372036854775808" }, {}, }; int i; @@ -1525,7 +1485,6 @@ int main(int argc, char **argv) g_test_add_func("/literals/number/int", int_number); g_test_add_func("/literals/number/uint", uint_number); - g_test_add_func("/literals/number/large", large_number); g_test_add_func("/literals/number/float", float_number); g_test_add_func("/literals/keyword", keyword_literal); From 1a9076919f5367309ee8d89b91aae5330dec37d6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:47 +0100 Subject: [PATCH 08/33] tests/check-qnum: Cover qnum_to_string() for "unround" argument qnum_to_string() has a FIXME comment about rounding errors due to insufficient precision. Cover it: 2.718281828459045 gets converted to "2.718282". The next commit will fix it. Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-6-armbru@redhat.com> --- tests/check-qnum.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/check-qnum.c b/tests/check-qnum.c index 4105015872..a73809d021 100644 --- a/tests/check-qnum.c +++ b/tests/check-qnum.c @@ -150,6 +150,12 @@ static void qnum_to_string_test(void) g_assert_cmpstr(tmp, ==, "0.42"); g_free(tmp); qobject_unref(qn); + + qn = qnum_from_double(2.718281828459045); + tmp = qnum_to_string(qn); + g_assert_cmpstr(tmp, ==, "2.718282"); /* BUG */ + g_free(tmp); + qobject_unref(qn); } int main(int argc, char **argv) From f917eed3069640f6fa15f07cc5a61ecf4270e6a3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:48 +0100 Subject: [PATCH 09/33] qobject: Fix qnum_to_string() to use sufficient precision We should serialize numbers to JSON so that they deserialize back to the same number. We fail to do so. The culprit is qnum_to_string(): it uses format %f with trailing '0' trimmed. Results in pretty output for "nice" numbers, but is prone to nasty rounding errors. For instance, numbers between 0 and 0.0000005 get flushed to zero. Where exactly the incorrect rounding can bite is tiresome to gauge. Here's my take. * In QMP output, type 'number': - query-blockstats value avg_rd_queue_depth - QMP query-migrate values mbps, cache-miss-rate, encoding-rate, busy-rate, compression-rate. Relatively harmless, I guess. * In tracing QMP input. Harmless. * In qemu-ga output, type 'number': guest-get-users value login-time. Harmless. * In output of HMP qom-get. Harmless. Not affected, because double values don't actually occur there (I think): * QMP output, type 'any': * qom-get value * qom-list, qom-list-properties value default-value * query-cpu-model-comparison, query-cpu-model-baseline, query-cpu-model-expansion value props. * qemu-img --output json output. * "json:" pseudo-filenames generated by bdrv_refresh_filename(). * The rbd block driver's "=keyvalue-pairs" hack. * In -object help on property default values. Aside: use of JSON feels inappropriate here. * Output of HMP qom-get. * Argument conversion to QemuOpts for qdev_device_add() and HMP with qemu_opts_from_qdict() QMP and HMP device_add, virtio-net failover primary creation, xen-usb "usb-host" creation, HMP netdev_add, object_add. * The uses of qobject_input_visitor_new_flat_confused() As far as I can tell, none of the visited types contain double values. * Dumping ImageInfoSpecific with dump_qobject() Fix by formatting with %.17g. 17 decimal digits always suffice for IEEE double. The change to expected test output illustrates the effect: the rounding errors are gone, but some seemingly "nice" numbers now get converted to not so nice strings, e.g. 0.42 to "0.41999999999999998". This is because 0.42 is not representable exactly in double. It's more accurate in this example than strictly necessary, though. If ugly accuracy bothers us, we can we can try using the least number of digits that still converts back to the same double. In this example, "0.42" would do. Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-7-armbru@redhat.com> --- qobject/qnum.c | 24 +++--------------------- tests/check-qjson.c | 8 ++++---- tests/check-qnum.c | 4 ++-- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/qobject/qnum.c b/qobject/qnum.c index 7012fc57f2..bf1240ecec 100644 --- a/qobject/qnum.c +++ b/qobject/qnum.c @@ -161,37 +161,19 @@ double qnum_get_double(QNum *qn) char *qnum_to_string(QNum *qn) { - char *buffer; - int len; - switch (qn->kind) { case QNUM_I64: return g_strdup_printf("%" PRId64, qn->u.i64); case QNUM_U64: return g_strdup_printf("%" PRIu64, qn->u.u64); case QNUM_DOUBLE: - /* FIXME: snprintf() is locale dependent; but JSON requires + /* FIXME: g_strdup_printf() is locale dependent; but JSON requires * numbers to be formatted as if in the C locale. Dependence * on C locale is a pervasive issue in QEMU. */ /* FIXME: This risks printing Inf or NaN, which are not valid * JSON values. */ - /* FIXME: the default precision of 6 for %f often causes - * rounding errors; we should be using DBL_DECIMAL_DIG (17), - * and only rounding to a shorter number if the result would - * still produce the same floating point value. */ - buffer = g_strdup_printf("%f" , qn->u.dbl); - len = strlen(buffer); - while (len > 0 && buffer[len - 1] == '0') { - len--; - } - - if (len && buffer[len - 1] == '.') { - buffer[len - 1] = 0; - } else { - buffer[len] = 0; - } - - return buffer; + /* 17 digits suffice for IEEE double */ + return g_strdup_printf("%.17g", qn->u.dbl); } assert(0); diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 98515b1fd6..ca8fb816e9 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -882,10 +882,10 @@ static void float_number(void) } test_cases[] = { { "32.43", 32.43 }, { "0.222", 0.222 }, - { "-32.12313", -32.12313 }, - { "-32.20e-10", -32.20e-10, "-0" /* BUG */ }, - { "18446744073709551616", 0x1p64 }, - { "-9223372036854775809", -0x1p63, "-9223372036854775808" }, + { "-32.12313", -32.12313, "-32.123130000000003" }, + { "-32.20e-10", -32.20e-10, "-3.22e-09" }, + { "18446744073709551616", 0x1p64, "1.8446744073709552e+19" }, + { "-9223372036854775809", -0x1p63, "-9.2233720368547758e+18" }, {}, }; int i; diff --git a/tests/check-qnum.c b/tests/check-qnum.c index a73809d021..b85fca2302 100644 --- a/tests/check-qnum.c +++ b/tests/check-qnum.c @@ -147,13 +147,13 @@ static void qnum_to_string_test(void) qn = qnum_from_double(0.42); tmp = qnum_to_string(qn); - g_assert_cmpstr(tmp, ==, "0.42"); + g_assert_cmpstr(tmp, ==, "0.41999999999999998"); g_free(tmp); qobject_unref(qn); qn = qnum_from_double(2.718281828459045); tmp = qnum_to_string(qn); - g_assert_cmpstr(tmp, ==, "2.718282"); /* BUG */ + g_assert_cmpstr(tmp, ==, "2.7182818284590451"); g_free(tmp); qobject_unref(qn); } From 7b205a7373c25db2f3680dee5a8c82e038135ec1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:49 +0100 Subject: [PATCH 10/33] test-string-output-visitor: Cover "unround" number This demonstrates rounding error due to insufficient precision: double 3.1415926535897932 gets converted to JSON 3.141593. Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-8-armbru@redhat.com> --- tests/test-string-output-visitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index 9f6581439a..cec20848ea 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -130,13 +130,13 @@ static void test_visitor_out_bool(TestOutputVisitorData *data, static void test_visitor_out_number(TestOutputVisitorData *data, const void *unused) { - double value = 3.14; + double value = 3.1415926535897932; char *str; visit_type_number(data->ov, NULL, &value, &error_abort); str = visitor_get(data); - g_assert_cmpstr(str, ==, "3.140000"); + g_assert_cmpstr(str, ==, "3.141593"); } static void test_visitor_out_string(TestOutputVisitorData *data, From 54addb01d8c2511ef96b0f0ca6b695d120dd8363 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:50 +0100 Subject: [PATCH 11/33] string-output-visitor: Fix to use sufficient precision The string output visitor should serialize numbers so that the string input visitor deserializes them back to the same number. It fails to do so. print_type_number() uses format %f. This is prone to nasty rounding errors. For instance, numbers between 0 and 0.0000005 get flushed to zero. We currently use this visitor only for HMP info migrate, info network, info qtree, and info memdev. No double values occur there as far as I can tell. Fix anyway by formatting with %.17g. 17 decimal digits always suffice for IEEE double. See also recent commit "qobject: Fix qnum_to_string() to use sufficient precision". Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-9-armbru@redhat.com> --- qapi/string-output-visitor.c | 2 +- tests/test-string-output-visitor.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index b74aa4d44c..5506c933de 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -258,7 +258,7 @@ static bool print_type_number(Visitor *v, const char *name, double *obj, Error **errp) { StringOutputVisitor *sov = to_sov(v); - string_output_set(sov, g_strdup_printf("%f", *obj)); + string_output_set(sov, g_strdup_printf("%.17g", *obj)); return true; } diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index cec20848ea..0dae04b960 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -136,7 +136,7 @@ static void test_visitor_out_number(TestOutputVisitorData *data, visit_type_number(data->ov, NULL, &value, &error_abort); str = visitor_get(data); - g_assert_cmpstr(str, ==, "3.141593"); + g_assert_cmpstr(str, ==, "3.1415926535897931"); } static void test_visitor_out_string(TestOutputVisitorData *data, From 2a02c1398a47e75aa6963baf7dbfa68a54dc2e41 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:51 +0100 Subject: [PATCH 12/33] test-visitor-serialization: Drop insufficient precision workaround Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-10-armbru@redhat.com> --- tests/test-visitor-serialization.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 12275e56d8..cf19924068 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -311,17 +311,7 @@ static void test_primitives(gconstpointer opaque) g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string); g_free((char *)pt_copy->value.string); } else if (pt->type == PTYPE_NUMBER) { - GString *double_expected = g_string_new(""); - GString *double_actual = g_string_new(""); - /* we serialize with %f for our reference visitors, so rather than fuzzy - * floating math to test "equality", just compare the formatted values - */ - g_string_printf(double_expected, "%.6f", pt->value.number); - g_string_printf(double_actual, "%.6f", pt_copy->value.number); - g_assert_cmpstr(double_actual->str, ==, double_expected->str); - - g_string_free(double_expected, true); - g_string_free(double_actual, true); + g_assert_cmpfloat(pt->value.number, ==, pt_copy->value.number); } else if (pt->type == PTYPE_BOOLEAN) { g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max); } else { @@ -703,10 +693,6 @@ static PrimitiveType pt_values[] = { .value.boolean = 0, }, /* number tests (double) */ - /* note: we format these to %.6f before comparing, since that's how - * we serialize them and it doesn't make sense to check precision - * beyond that. - */ { .description = "number_sanity1", .type = PTYPE_NUMBER, @@ -715,7 +701,7 @@ static PrimitiveType pt_values[] = { { .description = "number_sanity2", .type = PTYPE_NUMBER, - .value.number = 3.14159265, + .value.number = 3.141593, }, { .description = "number_min", From 28f1c1f6e07c4bb4d79bed9474d1425c55e21712 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2020 17:14:52 +0100 Subject: [PATCH 13/33] test-visitor-serialization: Clean up test_primitives() test_primitives() uses union member intmax_t max to compare the integer members. Unspecified behavior. Has worked fine for many years, though. Clean it up. Signed-off-by: Markus Armbruster Message-Id: <20201210161452.2813491-11-armbru@redhat.com> --- tests/test-visitor-serialization.c | 44 +++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index cf19924068..dfe120a50d 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -55,7 +55,6 @@ typedef struct PrimitiveType { int16_t s16; int32_t s32; int64_t s64; - intmax_t max; } value; enum PrimitiveTypeKind type; const char *description; @@ -307,15 +306,46 @@ static void test_primitives(gconstpointer opaque) &error_abort); g_assert(pt_copy != NULL); - if (pt->type == PTYPE_STRING) { + switch (pt->type) { + case PTYPE_STRING: g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string); g_free((char *)pt_copy->value.string); - } else if (pt->type == PTYPE_NUMBER) { + break; + case PTYPE_BOOLEAN: + g_assert_cmpint(pt->value.boolean, ==, pt->value.boolean); + break; + case PTYPE_NUMBER: g_assert_cmpfloat(pt->value.number, ==, pt_copy->value.number); - } else if (pt->type == PTYPE_BOOLEAN) { - g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max); - } else { - g_assert_cmpint(pt->value.max, ==, pt_copy->value.max); + break; + case PTYPE_INTEGER: + g_assert_cmpint(pt->value.integer, ==, pt_copy->value.integer); + break; + case PTYPE_U8: + g_assert_cmpuint(pt->value.u8, ==, pt_copy->value.u8); + break; + case PTYPE_U16: + g_assert_cmpuint(pt->value.u16, ==, pt_copy->value.u16); + break; + case PTYPE_U32: + g_assert_cmpuint(pt->value.u32, ==, pt_copy->value.u32); + break; + case PTYPE_U64: + g_assert_cmpuint(pt->value.u64, ==, pt_copy->value.u64); + break; + case PTYPE_S8: + g_assert_cmpint(pt->value.s8, ==, pt_copy->value.s8); + break; + case PTYPE_S16: + g_assert_cmpint(pt->value.s16, ==, pt_copy->value.s16); + break; + case PTYPE_S32: + g_assert_cmpint(pt->value.s32, ==, pt_copy->value.s32); + break; + case PTYPE_S64: + g_assert_cmpint(pt->value.s64, ==, pt_copy->value.s64); + break; + case PTYPE_EOL: + g_assert_not_reached(); } ops->cleanup(serialize_data); From 436054e22f8447692196ddae7012f6083140e461 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:33 +0100 Subject: [PATCH 14/33] hmp: Simplify how qmp_human_monitor_command() gets output Commit 48c043d0d1 "hmp: human-monitor-command: stop using the Memory chardev driver" left us "if string is non-empty, duplicate it, else duplicate the empty string". Meh. Duplicate it unconditionally. Cc: Dr. David Alan Gilbert Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-2-armbru@redhat.com> Reviewed-by: Dr. David Alan Gilbert --- monitor/misc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/monitor/misc.c b/monitor/misc.c index f2ee7cd77a..33d0dae2e8 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -136,11 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, handle_hmp_command(&hmp, command_line); WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) { - if (qstring_get_length(hmp.common.outbuf) > 0) { - output = g_strdup(qstring_get_str(hmp.common.outbuf)); - } else { - output = g_strdup(""); - } + output = g_strdup(qstring_get_str(hmp.common.outbuf)); } out: From 20076f4a8c3b275272d0b907f37c126bd91ec1b9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:34 +0100 Subject: [PATCH 15/33] monitor: Use GString instead of QString for output buffer GString has a richer set of string operations than QString. It should be preferred to QString except where we need a QObject or reference counting. We don't here. Switch to GString, and put its richer interface to use. Cc: Dr. David Alan Gilbert Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-3-armbru@redhat.com> Reviewed-by: Dr. David Alan Gilbert --- monitor/misc.c | 2 +- monitor/monitor-internal.h | 2 +- monitor/monitor.c | 20 ++++++++------------ 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/monitor/misc.c b/monitor/misc.c index 33d0dae2e8..10444b4e7a 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -136,7 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, handle_hmp_command(&hmp, command_line); WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) { - output = g_strdup(qstring_get_str(hmp.common.outbuf)); + output = g_strdup(hmp.common.outbuf->str); } out: diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index a6131554da..40903d6386 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -105,7 +105,7 @@ struct Monitor { * Members that are protected by the per-monitor lock */ QLIST_HEAD(, mon_fd_t) fds; - QString *outbuf; + GString *outbuf; guint out_watch; /* Read under either BQL or mon_lock, written with BQL+mon_lock. */ int mux_out; diff --git a/monitor/monitor.c b/monitor/monitor.c index 84222cd130..1e4a6b3f20 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -29,7 +29,6 @@ #include "qapi/qapi-emit-events.h" #include "qapi/qapi-visit-control.h" #include "qapi/qmp/qdict.h" -#include "qapi/qmp/qstring.h" #include "qemu/error-report.h" #include "qemu/option.h" #include "sysemu/qtest.h" @@ -181,22 +180,19 @@ static void monitor_flush_locked(Monitor *mon) return; } - buf = qstring_get_str(mon->outbuf); - len = qstring_get_length(mon->outbuf); + buf = mon->outbuf->str; + len = mon->outbuf->len; if (len && !mon->mux_out) { rc = qemu_chr_fe_write(&mon->chr, (const uint8_t *) buf, len); if ((rc < 0 && errno != EAGAIN) || (rc == len)) { /* all flushed or error */ - qobject_unref(mon->outbuf); - mon->outbuf = qstring_new(); + g_string_truncate(mon->outbuf, 0); return; } if (rc > 0) { /* partial write */ - QString *tmp = qstring_from_str(buf + rc); - qobject_unref(mon->outbuf); - mon->outbuf = tmp; + g_string_erase(mon->outbuf, 0, rc); } if (mon->out_watch == 0) { mon->out_watch = @@ -223,9 +219,9 @@ int monitor_puts(Monitor *mon, const char *str) for (i = 0; str[i]; i++) { c = str[i]; if (c == '\n') { - qstring_append_chr(mon->outbuf, '\r'); + g_string_append_c(mon->outbuf, '\r'); } - qstring_append_chr(mon->outbuf, c); + g_string_append_c(mon->outbuf, c); if (c == '\n') { monitor_flush_locked(mon); } @@ -602,7 +598,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush, } qemu_mutex_init(&mon->mon_lock); mon->is_qmp = is_qmp; - mon->outbuf = qstring_new(); + mon->outbuf = g_string_new(NULL); mon->skip_flush = skip_flush; mon->use_io_thread = use_io_thread; } @@ -616,7 +612,7 @@ void monitor_data_destroy(Monitor *mon) } else { readline_free(container_of(mon, MonitorHMP, common)->rs); } - qobject_unref(mon->outbuf); + g_string_free(mon->outbuf, true); qemu_mutex_destroy(&mon->mon_lock); } From 6589f4599151201a61d6b1be8450adb63ae81017 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:35 +0100 Subject: [PATCH 16/33] qobject: Make qobject_to_json_pretty() take a pretty argument Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-4-armbru@redhat.com> --- include/qapi/qmp/qjson.h | 2 +- monitor/qmp.c | 2 +- qemu-img.c | 8 ++++---- qobject/qjson.c | 28 +++++++++++----------------- qom/qom-hmp-cmds.c | 2 +- tests/qtest/libqtest.c | 2 +- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h index 5ebbe5a118..82f4534f16 100644 --- a/include/qapi/qmp/qjson.h +++ b/include/qapi/qmp/qjson.h @@ -26,6 +26,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...) GCC_FMT_ATTR(1, 2); QString *qobject_to_json(const QObject *obj); -QString *qobject_to_json_pretty(const QObject *obj); +QString *qobject_to_json_pretty(const QObject *obj, bool pretty); #endif /* QJSON_H */ diff --git a/monitor/qmp.c b/monitor/qmp.c index b42f8c6af3..1197c50b20 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -112,7 +112,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp) const QObject *data = QOBJECT(rsp); QString *json; - json = mon->pretty ? qobject_to_json_pretty(data) : qobject_to_json(data); + json = qobject_to_json_pretty(data, mon->pretty); assert(json != NULL); qstring_append_chr(json, '\n'); diff --git a/qemu-img.c b/qemu-img.c index d599659c7f..1a59dfd3f3 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -633,7 +633,7 @@ static void dump_json_image_check(ImageCheck *check, bool quiet) visit_type_ImageCheck(v, NULL, &check, &error_abort); visit_complete(v, &obj); - str = qobject_to_json_pretty(obj); + str = qobject_to_json_pretty(obj, true); assert(str != NULL); qprintf(quiet, "%s\n", qstring_get_str(str)); qobject_unref(obj); @@ -2795,7 +2795,7 @@ static void dump_json_image_info_list(ImageInfoList *list) visit_type_ImageInfoList(v, NULL, &list, &error_abort); visit_complete(v, &obj); - str = qobject_to_json_pretty(obj); + str = qobject_to_json_pretty(obj, true); assert(str != NULL); printf("%s\n", qstring_get_str(str)); qobject_unref(obj); @@ -2811,7 +2811,7 @@ static void dump_json_image_info(ImageInfo *info) visit_type_ImageInfo(v, NULL, &info, &error_abort); visit_complete(v, &obj); - str = qobject_to_json_pretty(obj); + str = qobject_to_json_pretty(obj, true); assert(str != NULL); printf("%s\n", qstring_get_str(str)); qobject_unref(obj); @@ -5242,7 +5242,7 @@ static void dump_json_block_measure_info(BlockMeasureInfo *info) visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort); visit_complete(v, &obj); - str = qobject_to_json_pretty(obj); + str = qobject_to_json_pretty(obj, true); assert(str != NULL); printf("%s\n", qstring_get_str(str)); qobject_unref(obj); diff --git a/qobject/qjson.c b/qobject/qjson.c index f1f2c69704..523a4ab8de 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -149,8 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...) return qdict; } -static void to_json(const QObject *obj, QString *str, int pretty, int indent); - static void json_pretty_newline(QString *str, bool pretty, int indent) { int i; @@ -163,7 +161,7 @@ static void json_pretty_newline(QString *str, bool pretty, int indent) } } -static void to_json(const QObject *obj, QString *str, int pretty, int indent) +static void to_json(const QObject *obj, QString *str, bool pretty, int indent) { switch (qobject_type(obj)) { case QTYPE_QNULL: @@ -294,20 +292,16 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) } } +QString *qobject_to_json_pretty(const QObject *obj, bool pretty) +{ + QString *str = qstring_new(); + + to_json(obj, str, pretty, 0); + + return str; +} + QString *qobject_to_json(const QObject *obj) { - QString *str = qstring_new(); - - to_json(obj, str, 0, 0); - - return str; -} - -QString *qobject_to_json_pretty(const QObject *obj) -{ - QString *str = qstring_new(); - - to_json(obj, str, 1, 0); - - return str; + return qobject_to_json_pretty(obj, false); } diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index 8861a109d5..6b96dbe906 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -78,7 +78,7 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict) QObject *obj = qmp_qom_get(path, property, &err); if (err == NULL) { - QString *str = qobject_to_json_pretty(obj); + QString *str = qobject_to_json_pretty(obj, true); monitor_printf(mon, "%s\n", qstring_get_str(str)); qobject_unref(str); } diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index e49f3a1e45..213fa4f8fe 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -1197,7 +1197,7 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...) g_assert(response); if (!qdict_haskey(response, "return")) { - QString *s = qobject_to_json_pretty(QOBJECT(response)); + QString *s = qobject_to_json_pretty(QOBJECT(response), true); g_test_message("%s", qstring_get_str(s)); qobject_unref(s); } From f1cc129df8341ebb6176363d24b57035bb5dabe4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:36 +0100 Subject: [PATCH 17/33] qobject: Use GString instead of QString to accumulate JSON QString supports modifying its string, but it's quite limited: you can only append. The remaining callers use it for building an initial string, never for modifying it later. Use of GString for building the initial string is actually more convenient here. Change qobject_to_json() & friends to do that. Once all such uses are replaced this way, QString can become immutable. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-5-armbru@redhat.com> --- include/qapi/qmp/qstring.h | 1 + qobject/qjson.c | 85 +++++++++++++++++--------------------- qobject/qstring.c | 19 +++++++++ 3 files changed, 58 insertions(+), 47 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index e2e356e5e7..ae7698d6c7 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -25,6 +25,7 @@ struct QString { QString *qstring_new(void); QString *qstring_from_str(const char *str); QString *qstring_from_substr(const char *str, size_t start, size_t end); +QString *qstring_from_gstring(GString *gstr); size_t qstring_get_length(const QString *qstring); const char *qstring_get_str(const QString *qstring); const char *qstring_get_try_str(const QString *qstring); diff --git a/qobject/qjson.c b/qobject/qjson.c index 523a4ab8de..e7100a539c 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -149,28 +149,23 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...) return qdict; } -static void json_pretty_newline(QString *str, bool pretty, int indent) +static void json_pretty_newline(GString *accu, bool pretty, int indent) { - int i; - if (pretty) { - qstring_append(str, "\n"); - for (i = 0; i < indent; i++) { - qstring_append(str, " "); - } + g_string_append_printf(accu, "\n%*s", indent * 4, ""); } } -static void to_json(const QObject *obj, QString *str, bool pretty, int indent) +static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) { switch (qobject_type(obj)) { case QTYPE_QNULL: - qstring_append(str, "null"); + g_string_append(accu, "null"); break; case QTYPE_QNUM: { QNum *val = qobject_to(QNum, obj); char *buffer = qnum_to_string(val); - qstring_append(str, buffer); + g_string_append(accu, buffer); g_free(buffer); break; } @@ -178,35 +173,34 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent) QString *val = qobject_to(QString, obj); const char *ptr; int cp; - char buf[16]; char *end; ptr = qstring_get_str(val); - qstring_append(str, "\""); + g_string_append_c(accu, '"'); for (; *ptr; ptr = end) { cp = mod_utf8_codepoint(ptr, 6, &end); switch (cp) { case '\"': - qstring_append(str, "\\\""); + g_string_append(accu, "\\\""); break; case '\\': - qstring_append(str, "\\\\"); + g_string_append(accu, "\\\\"); break; case '\b': - qstring_append(str, "\\b"); + g_string_append(accu, "\\b"); break; case '\f': - qstring_append(str, "\\f"); + g_string_append(accu, "\\f"); break; case '\n': - qstring_append(str, "\\n"); + g_string_append(accu, "\\n"); break; case '\r': - qstring_append(str, "\\r"); + g_string_append(accu, "\\r"); break; case '\t': - qstring_append(str, "\\t"); + g_string_append(accu, "\\t"); break; default: if (cp < 0) { @@ -214,20 +208,18 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent) } if (cp > 0xFFFF) { /* beyond BMP; need a surrogate pair */ - snprintf(buf, sizeof(buf), "\\u%04X\\u%04X", - 0xD800 + ((cp - 0x10000) >> 10), - 0xDC00 + ((cp - 0x10000) & 0x3FF)); + g_string_append_printf(accu, "\\u%04X\\u%04X", + 0xD800 + ((cp - 0x10000) >> 10), + 0xDC00 + ((cp - 0x10000) & 0x3FF)); } else if (cp < 0x20 || cp >= 0x7F) { - snprintf(buf, sizeof(buf), "\\u%04X", cp); + g_string_append_printf(accu, "\\u%04X", cp); } else { - buf[0] = cp; - buf[1] = 0; + g_string_append_c(accu, cp); } - qstring_append(str, buf); } }; - qstring_append(str, "\""); + g_string_append_c(accu, '"'); break; } case QTYPE_QDICT: { @@ -237,25 +229,25 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent) const QDictEntry *entry; QString *qkey; - qstring_append(str, "{"); + g_string_append_c(accu, '{'); for (entry = qdict_first(val); entry; entry = qdict_next(val, entry)) { - qstring_append(str, sep); - json_pretty_newline(str, pretty, indent + 1); + g_string_append(accu, sep); + json_pretty_newline(accu, pretty, indent + 1); qkey = qstring_from_str(qdict_entry_key(entry)); - to_json(QOBJECT(qkey), str, pretty, indent + 1); + to_json(QOBJECT(qkey), accu, pretty, indent + 1); qobject_unref(qkey); - qstring_append(str, ": "); - to_json(qdict_entry_value(entry), str, pretty, indent + 1); + g_string_append(accu, ": "); + to_json(qdict_entry_value(entry), accu, pretty, indent + 1); sep = comma; } - json_pretty_newline(str, pretty, indent); - qstring_append(str, "}"); + json_pretty_newline(accu, pretty, indent); + g_string_append_c(accu, '}'); break; } case QTYPE_QLIST: { @@ -264,26 +256,26 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent) const char *sep = ""; QListEntry *entry; - qstring_append(str, "["); + g_string_append_c(accu, '['); QLIST_FOREACH_ENTRY(val, entry) { - qstring_append(str, sep); - json_pretty_newline(str, pretty, indent + 1); - to_json(qlist_entry_obj(entry), str, pretty, indent + 1); + g_string_append(accu, sep); + json_pretty_newline(accu, pretty, indent + 1); + to_json(qlist_entry_obj(entry), accu, pretty, indent + 1); sep = comma; } - json_pretty_newline(str, pretty, indent); - qstring_append(str, "]"); + json_pretty_newline(accu, pretty, indent); + g_string_append_c(accu, ']'); break; } case QTYPE_QBOOL: { QBool *val = qobject_to(QBool, obj); if (qbool_get_bool(val)) { - qstring_append(str, "true"); + g_string_append(accu, "true"); } else { - qstring_append(str, "false"); + g_string_append(accu, "false"); } break; } @@ -294,11 +286,10 @@ static void to_json(const QObject *obj, QString *str, bool pretty, int indent) QString *qobject_to_json_pretty(const QObject *obj, bool pretty) { - QString *str = qstring_new(); + GString *accu = g_string_new(NULL); - to_json(obj, str, pretty, 0); - - return str; + to_json(obj, accu, pretty, 0); + return qstring_from_gstring(accu); } QString *qobject_to_json(const QObject *obj) diff --git a/qobject/qstring.c b/qobject/qstring.c index b66a2c35f2..af7c18ca73 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -66,6 +66,25 @@ QString *qstring_from_str(const char *str) return qstring_from_substr(str, 0, strlen(str)); } +/** + * qstring_from_gstring(): Convert a GString to a QString + * + * Return strong reference. + */ + +QString *qstring_from_gstring(GString *gstr) +{ + QString *qstring; + + qstring = g_malloc(sizeof(*qstring)); + qobject_init(QOBJECT(qstring), QTYPE_QSTRING); + qstring->length = gstr->len; + qstring->capacity = gstr->allocated_len; + qstring->string = g_string_free(gstr, false); + return qstring; +} + + static void capacity_increase(QString *qstring, size_t len) { if (qstring->capacity < (qstring->length + len)) { From eab3a4678b07267c39e7290a6e9e7690b1d2a521 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:37 +0100 Subject: [PATCH 18/33] qobject: Change qobject_to_json()'s value to GString qobject_to_json() and qobject_to_json_pretty() build a GString, then covert it to QString. Just one of the callers actually needs a QString: qemu_rbd_parse_filename(). A few others need a string they can modify: qmp_send_response(), qga's send_response(), to_json_str(), and qmp_fd_vsend_fds(). The remainder just need a string. Change qobject_to_json() and qobject_to_json_pretty() to return the GString. qemu_rbd_parse_filename() now has to convert to QString. All others save a QString temporary. to_json_str() actually becomes a bit simpler, because GString provides more convenient modification functions. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-6-armbru@redhat.com> --- block.c | 6 ++-- block/rbd.c | 2 +- include/qapi/qmp/qjson.h | 4 +-- monitor/qmp.c | 14 ++++---- qemu-img.c | 25 +++++++------ qga/main.c | 22 ++++-------- qobject/qjson.c | 6 ++-- qom/object_interfaces.c | 4 +-- qom/qom-hmp-cmds.c | 7 ++-- tests/check-qjson.c | 56 ++++++++++++++---------------- tests/qtest/libqtest.c | 20 +++++------ tests/test-visitor-serialization.c | 6 ++-- 12 files changed, 79 insertions(+), 93 deletions(-) diff --git a/block.c b/block.c index 8f177504d4..09d4c6bd25 100644 --- a/block.c +++ b/block.c @@ -6995,13 +6995,13 @@ void bdrv_refresh_filename(BlockDriverState *bs) if (bs->exact_filename[0]) { pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename); } else { - QString *json = qobject_to_json(QOBJECT(bs->full_open_options)); + GString *json = qobject_to_json(QOBJECT(bs->full_open_options)); if (snprintf(bs->filename, sizeof(bs->filename), "json:%s", - qstring_get_str(json)) >= sizeof(bs->filename)) { + json->str) >= sizeof(bs->filename)) { /* Give user a hint if we truncated things. */ strcpy(bs->filename + sizeof(bs->filename) - 4, "..."); } - qobject_unref(json); + g_string_free(json, true); } } diff --git a/block/rbd.c b/block/rbd.c index 9bd2bce716..9071a00e3f 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -232,7 +232,7 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options, if (keypairs) { qdict_put(options, "=keyvalue-pairs", - qobject_to_json(QOBJECT(keypairs))); + qstring_from_gstring(qobject_to_json(QOBJECT(keypairs)))); } done: diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h index 82f4534f16..593b40b4e0 100644 --- a/include/qapi/qmp/qjson.h +++ b/include/qapi/qmp/qjson.h @@ -25,7 +25,7 @@ QDict *qdict_from_vjsonf_nofail(const char *string, va_list ap) QDict *qdict_from_jsonf_nofail(const char *string, ...) GCC_FMT_ATTR(1, 2); -QString *qobject_to_json(const QObject *obj); -QString *qobject_to_json_pretty(const QObject *obj, bool pretty); +GString *qobject_to_json(const QObject *obj); +GString *qobject_to_json_pretty(const QObject *obj, bool pretty); #endif /* QJSON_H */ diff --git a/monitor/qmp.c b/monitor/qmp.c index 1197c50b20..374bb4b81c 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -110,15 +110,15 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon) void qmp_send_response(MonitorQMP *mon, const QDict *rsp) { const QObject *data = QOBJECT(rsp); - QString *json; + GString *json; json = qobject_to_json_pretty(data, mon->pretty); assert(json != NULL); - qstring_append_chr(json, '\n'); - monitor_puts(&mon->common, qstring_get_str(json)); + g_string_append_c(json, '\n'); + monitor_puts(&mon->common, json->str); - qobject_unref(json); + g_string_free(json, true); } /* @@ -320,9 +320,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) } /* else will fail qmp_dispatch() */ if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) { - QString *req_json = qobject_to_json(req); - trace_handle_qmp_command(mon, qstring_get_str(req_json)); - qobject_unref(req_json); + GString *req_json = qobject_to_json(req); + trace_handle_qmp_command(mon, req_json->str); + g_string_free(req_json, true); } if (qdict && qmp_is_oob(qdict)) { diff --git a/qemu-img.c b/qemu-img.c index 1a59dfd3f3..8597d069af 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -33,7 +33,6 @@ #include "qapi/qobject-output-visitor.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qdict.h" -#include "qapi/qmp/qstring.h" #include "qemu/cutils.h" #include "qemu/config-file.h" #include "qemu/option.h" @@ -627,7 +626,7 @@ fail: static void dump_json_image_check(ImageCheck *check, bool quiet) { - QString *str; + GString *str; QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); @@ -635,10 +634,10 @@ static void dump_json_image_check(ImageCheck *check, bool quiet) visit_complete(v, &obj); str = qobject_to_json_pretty(obj, true); assert(str != NULL); - qprintf(quiet, "%s\n", qstring_get_str(str)); + qprintf(quiet, "%s\n", str->str); qobject_unref(obj); visit_free(v); - qobject_unref(str); + g_string_free(str, true); } static void dump_human_image_check(ImageCheck *check, bool quiet) @@ -2789,7 +2788,7 @@ static void dump_snapshots(BlockDriverState *bs) static void dump_json_image_info_list(ImageInfoList *list) { - QString *str; + GString *str; QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); @@ -2797,15 +2796,15 @@ static void dump_json_image_info_list(ImageInfoList *list) visit_complete(v, &obj); str = qobject_to_json_pretty(obj, true); assert(str != NULL); - printf("%s\n", qstring_get_str(str)); + printf("%s\n", str->str); qobject_unref(obj); visit_free(v); - qobject_unref(str); + g_string_free(str, true); } static void dump_json_image_info(ImageInfo *info) { - QString *str; + GString *str; QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); @@ -2813,10 +2812,10 @@ static void dump_json_image_info(ImageInfo *info) visit_complete(v, &obj); str = qobject_to_json_pretty(obj, true); assert(str != NULL); - printf("%s\n", qstring_get_str(str)); + printf("%s\n", str->str); qobject_unref(obj); visit_free(v); - qobject_unref(str); + g_string_free(str, true); } static void dump_human_image_info_list(ImageInfoList *list) @@ -5236,7 +5235,7 @@ out: static void dump_json_block_measure_info(BlockMeasureInfo *info) { - QString *str; + GString *str; QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); @@ -5244,10 +5243,10 @@ static void dump_json_block_measure_info(BlockMeasureInfo *info) visit_complete(v, &obj); str = qobject_to_json_pretty(obj, true); assert(str != NULL); - printf("%s\n", qstring_get_str(str)); + printf("%s\n", str->str); qobject_unref(obj); visit_free(v); - qobject_unref(str); + g_string_free(str, true); } static int img_measure(int argc, char **argv) diff --git a/qga/main.c b/qga/main.c index dea6a3aa64..e7f8f3b161 100644 --- a/qga/main.c +++ b/qga/main.c @@ -22,7 +22,6 @@ #include "qapi/qmp/json-parser.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" -#include "qapi/qmp/qstring.h" #include "guest-agent-core.h" #include "qga-qapi-init-commands.h" #include "qapi/qmp/qerror.h" @@ -528,8 +527,7 @@ fail: static int send_response(GAState *s, const QDict *rsp) { - const char *buf; - QString *payload_qstr, *response_qstr; + GString *response; GIOStatus status; g_assert(s->channel); @@ -538,25 +536,19 @@ static int send_response(GAState *s, const QDict *rsp) return 0; } - payload_qstr = qobject_to_json(QOBJECT(rsp)); - if (!payload_qstr) { + response = qobject_to_json(QOBJECT(rsp)); + if (!response) { return -EINVAL; } if (s->delimit_response) { s->delimit_response = false; - response_qstr = qstring_new(); - qstring_append_chr(response_qstr, QGA_SENTINEL_BYTE); - qstring_append(response_qstr, qstring_get_str(payload_qstr)); - qobject_unref(payload_qstr); - } else { - response_qstr = payload_qstr; + g_string_prepend_c(response, QGA_SENTINEL_BYTE); } - qstring_append_chr(response_qstr, '\n'); - buf = qstring_get_str(response_qstr); - status = ga_channel_write_all(s->channel, buf, strlen(buf)); - qobject_unref(response_qstr); + g_string_append_c(response, '\n'); + status = ga_channel_write_all(s->channel, response->str, response->len); + g_string_free(response, true); if (status != G_IO_STATUS_NORMAL) { return -EIO; } diff --git a/qobject/qjson.c b/qobject/qjson.c index e7100a539c..2f690c1816 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -284,15 +284,15 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) } } -QString *qobject_to_json_pretty(const QObject *obj, bool pretty) +GString *qobject_to_json_pretty(const QObject *obj, bool pretty) { GString *accu = g_string_new(NULL); to_json(obj, accu, pretty, 0); - return qstring_from_gstring(accu); + return accu; } -QString *qobject_to_json(const QObject *obj) +GString *qobject_to_json(const QObject *obj) { return qobject_to_json_pretty(obj, false); } diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index ed896fe764..1e9ad6f08a 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -5,7 +5,6 @@ #include "qapi/qmp/qdict.h" #include "qapi/qmp/qerror.h" #include "qapi/qmp/qjson.h" -#include "qapi/qmp/qstring.h" #include "qapi/qobject-input-visitor.h" #include "qom/object_interfaces.h" #include "qemu/help_option.h" @@ -207,7 +206,8 @@ char *object_property_help(const char *name, const char *type, g_string_append(str, description); } if (defval) { - g_autofree char *def_json = qstring_free(qobject_to_json(defval), TRUE); + g_autofree char *def_json = g_string_free(qobject_to_json(defval), + true); g_string_append_printf(str, " (default: %s)", def_json); } diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index 6b96dbe906..453fbfeabc 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -13,7 +13,6 @@ #include "qapi/qapi-commands-qom.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" -#include "qapi/qmp/qstring.h" #include "qom/object.h" void hmp_qom_list(Monitor *mon, const QDict *qdict) @@ -78,9 +77,9 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict) QObject *obj = qmp_qom_get(path, property, &err); if (err == NULL) { - QString *str = qobject_to_json_pretty(obj, true); - monitor_printf(mon, "%s\n", qstring_get_str(str)); - qobject_unref(str); + GString *str = qobject_to_json_pretty(obj, true); + monitor_printf(mon, "%s\n", str->str); + g_string_free(str, true); } qobject_unref(obj); diff --git a/tests/check-qjson.c b/tests/check-qjson.c index ca8fb816e9..337add0838 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -35,17 +35,15 @@ static QString *from_json_str(const char *jstr, bool single, Error **errp) static char *to_json_str(QString *str) { - QString *json = qobject_to_json(QOBJECT(str)); - char *jstr; + GString *json = qobject_to_json(QOBJECT(str)); if (!json) { return NULL; } /* peel off double quotes */ - jstr = g_strndup(qstring_get_str(json) + 1, - qstring_get_length(json) - 2); - qobject_unref(json); - return jstr; + g_string_truncate(json, json->len - 1); + g_string_erase(json, 0, 1); + return g_string_free(json, false); } static void escaped_string(void) @@ -809,7 +807,7 @@ static void int_number(void) QNum *qnum; int64_t ival; uint64_t uval; - QString *str; + GString *str; for (i = 0; test_cases[i].encoded; i++) { qnum = qobject_to(QNum, @@ -828,9 +826,9 @@ static void int_number(void) (double)test_cases[i].decoded); str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, + g_assert_cmpstr(str->str, ==, test_cases[i].reencoded ?: test_cases[i].encoded); - qobject_unref(str); + g_string_free(str, true); qobject_unref(qnum); } @@ -851,7 +849,7 @@ static void uint_number(void) QNum *qnum; int64_t ival; uint64_t uval; - QString *str; + GString *str; for (i = 0; test_cases[i].encoded; i++) { qnum = qobject_to(QNum, @@ -865,9 +863,9 @@ static void uint_number(void) (double)test_cases[i].decoded); str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, + g_assert_cmpstr(str->str, ==, test_cases[i].reencoded ?: test_cases[i].encoded); - qobject_unref(str); + g_string_free(str, true); qobject_unref(qnum); } @@ -892,7 +890,7 @@ static void float_number(void) QNum *qnum; int64_t ival; uint64_t uval; - QString *str; + GString *str; for (i = 0; test_cases[i].encoded; i++) { qnum = qobject_to(QNum, @@ -904,9 +902,9 @@ static void float_number(void) g_assert(!qnum_get_try_uint(qnum, &uval)); str = qobject_to_json(QOBJECT(qnum)); - g_assert_cmpstr(qstring_get_str(str), ==, + g_assert_cmpstr(str->str, ==, test_cases[i].reencoded ?: test_cases[i].encoded); - qobject_unref(str); + g_string_free(str, true); qobject_unref(qnum); } @@ -917,7 +915,7 @@ static void keyword_literal(void) QObject *obj; QBool *qbool; QNull *null; - QString *str; + GString *str; obj = qobject_from_json("true", &error_abort); qbool = qobject_to(QBool, obj); @@ -925,8 +923,8 @@ static void keyword_literal(void) g_assert(qbool_get_bool(qbool) == true); str = qobject_to_json(obj); - g_assert(strcmp(qstring_get_str(str), "true") == 0); - qobject_unref(str); + g_assert_cmpstr(str->str, ==, "true"); + g_string_free(str, true); qobject_unref(qbool); @@ -936,8 +934,8 @@ static void keyword_literal(void) g_assert(qbool_get_bool(qbool) == false); str = qobject_to_json(obj); - g_assert(strcmp(qstring_get_str(str), "false") == 0); - qobject_unref(str); + g_assert_cmpstr(str->str, ==, "false"); + g_string_free(str, true); qobject_unref(qbool); @@ -1087,7 +1085,7 @@ static void simple_dict(void) for (i = 0; test_cases[i].encoded; i++) { QObject *obj; - QString *str; + GString *str; obj = qobject_from_json(test_cases[i].encoded, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); @@ -1095,10 +1093,10 @@ static void simple_dict(void) str = qobject_to_json(obj); qobject_unref(obj); - obj = qobject_from_json(qstring_get_str(str), &error_abort); + obj = qobject_from_json(str->str, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); qobject_unref(obj); - qobject_unref(str); + g_string_free(str, true); } } @@ -1196,7 +1194,7 @@ static void simple_list(void) for (i = 0; test_cases[i].encoded; i++) { QObject *obj; - QString *str; + GString *str; obj = qobject_from_json(test_cases[i].encoded, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); @@ -1204,10 +1202,10 @@ static void simple_list(void) str = qobject_to_json(obj); qobject_unref(obj); - obj = qobject_from_json(qstring_get_str(str), &error_abort); + obj = qobject_from_json(str->str, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); qobject_unref(obj); - qobject_unref(str); + g_string_free(str, true); } } @@ -1258,7 +1256,7 @@ static void simple_whitespace(void) for (i = 0; test_cases[i].encoded; i++) { QObject *obj; - QString *str; + GString *str; obj = qobject_from_json(test_cases[i].encoded, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); @@ -1266,11 +1264,11 @@ static void simple_whitespace(void) str = qobject_to_json(obj); qobject_unref(obj); - obj = qobject_from_json(qstring_get_str(str), &error_abort); + obj = qobject_from_json(str->str, &error_abort); g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj)); qobject_unref(obj); - qobject_unref(str); + g_string_free(str, true); } } diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 213fa4f8fe..8e93b0a707 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -652,27 +652,25 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num, /* No need to send anything for an empty QObject. */ if (qobj) { int log = getenv("QTEST_LOG") != NULL; - QString *qstr = qobject_to_json(qobj); - const char *str; + GString *str = qobject_to_json(qobj); /* * BUG: QMP doesn't react to input until it sees a newline, an * object, or an array. Work-around: give it a newline. */ - qstring_append_chr(qstr, '\n'); - str = qstring_get_str(qstr); + g_string_append_c(str, '\n'); if (log) { - fprintf(stderr, "%s", str); + fprintf(stderr, "%s", str->str); } /* Send QMP request */ if (fds && fds_num > 0) { - socket_send_fds(fd, fds, fds_num, str, qstring_get_length(qstr)); + socket_send_fds(fd, fds, fds_num, str->str, str->len); } else { - socket_send(fd, str, qstring_get_length(qstr)); + socket_send(fd, str->str, str->len); } - qobject_unref(qstr); + g_string_free(str, true); qobject_unref(qobj); } } @@ -1197,9 +1195,9 @@ void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...) g_assert(response); if (!qdict_haskey(response, "return")) { - QString *s = qobject_to_json_pretty(QOBJECT(response), true); - g_test_message("%s", qstring_get_str(s)); - qobject_unref(s); + GString *s = qobject_to_json_pretty(QOBJECT(response), true); + g_test_message("%s", s->str); + g_string_free(s, true); } g_assert(qdict_haskey(response, "return")); qobject_unref(response); diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index dfe120a50d..4629958647 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -957,15 +957,15 @@ static void qmp_deserialize(void **native_out, void *datap, VisitorFunc visit, Error **errp) { QmpSerializeData *d = datap; - QString *output_json; + GString *output_json; QObject *obj_orig, *obj; visit_complete(d->qov, &d->obj); obj_orig = d->obj; output_json = qobject_to_json(obj_orig); - obj = qobject_from_json(qstring_get_str(output_json), &error_abort); + obj = qobject_from_json(output_json->str, &error_abort); - qobject_unref(output_json); + g_string_free(output_json, true); d->qiv = qobject_input_visitor_new(obj); qobject_unref(obj_orig); qobject_unref(obj); From 88e25b1e6d8a0e3672ba8d5bae5c1df768c35bc8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:38 +0100 Subject: [PATCH 19/33] Revert "qstring: add qstring_free()" This reverts commit 164c374b75f87c6765a705c4418ab7005a2d356f. A free function for a reference-counted object is in bad taste. Fortunately, this one is now also unused. Drop it. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-7-armbru@redhat.com> --- include/qapi/qmp/qstring.h | 1 - qobject/qstring.c | 27 +++++---------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index ae7698d6c7..ae5b4b44d2 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -34,7 +34,6 @@ void qstring_append_int(QString *qstring, int64_t value); void qstring_append(QString *qstring, const char *str); void qstring_append_chr(QString *qstring, int c); bool qstring_is_equal(const QObject *x, const QObject *y); -char *qstring_free(QString *qstring, bool return_str); void qstring_destroy_obj(QObject *obj); #endif /* QSTRING_H */ diff --git a/qobject/qstring.c b/qobject/qstring.c index af7c18ca73..c1891beda0 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -168,33 +168,16 @@ bool qstring_is_equal(const QObject *x, const QObject *y) qobject_to(QString, y)->string); } -/** - * qstring_free(): Free the memory allocated by a QString object - * - * Return: if @return_str, return the underlying string, to be - * g_free(), otherwise NULL is returned. - */ -char *qstring_free(QString *qstring, bool return_str) -{ - char *rv = NULL; - - if (return_str) { - rv = qstring->string; - } else { - g_free(qstring->string); - } - - g_free(qstring); - - return rv; -} - /** * qstring_destroy_obj(): Free all memory allocated by a QString * object */ void qstring_destroy_obj(QObject *obj) { + QString *qs; + assert(obj != NULL); - qstring_free(qobject_to(QString, obj), FALSE); + qs = qobject_to(QString, obj); + g_free(qs->string); + g_free(qs); } From bce800869b44ce82705205147018bffbbc798834 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:39 +0100 Subject: [PATCH 20/33] hw/rdma: Replace QList by GQueue RdmaProtectedQList provides a thread-safe queue of int64_t on top of a QList. rdma_protected_qlist_destroy() calls qlist_destroy_obj() directly. qlist_destroy_obj() is actually for use by qobject_destroy() only. The next commit will make that obvious. The minimal fix would be calling qobject_unref() instead. But QList is actually a bad fit here. It's designed for representing JSON arrays. We're better off with a GQueue here. Replace. Cc: Yuval Shaia Cc: Marcel Apfelbaum Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-8-armbru@redhat.com> --- hw/rdma/rdma_backend.c | 10 +++++----- hw/rdma/rdma_backend_defs.h | 2 +- hw/rdma/rdma_utils.c | 29 ++++++++++++++++------------- hw/rdma/rdma_utils.h | 15 ++++++++------- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index 5de010b1fa..6dcdfbbbe2 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -78,7 +78,7 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev) unsigned long cqe_ctx_id; do { - cqe_ctx_id = rdma_protected_qlist_pop_int64(&backend_dev-> + cqe_ctx_id = rdma_protected_gqueue_pop_int64(&backend_dev-> recv_mads_list); if (cqe_ctx_id != -ENOENT) { qatomic_inc(&backend_dev->rdma_dev_res->stats.missing_cqe); @@ -597,7 +597,7 @@ static unsigned int save_mad_recv_buffer(RdmaBackendDev *backend_dev, bctx->up_ctx = ctx; bctx->sge = *sge; - rdma_protected_qlist_append_int64(&backend_dev->recv_mads_list, bctx_id); + rdma_protected_gqueue_append_int64(&backend_dev->recv_mads_list, bctx_id); return 0; } @@ -1111,7 +1111,7 @@ static void process_incoming_mad_req(RdmaBackendDev *backend_dev, trace_mad_message("recv", msg->umad.mad, msg->umad_len); - cqe_ctx_id = rdma_protected_qlist_pop_int64(&backend_dev->recv_mads_list); + cqe_ctx_id = rdma_protected_gqueue_pop_int64(&backend_dev->recv_mads_list); if (cqe_ctx_id == -ENOENT) { rdma_warn_report("No more free MADs buffers, waiting for a while"); sleep(THR_POLL_TO); @@ -1185,7 +1185,7 @@ static int mad_init(RdmaBackendDev *backend_dev, CharBackend *mad_chr_be) return -EIO; } - rdma_protected_qlist_init(&backend_dev->recv_mads_list); + rdma_protected_gqueue_init(&backend_dev->recv_mads_list); enable_rdmacm_mux_async(backend_dev); @@ -1205,7 +1205,7 @@ static void mad_fini(RdmaBackendDev *backend_dev) { disable_rdmacm_mux_async(backend_dev); qemu_chr_fe_disconnect(backend_dev->rdmacm_mux.chr_be); - rdma_protected_qlist_destroy(&backend_dev->recv_mads_list); + rdma_protected_gqueue_destroy(&backend_dev->recv_mads_list); } int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev, diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h index 0b55be3503..4e6c0ad695 100644 --- a/hw/rdma/rdma_backend_defs.h +++ b/hw/rdma/rdma_backend_defs.h @@ -43,7 +43,7 @@ typedef struct RdmaBackendDev { struct ibv_context *context; struct ibv_comp_channel *channel; uint8_t port_num; - RdmaProtectedQList recv_mads_list; + RdmaProtectedGQueue recv_mads_list; RdmaCmMux rdmacm_mux; } RdmaBackendDev; diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c index 698ed4716c..98df58f689 100644 --- a/hw/rdma/rdma_utils.c +++ b/hw/rdma/rdma_utils.c @@ -14,8 +14,6 @@ */ #include "qemu/osdep.h" -#include "qapi/qmp/qlist.h" -#include "qapi/qmp/qnum.h" #include "trace.h" #include "rdma_utils.h" @@ -54,41 +52,46 @@ void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len) } } -void rdma_protected_qlist_init(RdmaProtectedQList *list) +void rdma_protected_gqueue_init(RdmaProtectedGQueue *list) { qemu_mutex_init(&list->lock); - list->list = qlist_new(); + list->list = g_queue_new(); } -void rdma_protected_qlist_destroy(RdmaProtectedQList *list) +void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list) { if (list->list) { - qlist_destroy_obj(QOBJECT(list->list)); + g_queue_free_full(list->list, g_free); qemu_mutex_destroy(&list->lock); list->list = NULL; } } -void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t value) +void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list, + int64_t value) { qemu_mutex_lock(&list->lock); - qlist_append_int(list->list, value); + g_queue_push_tail(list->list, g_memdup(&value, sizeof(value))); qemu_mutex_unlock(&list->lock); } -int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list) +int64_t rdma_protected_gqueue_pop_int64(RdmaProtectedGQueue *list) { - QObject *obj; + int64_t *valp; + int64_t val; qemu_mutex_lock(&list->lock); - obj = qlist_pop(list->list); + + valp = g_queue_pop_head(list->list); qemu_mutex_unlock(&list->lock); - if (!obj) { + if (!valp) { return -ENOENT; } - return qnum_get_uint(qobject_to(QNum, obj)); + val = *valp; + g_free(valp); + return val; } void rdma_protected_gslist_init(RdmaProtectedGSList *list) diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h index e7babe96cb..9fd0efd940 100644 --- a/hw/rdma/rdma_utils.h +++ b/hw/rdma/rdma_utils.h @@ -28,10 +28,10 @@ #define rdma_info_report(fmt, ...) \ info_report("%s: " fmt, "rdma", ## __VA_ARGS__) -typedef struct RdmaProtectedQList { +typedef struct RdmaProtectedGQueue { QemuMutex lock; - QList *list; -} RdmaProtectedQList; + GQueue *list; +} RdmaProtectedGQueue; typedef struct RdmaProtectedGSList { QemuMutex lock; @@ -40,10 +40,11 @@ typedef struct RdmaProtectedGSList { void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen); void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len); -void rdma_protected_qlist_init(RdmaProtectedQList *list); -void rdma_protected_qlist_destroy(RdmaProtectedQList *list); -void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t value); -int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list); +void rdma_protected_gqueue_init(RdmaProtectedGQueue *list); +void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list); +void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list, + int64_t value); +int64_t rdma_protected_gqueue_pop_int64(RdmaProtectedGQueue *list); void rdma_protected_gslist_init(RdmaProtectedGSList *list); void rdma_protected_gslist_destroy(RdmaProtectedGSList *list); void rdma_protected_gslist_append_int32(RdmaProtectedGSList *list, From 80d71121b719c610c7f6e05c932c35ded4cc92d4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:40 +0100 Subject: [PATCH 21/33] qobject: Move internals to qobject-internal.h Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-9-armbru@redhat.com> --- include/qapi/qmp/qbool.h | 2 -- include/qapi/qmp/qdict.h | 2 -- include/qapi/qmp/qlist.h | 2 -- include/qapi/qmp/qnull.h | 2 -- include/qapi/qmp/qnum.h | 3 --- include/qapi/qmp/qobject.h | 9 +-------- include/qapi/qmp/qstring.h | 2 -- qobject/qbool.c | 1 + qobject/qdict.c | 1 + qobject/qlist.c | 1 + qobject/qnull.c | 1 + qobject/qnum.c | 1 + qobject/qobject-internal.h | 39 ++++++++++++++++++++++++++++++++++++++ qobject/qobject.c | 1 + qobject/qstring.c | 1 + 15 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 qobject/qobject-internal.h diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h index 5f61e38e64..2f888d1057 100644 --- a/include/qapi/qmp/qbool.h +++ b/include/qapi/qmp/qbool.h @@ -23,7 +23,5 @@ struct QBool { QBool *qbool_from_bool(bool value); bool qbool_get_bool(const QBool *qb); -bool qbool_is_equal(const QObject *x, const QObject *y); -void qbool_destroy_obj(QObject *obj); #endif /* QBOOL_H */ diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index da942347a7..9934539c1b 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -39,10 +39,8 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject *value); void qdict_del(QDict *qdict, const char *key); int qdict_haskey(const QDict *qdict, const char *key); QObject *qdict_get(const QDict *qdict, const char *key); -bool qdict_is_equal(const QObject *x, const QObject *y); const QDictEntry *qdict_first(const QDict *qdict); const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry); -void qdict_destroy_obj(QObject *obj); /* Helper to qdict_put_obj(), accepts any object */ #define qdict_put(qdict, key, obj) \ diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h index 595b7943e1..06e98ad5f4 100644 --- a/include/qapi/qmp/qlist.h +++ b/include/qapi/qmp/qlist.h @@ -51,8 +51,6 @@ QObject *qlist_pop(QList *qlist); QObject *qlist_peek(QList *qlist); int qlist_empty(const QList *qlist); size_t qlist_size(const QList *qlist); -bool qlist_is_equal(const QObject *x, const QObject *y); -void qlist_destroy_obj(QObject *obj); static inline const QListEntry *qlist_first(const QList *qlist) { diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h index c1426882c5..e84ecceedb 100644 --- a/include/qapi/qmp/qnull.h +++ b/include/qapi/qmp/qnull.h @@ -26,6 +26,4 @@ static inline QNull *qnull(void) return qobject_ref(&qnull_); } -bool qnull_is_equal(const QObject *x, const QObject *y); - #endif /* QNULL_H */ diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h index bbae0a5ec8..7f84e20bfb 100644 --- a/include/qapi/qmp/qnum.h +++ b/include/qapi/qmp/qnum.h @@ -68,7 +68,4 @@ double qnum_get_double(QNum *qn); char *qnum_to_string(QNum *qn); -bool qnum_is_equal(const QObject *x, const QObject *y); -void qnum_destroy_obj(QObject *obj); - #endif /* QNUM_H */ diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h index fcfd549220..9003b71fd3 100644 --- a/include/qapi/qmp/qobject.h +++ b/include/qapi/qmp/qobject.h @@ -64,14 +64,6 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7, #define qobject_to(type, obj) \ ((type *)qobject_check_type(obj, glue(QTYPE_CAST_TO_, type))) -/* Initialize an object to default values */ -static inline void qobject_init(QObject *obj, QType type) -{ - assert(QTYPE_NONE < type && type < QTYPE__MAX); - obj->base.refcnt = 1; - obj->base.type = type; -} - static inline void qobject_ref_impl(QObject *obj) { if (obj) { @@ -90,6 +82,7 @@ bool qobject_is_equal(const QObject *x, const QObject *y); /** * qobject_destroy(): Free resources used by the object + * For use via qobject_unref() only! */ void qobject_destroy(QObject *obj); diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index ae5b4b44d2..e4ac761a22 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -33,7 +33,5 @@ const char *qobject_get_try_str(const QObject *qstring); void qstring_append_int(QString *qstring, int64_t value); void qstring_append(QString *qstring, const char *str); void qstring_append_chr(QString *qstring, int c); -bool qstring_is_equal(const QObject *x, const QObject *y); -void qstring_destroy_obj(QObject *obj); #endif /* QSTRING_H */ diff --git a/qobject/qbool.c b/qobject/qbool.c index 06dfc43498..16a600abb9 100644 --- a/qobject/qbool.c +++ b/qobject/qbool.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qapi/qmp/qbool.h" +#include "qobject-internal.h" /** * qbool_from_bool(): Create a new QBool from a bool diff --git a/qobject/qdict.c b/qobject/qdict.c index 1079bd3f6f..d84443391e 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -16,6 +16,7 @@ #include "qapi/qmp/qbool.h" #include "qapi/qmp/qnull.h" #include "qapi/qmp/qstring.h" +#include "qobject-internal.h" /** * qdict_new(): Create a new QDict diff --git a/qobject/qlist.c b/qobject/qlist.c index 1be95367d1..60562a1f52 100644 --- a/qobject/qlist.c +++ b/qobject/qlist.c @@ -17,6 +17,7 @@ #include "qapi/qmp/qnum.h" #include "qapi/qmp/qstring.h" #include "qemu/queue.h" +#include "qobject-internal.h" /** * qlist_new(): Create a new QList diff --git a/qobject/qnull.c b/qobject/qnull.c index 00870a1824..b26b368219 100644 --- a/qobject/qnull.c +++ b/qobject/qnull.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qapi/qmp/qnull.h" +#include "qobject-internal.h" QNull qnull_ = { .base = { diff --git a/qobject/qnum.c b/qobject/qnum.c index bf1240ecec..35ba41e61c 100644 --- a/qobject/qnum.c +++ b/qobject/qnum.c @@ -14,6 +14,7 @@ #include "qemu/osdep.h" #include "qapi/qmp/qnum.h" +#include "qobject-internal.h" /** * qnum_from_int(): Create a new QNum from an int64_t diff --git a/qobject/qobject-internal.h b/qobject/qobject-internal.h new file mode 100644 index 0000000000..b310c8e1b5 --- /dev/null +++ b/qobject/qobject-internal.h @@ -0,0 +1,39 @@ +/* + * QObject internals + * + * Copyright (C) 2015 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 + * or later. See the COPYING.LIB file in the top-level directory. + */ + +#ifndef QOBJECT_INTERNAL_H +#define QOBJECT_INTERNAL_H + +#include "qapi/qmp/qobject.h" + +static inline void qobject_init(QObject *obj, QType type) +{ + assert(QTYPE_NONE < type && type < QTYPE__MAX); + obj->base.refcnt = 1; + obj->base.type = type; +} + +void qbool_destroy_obj(QObject *obj); +bool qbool_is_equal(const QObject *x, const QObject *y); + +void qdict_destroy_obj(QObject *obj); +bool qdict_is_equal(const QObject *x, const QObject *y); + +void qlist_destroy_obj(QObject *obj); +bool qlist_is_equal(const QObject *x, const QObject *y); + +bool qnull_is_equal(const QObject *x, const QObject *y); + +void qnum_destroy_obj(QObject *obj); +bool qnum_is_equal(const QObject *x, const QObject *y); + +void qstring_destroy_obj(QObject *obj); +bool qstring_is_equal(const QObject *x, const QObject *y); + +#endif diff --git a/qobject/qobject.c b/qobject/qobject.c index 878dd76e79..d7077b8f2a 100644 --- a/qobject/qobject.c +++ b/qobject/qobject.c @@ -14,6 +14,7 @@ #include "qapi/qmp/qdict.h" #include "qapi/qmp/qlist.h" #include "qapi/qmp/qstring.h" +#include "qobject-internal.h" QEMU_BUILD_BUG_MSG( offsetof(QNull, base) != 0 || diff --git a/qobject/qstring.c b/qobject/qstring.c index c1891beda0..d6724bd4e5 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qapi/qmp/qstring.h" +#include "qobject-internal.h" /** * qstring_new(): Create a new empty QString From d403d92dfc243f0fb7dec432f971961c522ba077 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:41 +0100 Subject: [PATCH 22/33] qmp: Fix tracing of non-string command IDs Tracepoints monitor_qmp_cmd_in_band and monitor_qmp_cmd_out_of_band (commit cf869d5317 "qmp: support out-of-band (oob) execution") treat non-string "id" like absent "id". Fix that. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-10-armbru@redhat.com> --- monitor/qmp.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/monitor/qmp.c b/monitor/qmp.c index 374bb4b81c..8f91af32be 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -31,7 +31,6 @@ #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qlist.h" -#include "qapi/qmp/qstring.h" #include "trace.h" struct QMPRequest { @@ -276,9 +275,15 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; qemu_mutex_unlock(&mon->qmp_queue_lock); if (req_obj->req) { - QDict *qdict = qobject_to(QDict, req_obj->req); - QObject *id = qdict ? qdict_get(qdict, "id") : NULL; - trace_monitor_qmp_cmd_in_band(qobject_get_try_str(id) ?: ""); + if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_IN_BAND)) { + QDict *qdict = qobject_to(QDict, req_obj->req); + QObject *id = qdict ? qdict_get(qdict, "id") : NULL; + GString *id_json; + + id_json = id ? qobject_to_json(id) : g_string_new(NULL); + trace_monitor_qmp_cmd_in_band(id_json->str); + g_string_free(id_json, true); + } monitor_qmp_dispatch(mon, req_obj->req); } else { assert(req_obj->err); @@ -308,17 +313,11 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) static void handle_qmp_command(void *opaque, QObject *req, Error *err) { MonitorQMP *mon = opaque; - QObject *id = NULL; - QDict *qdict; + QDict *qdict = qobject_to(QDict, req); QMPRequest *req_obj; assert(!req != !err); - qdict = qobject_to(QDict, req); - if (qdict) { - id = qdict_get(qdict, "id"); - } /* else will fail qmp_dispatch() */ - if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) { GString *req_json = qobject_to_json(req); trace_handle_qmp_command(mon, req_json->str); @@ -327,7 +326,14 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) if (qdict && qmp_is_oob(qdict)) { /* OOB commands are executed immediately */ - trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) ?: ""); + if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_OUT_OF_BAND)) { + QObject *id = qdict_get(qdict, "id"); + GString *id_json; + + id_json = id ? qobject_to_json(id) : g_string_new(NULL); + trace_monitor_qmp_cmd_out_of_band(id_json->str); + g_string_free(id_json, true); + } monitor_qmp_dispatch(mon, req); qobject_unref(req); return; From 410f44f59685b05fc76e813fb2716bdb17c8da8d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:42 +0100 Subject: [PATCH 23/33] block: Avoid qobject_get_try_str() I'm about to remove qobject_get_try_str(). Use qstring_get_str() instead. Safe because the argument is known to be a QString here. Cc: Kevin Wolf Cc: Max Reitz Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-11-armbru@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 09d4c6bd25..934d1bcc8f 100644 --- a/block.c +++ b/block.c @@ -4021,7 +4021,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state, new_backing_bs = NULL; break; case QTYPE_QSTRING: - str = qobject_get_try_str(value); + str = qstring_get_str(qobject_to(QString, value)); new_backing_bs = bdrv_lookup_bs(NULL, str, errp); if (new_backing_bs == NULL) { return -EINVAL; @@ -4284,8 +4284,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, } if (child) { - const char *str = qobject_get_try_str(new); - if (!strcmp(child->bs->node_name, str)) { + if (!strcmp(child->bs->node_name, + qstring_get_str(qobject_to(QString, new)))) { continue; /* Found child with this name, skip option */ } } From 26c52828f6e7760407e5bb43a2fab3a276eccf43 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:43 +0100 Subject: [PATCH 24/33] Revert "qobject: let object_property_get_str() use new API" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit aafb21a0b9 "qobject: let object_property_get_str() use new API" isn't much of a simplification. Not worth having object_property_get_str() differ from the other object_property_get_FOO(). Revert. This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02. Cc: Paolo Bonzini Cc: Daniel P. Berrangé Cc: Eduardo Habkost Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-12-armbru@redhat.com> Reviewed-by: Eduardo Habkost --- qom/object.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/qom/object.c b/qom/object.c index f2ae6e6b2a..5cd43fe366 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1415,15 +1415,18 @@ char *object_property_get_str(Object *obj, const char *name, Error **errp) { QObject *ret = object_property_get_qobject(obj, name, errp); + QString *qstring; char *retval; if (!ret) { return NULL; } - - retval = g_strdup(qobject_get_try_str(ret)); - if (!retval) { + qstring = qobject_to(QString, ret); + if (!qstring) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "string"); + retval = NULL; + } else { + retval = g_strdup(qstring_get_str(qstring)); } qobject_unref(ret); From 808ac3657e4eb8e9fa62f57507c6d2eaa2557466 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:44 +0100 Subject: [PATCH 25/33] qobject: Drop qobject_get_try_str() Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-13-armbru@redhat.com> --- include/qapi/qmp/qstring.h | 1 - qobject/qstring.c | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index e4ac761a22..56034dae54 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -29,7 +29,6 @@ QString *qstring_from_gstring(GString *gstr); size_t qstring_get_length(const QString *qstring); const char *qstring_get_str(const QString *qstring); const char *qstring_get_try_str(const QString *qstring); -const char *qobject_get_try_str(const QObject *qstring); void qstring_append_int(QString *qstring, int64_t value); void qstring_append(QString *qstring, const char *str); void qstring_append_chr(QString *qstring, int c); diff --git a/qobject/qstring.c b/qobject/qstring.c index d6724bd4e5..cfe3f3bf00 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -149,17 +149,6 @@ const char *qstring_get_try_str(const QString *qstring) return qstring ? qstring_get_str(qstring) : NULL; } -/** - * qobject_get_try_str(): Return a pointer to the corresponding string - * - * NOTE: the string will only be returned if the object is valid, and - * its type is QString, otherwise NULL is returned. - */ -const char *qobject_get_try_str(const QObject *qstring) -{ - return qstring_get_try_str(qobject_to(QString, qstring)); -} - /** * qstring_is_equal(): Test whether the two QStrings are equal */ From b3119b08143a036e8089fb442f9d42c4920ba22c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:45 +0100 Subject: [PATCH 26/33] qobject: Drop qstring_get_try_str() No users left outside tests/, and the ones in tests/ can just as well use qstring_get_str(). Do that, and drop the function. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-14-armbru@redhat.com> --- include/qapi/qmp/qstring.h | 1 - qobject/qstring.c | 10 ---------- tests/check-qjson.c | 11 +++++------ 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index 56034dae54..53567db6c0 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -28,7 +28,6 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end); QString *qstring_from_gstring(GString *gstr); size_t qstring_get_length(const QString *qstring); const char *qstring_get_str(const QString *qstring); -const char *qstring_get_try_str(const QString *qstring); void qstring_append_int(QString *qstring, int64_t value); void qstring_append(QString *qstring, const char *str); void qstring_append_chr(QString *qstring, int c); diff --git a/qobject/qstring.c b/qobject/qstring.c index cfe3f3bf00..ea86d80cf0 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -139,16 +139,6 @@ const char *qstring_get_str(const QString *qstring) return qstring->string; } -/** - * qstring_get_try_str(): Return a pointer to the stored string - * - * NOTE: will return NULL if qstring is not provided. - */ -const char *qstring_get_try_str(const QString *qstring) -{ - return qstring ? qstring_get_str(qstring) : NULL; -} - /** * qstring_is_equal(): Test whether the two QStrings are equal */ diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 337add0838..c845f91d43 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -89,7 +89,7 @@ static void escaped_string(void) for (j = 0; j < 2; j++) { if (test_cases[i].utf8_out) { cstr = from_json_str(test_cases[i].json_in, j, &error_abort); - g_assert_cmpstr(qstring_get_try_str(cstr), + g_assert_cmpstr(qstring_get_str(cstr), ==, test_cases[i].utf8_out); if (!test_cases[i].skip) { jstr = to_json_str(cstr); @@ -751,7 +751,7 @@ static void utf8_string(void) /* Parse @json_in, expect @utf8_out */ if (utf8_out) { str = from_json_str(json_in, j, &error_abort); - g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_out); + g_assert_cmpstr(qstring_get_str(str), ==, utf8_out); qobject_unref(str); } else { str = from_json_str(json_in, j, NULL); @@ -782,7 +782,7 @@ static void utf8_string(void) /* Parse @json_out right back, unless it has replacements */ if (!strstr(json_out, "\\uFFFD")) { str = from_json_str(json_out, j, &error_abort); - g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_in); + g_assert_cmpstr(qstring_get_str(str), ==, utf8_in); qobject_unref(str); } } @@ -1021,9 +1021,8 @@ static void interpolation_valid(void) /* string */ - qstr = qobject_to(QString, - qobject_from_jsonf_nofail("%s", value_s)); - g_assert_cmpstr(qstring_get_try_str(qstr), ==, value_s); + qstr = qobject_to(QString, qobject_from_jsonf_nofail("%s", value_s)); + g_assert_cmpstr(qstring_get_str(qstr), ==, value_s); qobject_unref(qstr); /* object */ From 91f54d92c752e392be4903c9ad2846b2cdd0398d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:46 +0100 Subject: [PATCH 27/33] qobject: Factor quoted_str() out of to_json() Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-15-armbru@redhat.com> --- qobject/qjson.c | 110 ++++++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 56 deletions(-) diff --git a/qobject/qjson.c b/qobject/qjson.c index 2f690c1816..962214f5a7 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -156,6 +156,58 @@ static void json_pretty_newline(GString *accu, bool pretty, int indent) } } +static void quoted_str(const char *str, GString *accu) +{ + const char *ptr; + int cp; + char *end; + + g_string_append_c(accu, '"'); + + for (ptr = str; *ptr; ptr = end) { + cp = mod_utf8_codepoint(ptr, 6, &end); + switch (cp) { + case '\"': + g_string_append(accu, "\\\""); + break; + case '\\': + g_string_append(accu, "\\\\"); + break; + case '\b': + g_string_append(accu, "\\b"); + break; + case '\f': + g_string_append(accu, "\\f"); + break; + case '\n': + g_string_append(accu, "\\n"); + break; + case '\r': + g_string_append(accu, "\\r"); + break; + case '\t': + g_string_append(accu, "\\t"); + break; + default: + if (cp < 0) { + cp = 0xFFFD; /* replacement character */ + } + if (cp > 0xFFFF) { + /* beyond BMP; need a surrogate pair */ + g_string_append_printf(accu, "\\u%04X\\u%04X", + 0xD800 + ((cp - 0x10000) >> 10), + 0xDC00 + ((cp - 0x10000) & 0x3FF)); + } else if (cp < 0x20 || cp >= 0x7F) { + g_string_append_printf(accu, "\\u%04X", cp); + } else { + g_string_append_c(accu, cp); + } + } + }; + + g_string_append_c(accu, '"'); +} + static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) { switch (qobject_type(obj)) { @@ -170,56 +222,7 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) break; } case QTYPE_QSTRING: { - QString *val = qobject_to(QString, obj); - const char *ptr; - int cp; - char *end; - - ptr = qstring_get_str(val); - g_string_append_c(accu, '"'); - - for (; *ptr; ptr = end) { - cp = mod_utf8_codepoint(ptr, 6, &end); - switch (cp) { - case '\"': - g_string_append(accu, "\\\""); - break; - case '\\': - g_string_append(accu, "\\\\"); - break; - case '\b': - g_string_append(accu, "\\b"); - break; - case '\f': - g_string_append(accu, "\\f"); - break; - case '\n': - g_string_append(accu, "\\n"); - break; - case '\r': - g_string_append(accu, "\\r"); - break; - case '\t': - g_string_append(accu, "\\t"); - break; - default: - if (cp < 0) { - cp = 0xFFFD; /* replacement character */ - } - if (cp > 0xFFFF) { - /* beyond BMP; need a surrogate pair */ - g_string_append_printf(accu, "\\u%04X\\u%04X", - 0xD800 + ((cp - 0x10000) >> 10), - 0xDC00 + ((cp - 0x10000) & 0x3FF)); - } else if (cp < 0x20 || cp >= 0x7F) { - g_string_append_printf(accu, "\\u%04X", cp); - } else { - g_string_append_c(accu, cp); - } - } - }; - - g_string_append_c(accu, '"'); + quoted_str(qstring_get_str(qobject_to(QString, obj)), accu); break; } case QTYPE_QDICT: { @@ -227,7 +230,6 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) const char *comma = pretty ? "," : ", "; const char *sep = ""; const QDictEntry *entry; - QString *qkey; g_string_append_c(accu, '{'); @@ -236,11 +238,7 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) entry = qdict_next(val, entry)) { g_string_append(accu, sep); json_pretty_newline(accu, pretty, indent + 1); - - qkey = qstring_from_str(qdict_entry_key(entry)); - to_json(QOBJECT(qkey), accu, pretty, indent + 1); - qobject_unref(qkey); - + quoted_str(qdict_entry_key(entry), accu); g_string_append(accu, ": "); to_json(qdict_entry_value(entry), accu, pretty, indent + 1); sep = comma; From 998da0b1581bfda6d6d0e82b9e42edfa1bf5cfe5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:47 +0100 Subject: [PATCH 28/33] qobject: Factor JSON writer out of qobject_to_json() We have two JSON writers written in C: qobject/qjson.c provides qobject_to_json(), and migration/qjson.c provides a more low level imperative interface. They don't share code. The latter tacitly limits numbers to int64_t, and strings contents to characters that don't need escaping. Factor out qobject_to_json()'s JSON writer as qobject/json-writer.c. Straightforward, except for numbers: since the writer is to be independent of QObject, it can't use qnum_to_string(). Open-code it instead. This is actually an improvement of sorts, because it liberates qnum_to_string() from JSON's needs: its JSON-related FIXMEs move to the JSON writer, where they belong. The next commit will replace migration/qjson.c. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-16-armbru@redhat.com> --- include/qapi/qmp/json-writer.h | 37 +++++ qobject/json-writer.c | 247 +++++++++++++++++++++++++++++++++ qobject/meson.build | 5 +- qobject/qjson.c | 125 +++++------------ qobject/qnum.c | 5 - 5 files changed, 318 insertions(+), 101 deletions(-) create mode 100644 include/qapi/qmp/json-writer.h create mode 100644 qobject/json-writer.c diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h new file mode 100644 index 0000000000..708d129018 --- /dev/null +++ b/include/qapi/qmp/json-writer.h @@ -0,0 +1,37 @@ +/* + * JSON Writer + * + * Copyright (c) 2020 Red Hat Inc. + * + * Authors: + * Markus Armbruster + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ + +#ifndef JSON_WRITER_H +#define JSON_WRITER_H + +typedef struct JSONWriter JSONWriter; + +JSONWriter *json_writer_new(bool pretty); +const char *json_writer_get(JSONWriter *); +GString *json_writer_get_and_free(JSONWriter *); +void json_writer_free(JSONWriter *); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(JSONWriter, json_writer_free) + +void json_writer_start_object(JSONWriter *, const char *name); +void json_writer_end_object(JSONWriter *); +void json_writer_start_array(JSONWriter *, const char *name); +void json_writer_end_array(JSONWriter *); +void json_writer_bool(JSONWriter *, const char *name, bool val); +void json_writer_null(JSONWriter *, const char *name); +void json_writer_int64(JSONWriter *, const char *name, int64_t val); +void json_writer_uint64(JSONWriter *, const char *name, uint64_t val); +void json_writer_double(JSONWriter *, const char *name, double val); +void json_writer_str(JSONWriter *, const char *name, const char *str); + +#endif diff --git a/qobject/json-writer.c b/qobject/json-writer.c new file mode 100644 index 0000000000..309a31d57a --- /dev/null +++ b/qobject/json-writer.c @@ -0,0 +1,247 @@ +/* + * JSON Writer + * + * Copyright IBM, Corp. 2009 + * Copyright (c) 2010-2020 Red Hat Inc. + * + * Authors: + * Anthony Liguori + * Markus Armbruster + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qapi/qmp/json-writer.h" +#include "qemu/unicode.h" + +struct JSONWriter { + bool pretty; + bool need_comma; + GString *contents; + GByteArray *container_is_array; +}; + +JSONWriter *json_writer_new(bool pretty) +{ + JSONWriter *writer = g_new(JSONWriter, 1); + + writer->pretty = pretty; + writer->need_comma = false; + writer->contents = g_string_new(NULL); + writer->container_is_array = g_byte_array_new(); + return writer; +} + +const char *json_writer_get(JSONWriter *writer) +{ + g_assert(!writer->container_is_array->len); + return writer->contents->str; +} + +GString *json_writer_get_and_free(JSONWriter *writer) +{ + GString *contents = writer->contents; + + writer->contents = NULL; + g_byte_array_free(writer->container_is_array, true); + g_free(writer); + return contents; +} + +void json_writer_free(JSONWriter *writer) +{ + if (writer) { + g_string_free(json_writer_get_and_free(writer), true); + } +} + +static void enter_container(JSONWriter *writer, bool is_array) +{ + unsigned depth = writer->container_is_array->len; + + g_byte_array_set_size(writer->container_is_array, depth + 1); + writer->container_is_array->data[depth] = is_array; + writer->need_comma = false; +} + +static void leave_container(JSONWriter *writer, bool is_array) +{ + unsigned depth = writer->container_is_array->len; + + assert(depth); + assert(writer->container_is_array->data[depth - 1] == is_array); + g_byte_array_set_size(writer->container_is_array, depth - 1); + writer->need_comma = true; +} + +static bool in_object(JSONWriter *writer) +{ + unsigned depth = writer->container_is_array->len; + + return depth && !writer->container_is_array->data[depth - 1]; +} + +static void pretty_newline(JSONWriter *writer) +{ + if (writer->pretty) { + g_string_append_printf(writer->contents, "\n%*s", + writer->container_is_array->len * 4, ""); + } +} + +static void pretty_newline_or_space(JSONWriter *writer) +{ + if (writer->pretty) { + g_string_append_printf(writer->contents, "\n%*s", + writer->container_is_array->len * 4, ""); + } else { + g_string_append_c(writer->contents, ' '); + } +} + +static void quoted_str(JSONWriter *writer, const char *str) +{ + const char *ptr; + char *end; + int cp; + + g_string_append_c(writer->contents, '"'); + + for (ptr = str; *ptr; ptr = end) { + cp = mod_utf8_codepoint(ptr, 6, &end); + switch (cp) { + case '\"': + g_string_append(writer->contents, "\\\""); + break; + case '\\': + g_string_append(writer->contents, "\\\\"); + break; + case '\b': + g_string_append(writer->contents, "\\b"); + break; + case '\f': + g_string_append(writer->contents, "\\f"); + break; + case '\n': + g_string_append(writer->contents, "\\n"); + break; + case '\r': + g_string_append(writer->contents, "\\r"); + break; + case '\t': + g_string_append(writer->contents, "\\t"); + break; + default: + if (cp < 0) { + cp = 0xFFFD; /* replacement character */ + } + if (cp > 0xFFFF) { + /* beyond BMP; need a surrogate pair */ + g_string_append_printf(writer->contents, "\\u%04X\\u%04X", + 0xD800 + ((cp - 0x10000) >> 10), + 0xDC00 + ((cp - 0x10000) & 0x3FF)); + } else if (cp < 0x20 || cp >= 0x7F) { + g_string_append_printf(writer->contents, "\\u%04X", cp); + } else { + g_string_append_c(writer->contents, cp); + } + } + }; + + g_string_append_c(writer->contents, '"'); +} + +static void maybe_comma_name(JSONWriter *writer, const char *name) +{ + if (writer->need_comma) { + g_string_append_c(writer->contents, ','); + pretty_newline_or_space(writer); + } else { + if (writer->contents->len) { + pretty_newline(writer); + } + writer->need_comma = true; + } + + if (in_object(writer)) { + quoted_str(writer, name); + g_string_append(writer->contents, ": "); + } +} + +void json_writer_start_object(JSONWriter *writer, const char *name) +{ + maybe_comma_name(writer, name); + g_string_append_c(writer->contents, '{'); + enter_container(writer, false); +} + +void json_writer_end_object(JSONWriter *writer) +{ + leave_container(writer, false); + pretty_newline(writer); + g_string_append_c(writer->contents, '}'); +} + +void json_writer_start_array(JSONWriter *writer, const char *name) +{ + maybe_comma_name(writer, name); + g_string_append_c(writer->contents, '['); + enter_container(writer, true); +} + +void json_writer_end_array(JSONWriter *writer) +{ + leave_container(writer, true); + pretty_newline(writer); + g_string_append_c(writer->contents, ']'); +} + +void json_writer_bool(JSONWriter *writer, const char *name, bool val) +{ + maybe_comma_name(writer, name); + g_string_append(writer->contents, val ? "true" : "false"); +} + +void json_writer_null(JSONWriter *writer, const char *name) +{ + maybe_comma_name(writer, name); + g_string_append(writer->contents, "null"); +} + +void json_writer_int64(JSONWriter *writer, const char *name, int64_t val) +{ + maybe_comma_name(writer, name); + g_string_append_printf(writer->contents, "%" PRId64, val); +} + +void json_writer_uint64(JSONWriter *writer, const char *name, uint64_t val) +{ + maybe_comma_name(writer, name); + g_string_append_printf(writer->contents, "%" PRIu64, val); +} + +void json_writer_double(JSONWriter *writer, const char *name, double val) +{ + maybe_comma_name(writer, name); + + /* + * FIXME: g_string_append_printf() is locale dependent; but JSON + * requires numbers to be formatted as if in the C locale. + * Dependence on C locale is a pervasive issue in QEMU. + */ + /* + * FIXME: This risks printing Inf or NaN, which are not valid + * JSON values. + */ + g_string_append_printf(writer->contents, "%.17g", val); +} + +void json_writer_str(JSONWriter *writer, const char *name, const char *str) +{ + maybe_comma_name(writer, name); + quoted_str(writer, str); +} diff --git a/qobject/meson.build b/qobject/meson.build index bb63c06b63..4683a852a2 100644 --- a/qobject/meson.build +++ b/qobject/meson.build @@ -1,3 +1,4 @@ -util_ss.add(files('qnull.c', 'qnum.c', 'qstring.c', 'qdict.c', 'qlist.c', 'qbool.c', - 'qlit.c', 'qjson.c', 'qobject.c', 'json-lexer.c', 'json-streamer.c', 'json-parser.c', +util_ss.add(files('qnull.c', 'qnum.c', 'qstring.c', 'qdict.c', + 'qlist.c', 'qbool.c', 'qlit.c', 'qjson.c', 'qobject.c', + 'json-writer.c', 'json-lexer.c', 'json-streamer.c', 'json-parser.c', 'block-qdict.c')) diff --git a/qobject/qjson.c b/qobject/qjson.c index 962214f5a7..bcc376e626 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -14,13 +14,13 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qapi/qmp/json-parser.h" +#include "qapi/qmp/json-writer.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qlist.h" #include "qapi/qmp/qnum.h" #include "qapi/qmp/qstring.h" -#include "qemu/unicode.h" typedef struct JSONParsingState { @@ -149,132 +149,69 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...) return qdict; } -static void json_pretty_newline(GString *accu, bool pretty, int indent) -{ - if (pretty) { - g_string_append_printf(accu, "\n%*s", indent * 4, ""); - } -} - -static void quoted_str(const char *str, GString *accu) -{ - const char *ptr; - int cp; - char *end; - - g_string_append_c(accu, '"'); - - for (ptr = str; *ptr; ptr = end) { - cp = mod_utf8_codepoint(ptr, 6, &end); - switch (cp) { - case '\"': - g_string_append(accu, "\\\""); - break; - case '\\': - g_string_append(accu, "\\\\"); - break; - case '\b': - g_string_append(accu, "\\b"); - break; - case '\f': - g_string_append(accu, "\\f"); - break; - case '\n': - g_string_append(accu, "\\n"); - break; - case '\r': - g_string_append(accu, "\\r"); - break; - case '\t': - g_string_append(accu, "\\t"); - break; - default: - if (cp < 0) { - cp = 0xFFFD; /* replacement character */ - } - if (cp > 0xFFFF) { - /* beyond BMP; need a surrogate pair */ - g_string_append_printf(accu, "\\u%04X\\u%04X", - 0xD800 + ((cp - 0x10000) >> 10), - 0xDC00 + ((cp - 0x10000) & 0x3FF)); - } else if (cp < 0x20 || cp >= 0x7F) { - g_string_append_printf(accu, "\\u%04X", cp); - } else { - g_string_append_c(accu, cp); - } - } - }; - - g_string_append_c(accu, '"'); -} - -static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) +static void to_json(JSONWriter *writer, const char *name, + const QObject *obj) { switch (qobject_type(obj)) { case QTYPE_QNULL: - g_string_append(accu, "null"); + json_writer_null(writer, name); break; case QTYPE_QNUM: { QNum *val = qobject_to(QNum, obj); - char *buffer = qnum_to_string(val); - g_string_append(accu, buffer); - g_free(buffer); + + switch (val->kind) { + case QNUM_I64: + json_writer_int64(writer, name, val->u.i64); + break; + case QNUM_U64: + json_writer_uint64(writer, name, val->u.u64); + break; + case QNUM_DOUBLE: + json_writer_double(writer, name, val->u.dbl); + break; + default: + abort(); + } break; } case QTYPE_QSTRING: { - quoted_str(qstring_get_str(qobject_to(QString, obj)), accu); + QString *val = qobject_to(QString, obj); + + json_writer_str(writer, name, qstring_get_str(val)); break; } case QTYPE_QDICT: { QDict *val = qobject_to(QDict, obj); - const char *comma = pretty ? "," : ", "; - const char *sep = ""; const QDictEntry *entry; - g_string_append_c(accu, '{'); + json_writer_start_object(writer, name); for (entry = qdict_first(val); entry; entry = qdict_next(val, entry)) { - g_string_append(accu, sep); - json_pretty_newline(accu, pretty, indent + 1); - quoted_str(qdict_entry_key(entry), accu); - g_string_append(accu, ": "); - to_json(qdict_entry_value(entry), accu, pretty, indent + 1); - sep = comma; + to_json(writer, qdict_entry_key(entry), qdict_entry_value(entry)); } - json_pretty_newline(accu, pretty, indent); - g_string_append_c(accu, '}'); + json_writer_end_object(writer); break; } case QTYPE_QLIST: { QList *val = qobject_to(QList, obj); - const char *comma = pretty ? "," : ", "; - const char *sep = ""; QListEntry *entry; - g_string_append_c(accu, '['); + json_writer_start_array(writer, name); QLIST_FOREACH_ENTRY(val, entry) { - g_string_append(accu, sep); - json_pretty_newline(accu, pretty, indent + 1); - to_json(qlist_entry_obj(entry), accu, pretty, indent + 1); - sep = comma; + to_json(writer, NULL, qlist_entry_obj(entry)); } - json_pretty_newline(accu, pretty, indent); - g_string_append_c(accu, ']'); + json_writer_end_array(writer); break; } case QTYPE_QBOOL: { QBool *val = qobject_to(QBool, obj); - if (qbool_get_bool(val)) { - g_string_append(accu, "true"); - } else { - g_string_append(accu, "false"); - } + json_writer_bool(writer, name, qbool_get_bool(val)); break; } default: @@ -284,10 +221,10 @@ static void to_json(const QObject *obj, GString *accu, bool pretty, int indent) GString *qobject_to_json_pretty(const QObject *obj, bool pretty) { - GString *accu = g_string_new(NULL); + JSONWriter *writer = json_writer_new(pretty); - to_json(obj, accu, pretty, 0); - return accu; + to_json(writer, NULL, obj); + return json_writer_get_and_free(writer); } GString *qobject_to_json(const QObject *obj) diff --git a/qobject/qnum.c b/qobject/qnum.c index 35ba41e61c..5dd66938dd 100644 --- a/qobject/qnum.c +++ b/qobject/qnum.c @@ -168,11 +168,6 @@ char *qnum_to_string(QNum *qn) case QNUM_U64: return g_strdup_printf("%" PRIu64, qn->u.u64); case QNUM_DOUBLE: - /* FIXME: g_strdup_printf() is locale dependent; but JSON requires - * numbers to be formatted as if in the C locale. Dependence - * on C locale is a pervasive issue in QEMU. */ - /* FIXME: This risks printing Inf or NaN, which are not valid - * JSON values. */ /* 17 digits suffice for IEEE double */ return g_strdup_printf("%.17g", qn->u.dbl); } From 3ddba9a9e9bedd20a0b60dcdbe86f16223555555 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:48 +0100 Subject: [PATCH 29/33] migration: Replace migration's JSON writer by the general one Commit 8118f0950f "migration: Append JSON description of migration stream" needs a JSON writer. The existing qobject_to_json() wasn't a good fit, because it requires building a QObject to convert. Instead, migration got its very own JSON writer, in commit 190c882ce2 "QJSON: Add JSON writer". It tacitly limits numbers to int64_t, and strings contents to characters that don't need escaping, unlike qobject_to_json(). The previous commit factored the JSON writer out of qobject_to_json(). Replace migration's JSON writer by it. Cc: Juan Quintela Cc: Dr. David Alan Gilbert Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-17-armbru@redhat.com> Reviewed-by: Dr. David Alan Gilbert --- hw/display/virtio-gpu.c | 2 +- hw/intc/s390_flic_kvm.c | 2 +- hw/nvram/eeprom93xx.c | 2 +- hw/nvram/fw_cfg.c | 2 +- hw/pci/msix.c | 2 +- hw/pci/pci.c | 4 +- hw/pci/shpc.c | 2 +- hw/rtc/twl92230.c | 2 +- hw/scsi/scsi-bus.c | 2 +- hw/usb/redirect.c | 7 +- hw/virtio/virtio.c | 4 +- include/migration/vmstate.h | 7 +- include/qapi/qmp/json-writer.h | 2 - include/qemu/typedefs.h | 4 +- migration/meson.build | 1 - migration/qjson.c | 114 --------------------------------- migration/qjson.h | 29 --------- migration/savevm.c | 53 +++++++-------- migration/vmstate-types.c | 38 +++++------ migration/vmstate.c | 52 +++++++-------- target/alpha/machine.c | 2 +- target/arm/machine.c | 6 +- target/avr/machine.c | 4 +- target/hppa/machine.c | 4 +- target/microblaze/machine.c | 2 +- target/mips/machine.c | 4 +- target/openrisc/machine.c | 2 +- target/ppc/machine.c | 10 +-- target/sparc/machine.c | 2 +- 29 files changed, 114 insertions(+), 253 deletions(-) delete mode 100644 migration/qjson.c delete mode 100644 migration/qjson.h diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index f3b71fa9c7..0e833a462b 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -970,7 +970,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = { }; static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { VirtIOGPU *g = opaque; struct virtio_gpu_simple_resource *res; diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index 35d91afa55..b3fb9f8395 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -386,7 +386,7 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs, * reached */ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { KVMS390FLICState *flic = opaque; int len = FLIC_SAVE_INITIAL_SIZE; diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c index ca6f591c84..a1b9c78844 100644 --- a/hw/nvram/eeprom93xx.c +++ b/hw/nvram/eeprom93xx.c @@ -104,7 +104,7 @@ static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size, } static int put_unused(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { fprintf(stderr, "uint16_from_uint8 is used only for backwards compatibility.\n"); fprintf(stderr, "Never should be used to write a new state.\n"); diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 44cb274a32..9b8dcca4ea 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -583,7 +583,7 @@ static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size, } static int put_unused(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { fprintf(stderr, "uint32_as_uint16 is only used for backward compatibility.\n"); fprintf(stderr, "This functions shouldn't be called.\n"); diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 36491ee52b..ae9331cd0b 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -634,7 +634,7 @@ void msix_unset_vector_notifiers(PCIDevice *dev) } static int put_msix_state(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { msix_save(pv, f); diff --git a/hw/pci/pci.c b/hw/pci/pci.c index d4349ea577..dd589cd7c7 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -559,7 +559,7 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size, /* just put buffer */ static int put_pci_config_device(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { const uint8_t **v = pv; assert(size == pci_config_size(container_of(pv, PCIDevice, config))); @@ -597,7 +597,7 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size, } static int put_pci_irq_state(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int i; PCIDevice *s = container_of(pv, PCIDevice, irq_state); diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c index b00dce629c..4786a44996 100644 --- a/hw/pci/shpc.c +++ b/hw/pci/shpc.c @@ -699,7 +699,7 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l) } static int shpc_save(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { PCIDevice *d = container_of(pv, PCIDevice, shpc); qemu_put_buffer(f, d->shpc->config, SHPC_SIZEOF(d)); diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c index f838913b37..d942908223 100644 --- a/hw/rtc/twl92230.c +++ b/hw/rtc/twl92230.c @@ -762,7 +762,7 @@ static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size, } static int put_int32_as_uint16(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int *v = pv; qemu_put_be16(f, *v); diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index edb5c3492a..c349fb7f2d 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1638,7 +1638,7 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev) /* SCSI request list. For simplicity, pv points to the whole device */ static int put_scsi_requests(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { SCSIDevice *s = pv; SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus); diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 3238de6bb8..4ca7d47ef7 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -2227,7 +2227,7 @@ static int usbredir_post_load(void *priv, int version_id) /* For usbredirparser migration */ static int usbredir_put_parser(QEMUFile *f, void *priv, size_t unused, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { USBRedirDevice *dev = priv; uint8_t *data; @@ -2294,7 +2294,7 @@ static const VMStateInfo usbredir_parser_vmstate_info = { /* For buffered packets (iso/irq) queue migration */ static int usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { struct endp_data *endp = priv; USBRedirDevice *dev = endp->dev; @@ -2421,7 +2421,8 @@ static const VMStateDescription usbredir_ep_vmstate = { /* For PacketIdQueue migration */ static int usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, + JSONWriter *vmdesc) { struct PacketIdQueue *q = priv; USBRedirDevice *dev = q->dev; diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index eff35fab7c..b308026596 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2745,7 +2745,7 @@ static int get_extra_state(QEMUFile *f, void *pv, size_t size, } static int put_extra_state(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { VirtIODevice *vdev = pv; BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); @@ -2919,7 +2919,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f) /* A wrapper for use as a VMState .put function */ static int virtio_device_put(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { return virtio_save(VIRTIO_DEVICE(opaque), f); } diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 4d71dc8fba..075ee80096 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -43,7 +43,7 @@ struct VMStateInfo { const char *name; int (*get)(QEMUFile *f, void *pv, size_t size, const VMStateField *field); int (*put)(QEMUFile *f, void *pv, size_t size, const VMStateField *field, - QJSON *vmdesc); + JSONWriter *vmdesc); }; enum VMStateFlags { @@ -1169,9 +1169,10 @@ extern const VMStateInfo vmstate_info_qlist; int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, void *opaque, int version_id); int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc); + void *opaque, JSONWriter *vmdesc); int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc, int version_id); + void *opaque, JSONWriter *vmdesc, + int version_id); bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque); diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h index 708d129018..b70ba64077 100644 --- a/include/qapi/qmp/json-writer.h +++ b/include/qapi/qmp/json-writer.h @@ -14,8 +14,6 @@ #ifndef JSON_WRITER_H #define JSON_WRITER_H -typedef struct JSONWriter JSONWriter; - JSONWriter *json_writer_new(bool pretty); const char *json_writer_get(JSONWriter *); GString *json_writer_get_and_free(JSONWriter *); diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 6281eae3b5..976b529dfb 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -57,8 +57,8 @@ typedef struct IOMMUMemoryRegion IOMMUMemoryRegion; typedef struct ISABus ISABus; typedef struct ISADevice ISADevice; typedef struct IsaDma IsaDma; +typedef struct JSONWriter JSONWriter; typedef struct MACAddr MACAddr; -typedef struct ReservedRegion ReservedRegion; typedef struct MachineClass MachineClass; typedef struct MachineState MachineState; typedef struct MemoryListener MemoryListener; @@ -107,7 +107,6 @@ typedef struct QEMUSGList QEMUSGList; typedef struct QemuSpin QemuSpin; typedef struct QEMUTimer QEMUTimer; typedef struct QEMUTimerListGroup QEMUTimerListGroup; -typedef struct QJSON QJSON; typedef struct QList QList; typedef struct QNull QNull; typedef struct QNum QNum; @@ -115,6 +114,7 @@ typedef struct QObject QObject; typedef struct QString QString; typedef struct RAMBlock RAMBlock; typedef struct Range Range; +typedef struct ReservedRegion ReservedRegion; typedef struct SavedIOTLB SavedIOTLB; typedef struct SHPCDevice SHPCDevice; typedef struct SSIBus SSIBus; diff --git a/migration/meson.build b/migration/meson.build index 980e37865c..291adc1337 100644 --- a/migration/meson.build +++ b/migration/meson.build @@ -6,7 +6,6 @@ migration_files = files( 'vmstate.c', 'qemu-file-channel.c', 'qemu-file.c', - 'qjson.c', ) softmmu_ss.add(migration_files) diff --git a/migration/qjson.c b/migration/qjson.c deleted file mode 100644 index e9889bdcb0..0000000000 --- a/migration/qjson.c +++ /dev/null @@ -1,114 +0,0 @@ -/* - * A simple JSON writer - * - * Copyright Alexander Graf - * - * Authors: - * Alexander Graf - * - * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. - * See the COPYING.LIB file in the top-level directory. - * - */ - -/* - * Type QJSON lets you build JSON text. Its interface mirrors (a - * subset of) abstract JSON syntax. - * - * It does *not* detect incorrect use. It happily produces invalid - * JSON then. This is what migration wants. - * - * QAPI output visitors also produce JSON text. However, they do - * assert their preconditions and invariants, and therefore abort on - * incorrect use. - */ - -#include "qemu/osdep.h" -#include "qapi/qmp/qstring.h" -#include "qjson.h" - -struct QJSON { - QString *str; - bool omit_comma; -}; - -static void json_emit_element(QJSON *json, const char *name) -{ - /* Check whether we need to print a , before an element */ - if (json->omit_comma) { - json->omit_comma = false; - } else { - qstring_append(json->str, ", "); - } - - if (name) { - qstring_append(json->str, "\""); - qstring_append(json->str, name); - qstring_append(json->str, "\" : "); - } -} - -void json_start_object(QJSON *json, const char *name) -{ - json_emit_element(json, name); - qstring_append(json->str, "{ "); - json->omit_comma = true; -} - -void json_end_object(QJSON *json) -{ - qstring_append(json->str, " }"); - json->omit_comma = false; -} - -void json_start_array(QJSON *json, const char *name) -{ - json_emit_element(json, name); - qstring_append(json->str, "[ "); - json->omit_comma = true; -} - -void json_end_array(QJSON *json) -{ - qstring_append(json->str, " ]"); - json->omit_comma = false; -} - -void json_prop_int(QJSON *json, const char *name, int64_t val) -{ - json_emit_element(json, name); - qstring_append_int(json->str, val); -} - -void json_prop_str(QJSON *json, const char *name, const char *str) -{ - json_emit_element(json, name); - qstring_append_chr(json->str, '"'); - qstring_append(json->str, str); - qstring_append_chr(json->str, '"'); -} - -const char *qjson_get_str(QJSON *json) -{ - return qstring_get_str(json->str); -} - -QJSON *qjson_new(void) -{ - QJSON *json = g_new0(QJSON, 1); - - json->str = qstring_from_str("{ "); - json->omit_comma = true; - return json; -} - -void qjson_finish(QJSON *json) -{ - json_end_object(json); -} - -void qjson_destroy(QJSON *json) -{ - qobject_unref(json->str); - g_free(json); -} diff --git a/migration/qjson.h b/migration/qjson.h deleted file mode 100644 index 1786bb5864..0000000000 --- a/migration/qjson.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * QEMU JSON writer - * - * Copyright Alexander Graf - * - * Authors: - * Alexander Graf - * - * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. - * See the COPYING.LIB file in the top-level directory. - * - */ -#ifndef QEMU_QJSON_H -#define QEMU_QJSON_H - -QJSON *qjson_new(void); -void qjson_destroy(QJSON *json); -void json_prop_str(QJSON *json, const char *name, const char *str); -void json_prop_int(QJSON *json, const char *name, int64_t val); -void json_end_array(QJSON *json); -void json_start_array(QJSON *json, const char *name); -void json_end_object(QJSON *json); -void json_start_object(QJSON *json, const char *name); -const char *qjson_get_str(QJSON *json); -void qjson_finish(QJSON *json); - -G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy) - -#endif /* QEMU_QJSON_H */ diff --git a/migration/savevm.c b/migration/savevm.c index 5f937a2762..138a96de4d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -42,6 +42,7 @@ #include "postcopy-ram.h" #include "qapi/error.h" #include "qapi/qapi-commands-migration.h" +#include "qapi/qmp/json-writer.h" #include "qapi/qmp/qerror.h" #include "qemu/error-report.h" #include "sysemu/cpus.h" @@ -58,7 +59,6 @@ #include "sysemu/runstate.h" #include "sysemu/sysemu.h" #include "sysemu/xen.h" -#include "qjson.h" #include "migration/colo.h" #include "qemu/bitmap.h" #include "net/announce.h" @@ -209,7 +209,7 @@ static int get_timer(QEMUFile *f, void *pv, size_t size, } static int put_timer(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { QEMUTimer *v = pv; timer_put(f, v); @@ -406,7 +406,7 @@ static int get_capability(QEMUFile *f, void *pv, size_t size, } static int put_capability(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { MigrationCapability *capability = pv; const char *capability_str = MigrationCapability_str(*capability); @@ -884,7 +884,8 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se) return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id); } -static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc) +static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, + JSONWriter *vmdesc) { int64_t old_offset, size; @@ -893,18 +894,19 @@ static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdes size = qemu_ftell_fast(f) - old_offset; if (vmdesc) { - json_prop_int(vmdesc, "size", size); - json_start_array(vmdesc, "fields"); - json_start_object(vmdesc, NULL); - json_prop_str(vmdesc, "name", "data"); - json_prop_int(vmdesc, "size", size); - json_prop_str(vmdesc, "type", "buffer"); - json_end_object(vmdesc); - json_end_array(vmdesc); + json_writer_int64(vmdesc, "size", size); + json_writer_start_array(vmdesc, "fields"); + json_writer_start_object(vmdesc, NULL); + json_writer_str(vmdesc, "name", "data"); + json_writer_int64(vmdesc, "size", size); + json_writer_str(vmdesc, "type", "buffer"); + json_writer_end_object(vmdesc); + json_writer_end_array(vmdesc); } } -static int vmstate_save(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc) +static int vmstate_save(QEMUFile *f, SaveStateEntry *se, + JSONWriter *vmdesc) { trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)"); if (!se->vmsd) { @@ -1357,14 +1359,15 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, bool in_postcopy, bool inactivate_disks) { - g_autoptr(QJSON) vmdesc = NULL; + g_autoptr(JSONWriter) vmdesc = NULL; int vmdesc_len; SaveStateEntry *se; int ret; - vmdesc = qjson_new(); - json_prop_int(vmdesc, "page_size", qemu_target_page_size()); - json_start_array(vmdesc, "devices"); + vmdesc = json_writer_new(false); + json_writer_start_object(vmdesc, NULL); + json_writer_int64(vmdesc, "page_size", qemu_target_page_size()); + json_writer_start_array(vmdesc, "devices"); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { if ((!se->ops || !se->ops->save_state) && !se->vmsd) { @@ -1377,9 +1380,9 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, trace_savevm_section_start(se->idstr, se->section_id); - json_start_object(vmdesc, NULL); - json_prop_str(vmdesc, "name", se->idstr); - json_prop_int(vmdesc, "instance_id", se->instance_id); + json_writer_start_object(vmdesc, NULL); + json_writer_str(vmdesc, "name", se->idstr); + json_writer_int64(vmdesc, "instance_id", se->instance_id); save_section_header(f, se, QEMU_VM_SECTION_FULL); ret = vmstate_save(f, se, vmdesc); @@ -1390,7 +1393,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, trace_savevm_section_end(se->idstr, se->section_id, 0); save_section_footer(f, se); - json_end_object(vmdesc); + json_writer_end_object(vmdesc); } if (inactivate_disks) { @@ -1409,14 +1412,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, qemu_put_byte(f, QEMU_VM_EOF); } - json_end_array(vmdesc); - qjson_finish(vmdesc); - vmdesc_len = strlen(qjson_get_str(vmdesc)); + json_writer_end_array(vmdesc); + json_writer_end_object(vmdesc); + vmdesc_len = strlen(json_writer_get(vmdesc)); if (should_send_vmdesc()) { qemu_put_byte(f, QEMU_VM_VMDESCRIPTION); qemu_put_be32(f, vmdesc_len); - qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len); + qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len); } return 0; diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c index e22d41d73d..bf4d440308 100644 --- a/migration/vmstate-types.c +++ b/migration/vmstate-types.c @@ -29,7 +29,7 @@ static int get_bool(QEMUFile *f, void *pv, size_t size, } static int put_bool(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { bool *v = pv; qemu_put_byte(f, *v); @@ -53,7 +53,7 @@ static int get_int8(QEMUFile *f, void *pv, size_t size, } static int put_int8(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int8_t *v = pv; qemu_put_s8s(f, v); @@ -77,7 +77,7 @@ static int get_int16(QEMUFile *f, void *pv, size_t size, } static int put_int16(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int16_t *v = pv; qemu_put_sbe16s(f, v); @@ -101,7 +101,7 @@ static int get_int32(QEMUFile *f, void *pv, size_t size, } static int put_int32(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int32_t *v = pv; qemu_put_sbe32s(f, v); @@ -178,7 +178,7 @@ static int get_int64(QEMUFile *f, void *pv, size_t size, } static int put_int64(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int64_t *v = pv; qemu_put_sbe64s(f, v); @@ -202,7 +202,7 @@ static int get_uint8(QEMUFile *f, void *pv, size_t size, } static int put_uint8(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint8_t *v = pv; qemu_put_8s(f, v); @@ -226,7 +226,7 @@ static int get_uint16(QEMUFile *f, void *pv, size_t size, } static int put_uint16(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint16_t *v = pv; qemu_put_be16s(f, v); @@ -250,7 +250,7 @@ static int get_uint32(QEMUFile *f, void *pv, size_t size, } static int put_uint32(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint32_t *v = pv; qemu_put_be32s(f, v); @@ -300,7 +300,7 @@ static int get_uint64(QEMUFile *f, void *pv, size_t size, } static int put_uint64(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint64_t *v = pv; qemu_put_be64s(f, v); @@ -325,7 +325,7 @@ static int get_nullptr(QEMUFile *f, void *pv, size_t size, } static int put_nullptr(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { if (pv == NULL) { @@ -432,7 +432,7 @@ static int get_cpudouble(QEMUFile *f, void *pv, size_t size, } static int put_cpudouble(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPU_DoubleU *v = pv; qemu_put_be32s(f, &v->l.upper); @@ -457,7 +457,7 @@ static int get_buffer(QEMUFile *f, void *pv, size_t size, } static int put_buffer(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint8_t *v = pv; qemu_put_buffer(f, v, size); @@ -488,7 +488,7 @@ static int get_unused_buffer(QEMUFile *f, void *pv, size_t size, } static int put_unused_buffer(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { static const uint8_t buf[1024]; int block_len; @@ -530,7 +530,7 @@ static int get_tmp(QEMUFile *f, void *pv, size_t size, } static int put_tmp(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { const VMStateDescription *vmsd = field->vmsd; void *tmp = g_malloc(size); @@ -573,7 +573,7 @@ static int get_bitmap(QEMUFile *f, void *pv, size_t size, } static int put_bitmap(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { unsigned long *bmp = pv; int i, idx = 0; @@ -637,7 +637,7 @@ static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, /* put for QTAILQ */ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { const VMStateDescription *vmsd = field->vmsd; /* offset of the QTAILQ entry in a QTAILQ element*/ @@ -670,7 +670,7 @@ struct put_gtree_data { QEMUFile *f; const VMStateDescription *key_vmsd; const VMStateDescription *val_vmsd; - QJSON *vmdesc; + JSONWriter *vmdesc; int ret; }; @@ -703,7 +703,7 @@ static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data) } static int put_gtree(QEMUFile *f, void *pv, size_t unused_size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { bool direct_key = (!field->start); const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[1]; @@ -819,7 +819,7 @@ const VMStateInfo vmstate_info_gtree = { }; static int put_qlist(QEMUFile *f, void *pv, size_t unused_size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { const VMStateDescription *vmsd = field->vmsd; /* offset of the QTAILQ entry in a QTAILQ element*/ diff --git a/migration/vmstate.c b/migration/vmstate.c index e9d2aef66b..05f87cdddc 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -14,14 +14,14 @@ #include "migration.h" #include "migration/vmstate.h" #include "savevm.h" +#include "qapi/qmp/json-writer.h" #include "qemu-file.h" #include "qemu/bitops.h" #include "qemu/error-report.h" #include "trace.h" -#include "qjson.h" static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc); + void *opaque, JSONWriter *vmdesc); static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, void *opaque); @@ -249,7 +249,8 @@ static bool vmsd_can_compress(const VMStateField *field) return true; } -static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc, +static void vmsd_desc_field_start(const VMStateDescription *vmsd, + JSONWriter *vmdesc, const VMStateField *field, int i, int max) { char *name, *old_name; @@ -270,25 +271,26 @@ static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON *vmdesc, g_free(old_name); } - json_start_object(vmdesc, NULL); - json_prop_str(vmdesc, "name", name); + json_writer_start_object(vmdesc, NULL); + json_writer_str(vmdesc, "name", name); if (is_array) { if (can_compress) { - json_prop_int(vmdesc, "array_len", max); + json_writer_int64(vmdesc, "array_len", max); } else { - json_prop_int(vmdesc, "index", i); + json_writer_int64(vmdesc, "index", i); } } - json_prop_str(vmdesc, "type", vmfield_get_type_name(field)); + json_writer_str(vmdesc, "type", vmfield_get_type_name(field)); if (field->flags & VMS_STRUCT) { - json_start_object(vmdesc, "struct"); + json_writer_start_object(vmdesc, "struct"); } g_free(name); } -static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc, +static void vmsd_desc_field_end(const VMStateDescription *vmsd, + JSONWriter *vmdesc, const VMStateField *field, size_t size, int i) { if (!vmdesc) { @@ -297,11 +299,11 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc, if (field->flags & VMS_STRUCT) { /* We printed a struct in between, close its child object */ - json_end_object(vmdesc); + json_writer_end_object(vmdesc); } - json_prop_int(vmdesc, "size", size); - json_end_object(vmdesc); + json_writer_int64(vmdesc, "size", size); + json_writer_end_object(vmdesc); } @@ -316,13 +318,13 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque) int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc_id) + void *opaque, JSONWriter *vmdesc_id) { return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id); } int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc, int version_id) + void *opaque, JSONWriter *vmdesc, int version_id) { int ret = 0; const VMStateField *field = vmsd->fields; @@ -339,9 +341,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, } if (vmdesc) { - json_prop_str(vmdesc, "vmsd_name", vmsd->name); - json_prop_int(vmdesc, "version", version_id); - json_start_array(vmdesc, "fields"); + json_writer_str(vmdesc, "vmsd_name", vmsd->name); + json_writer_int64(vmdesc, "version", version_id); + json_writer_start_array(vmdesc, "fields"); } while (field->name) { @@ -353,7 +355,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, int i, n_elems = vmstate_n_elems(opaque, field); int size = vmstate_size(opaque, field); int64_t old_offset, written_bytes; - QJSON *vmdesc_loop = vmdesc; + JSONWriter *vmdesc_loop = vmdesc; trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems); if (field->flags & VMS_POINTER) { @@ -413,7 +415,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, } if (vmdesc) { - json_end_array(vmdesc); + json_writer_end_array(vmdesc); } ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc); @@ -491,7 +493,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, } static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque, QJSON *vmdesc) + void *opaque, JSONWriter *vmdesc) { const VMStateDescription **sub = vmsd->subsections; bool vmdesc_has_subsections = false; @@ -507,11 +509,11 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, if (vmdesc) { /* Only create subsection array when we have any */ if (!vmdesc_has_subsections) { - json_start_array(vmdesc, "subsections"); + json_writer_start_array(vmdesc, "subsections"); vmdesc_has_subsections = true; } - json_start_object(vmdesc, NULL); + json_writer_start_object(vmdesc, NULL); } qemu_put_byte(f, QEMU_VM_SUBSECTION); @@ -525,14 +527,14 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, } if (vmdesc) { - json_end_object(vmdesc); + json_writer_end_object(vmdesc); } } sub++; } if (vmdesc_has_subsections) { - json_end_array(vmdesc); + json_writer_end_array(vmdesc); } return ret; diff --git a/target/alpha/machine.c b/target/alpha/machine.c index 9d20169d4f..2b7c8148ff 100644 --- a/target/alpha/machine.c +++ b/target/alpha/machine.c @@ -11,7 +11,7 @@ static int get_fpcr(QEMUFile *f, void *opaque, size_t size, } static int put_fpcr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPUAlphaState *env = opaque; qemu_put_be64(f, cpu_alpha_load_fpcr(env)); diff --git a/target/arm/machine.c b/target/arm/machine.c index c5a2114f51..581852bc53 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -27,7 +27,7 @@ static int get_fpscr(QEMUFile *f, void *opaque, size_t size, } static int put_fpscr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ARMCPU *cpu = opaque; CPUARMState *env = &cpu->env; @@ -573,7 +573,7 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size, } static int put_cpsr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ARMCPU *cpu = opaque; CPUARMState *env = &cpu->env; @@ -608,7 +608,7 @@ static int get_power(QEMUFile *f, void *opaque, size_t size, } static int put_power(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ARMCPU *cpu = opaque; diff --git a/target/avr/machine.c b/target/avr/machine.c index e315442787..de264f57c3 100644 --- a/target/avr/machine.c +++ b/target/avr/machine.c @@ -34,7 +34,7 @@ static int get_sreg(QEMUFile *f, void *opaque, size_t size, } static int put_sreg(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPUAVRState *env = opaque; uint8_t sreg = cpu_get_sreg(env); @@ -61,7 +61,7 @@ static int get_segment(QEMUFile *f, void *opaque, size_t size, } static int put_segment(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { uint32_t *ramp = opaque; uint8_t temp = *ramp >> 16; diff --git a/target/hppa/machine.c b/target/hppa/machine.c index b60b654efb..905991d7f9 100644 --- a/target/hppa/machine.c +++ b/target/hppa/machine.c @@ -52,7 +52,7 @@ static int get_psw(QEMUFile *f, void *opaque, size_t size, } static int put_psw(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPUHPPAState *env = opaque; qemu_put_betr(f, cpu_hppa_get_psw(env)); @@ -93,7 +93,7 @@ static int get_tlb(QEMUFile *f, void *opaque, size_t size, } static int put_tlb(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { hppa_tlb_entry *ent = opaque; uint32_t val = 0; diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c index c2074bbdfe..d24def3992 100644 --- a/target/microblaze/machine.c +++ b/target/microblaze/machine.c @@ -46,7 +46,7 @@ static int get_msr(QEMUFile *f, void *opaque, size_t size, } static int put_msr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPUMBState *env = container_of(opaque, CPUMBState, msr); diff --git a/target/mips/machine.c b/target/mips/machine.c index 5b23e3e912..77afe654e9 100644 --- a/target/mips/machine.c +++ b/target/mips/machine.c @@ -31,7 +31,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size, } static int put_fpr(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { int i; fpr_t *v = pv; @@ -156,7 +156,7 @@ static int get_tlb(QEMUFile *f, void *pv, size_t size, } static int put_tlb(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { r4k_tlb_t *v = pv; diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c index b92985d99b..6239725c4f 100644 --- a/target/openrisc/machine.c +++ b/target/openrisc/machine.c @@ -55,7 +55,7 @@ static int get_sr(QEMUFile *f, void *opaque, size_t size, } static int put_sr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { CPUOpenRISCState *env = opaque; qemu_put_be32(f, cpu_get_sr(env)); diff --git a/target/ppc/machine.c b/target/ppc/machine.c index d9d911b9b1..283db1d28a 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -134,7 +134,7 @@ static int get_avr(QEMUFile *f, void *pv, size_t size, } static int put_avr(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ppc_avr_t *v = pv; @@ -166,7 +166,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size, } static int put_fpr(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ppc_vsr_t *v = pv; @@ -197,7 +197,7 @@ static int get_vsr(QEMUFile *f, void *pv, size_t size, } static int put_vsr(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ppc_vsr_t *v = pv; @@ -455,7 +455,7 @@ static int get_vscr(QEMUFile *f, void *opaque, size_t size, } static int put_vscr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { PowerPCCPU *cpu = opaque; qemu_put_be32(f, helper_mfvscr(&cpu->env)); @@ -580,7 +580,7 @@ static int get_slbe(QEMUFile *f, void *pv, size_t size, } static int put_slbe(QEMUFile *f, void *pv, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { ppc_slb_t *v = pv; diff --git a/target/sparc/machine.c b/target/sparc/machine.c index f38cf229af..917375c3a1 100644 --- a/target/sparc/machine.c +++ b/target/sparc/machine.c @@ -68,7 +68,7 @@ static int get_psr(QEMUFile *f, void *opaque, size_t size, } static int put_psr(QEMUFile *f, void *opaque, size_t size, - const VMStateField *field, QJSON *vmdesc) + const VMStateField *field, JSONWriter *vmdesc) { SPARCCPU *cpu = opaque; CPUSPARCState *env = &cpu->env; From 68af4cc1213ec6057ac47765985b6704c6be7fd7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:49 +0100 Subject: [PATCH 30/33] json: Use GString instead of QString to accumulate strings QString supports modifying its string, but it's quite limited: you can only append. The remaining callers use it for building an initial string, never for modifying it later. Change parse_string() to do build the initial string with GString. This is another step towards making QString immutable. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-18-armbru@redhat.com> --- qobject/json-parser.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/qobject/json-parser.c b/qobject/json-parser.c index c0f521b56b..d351039b10 100644 --- a/qobject/json-parser.c +++ b/qobject/json-parser.c @@ -130,7 +130,7 @@ static int cvt4hex(const char *s) static QString *parse_string(JSONParserContext *ctxt, JSONToken *token) { const char *ptr = token->str; - QString *str; + GString *str; char quote; const char *beg; int cp, trailing; @@ -140,7 +140,7 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token) assert(*ptr == '"' || *ptr == '\''); quote = *ptr++; - str = qstring_new(); + str = g_string_new(NULL); while (*ptr != quote) { assert(*ptr); @@ -149,31 +149,31 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token) beg = ptr++; switch (*ptr++) { case '"': - qstring_append_chr(str, '"'); + g_string_append_c(str, '"'); break; case '\'': - qstring_append_chr(str, '\''); + g_string_append_c(str, '\''); break; case '\\': - qstring_append_chr(str, '\\'); + g_string_append_c(str, '\\'); break; case '/': - qstring_append_chr(str, '/'); + g_string_append_c(str, '/'); break; case 'b': - qstring_append_chr(str, '\b'); + g_string_append_c(str, '\b'); break; case 'f': - qstring_append_chr(str, '\f'); + g_string_append_c(str, '\f'); break; case 'n': - qstring_append_chr(str, '\n'); + g_string_append_c(str, '\n'); break; case 'r': - qstring_append_chr(str, '\r'); + g_string_append_c(str, '\r'); break; case 't': - qstring_append_chr(str, '\t'); + g_string_append_c(str, '\t'); break; case 'u': cp = cvt4hex(ptr); @@ -200,7 +200,7 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token) (int)(ptr - beg), beg); goto out; } - qstring_append(str, utf8_buf); + g_string_append(str, utf8_buf); break; default: parse_error(ctxt, token, "invalid escape sequence in string"); @@ -225,14 +225,14 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token) ptr = end; len = mod_utf8_encode(utf8_buf, sizeof(utf8_buf), cp); assert(len >= 0); - qstring_append(str, utf8_buf); + g_string_append(str, utf8_buf); } } - return str; + return qstring_from_gstring(str); out: - qobject_unref(str); + g_string_free(str, true); return NULL; } From 7ece42110d2cde04f8cbfbceec536340344eab4e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:50 +0100 Subject: [PATCH 31/33] keyval: Use GString to accumulate value strings QString supports modifying its string, but it's quite limited: you can only append. The remaining callers use it for building an initial string, never for modifying it later. Change keyval_parse_one() to do build the initial string with GString. This is another step towards making QString immutable. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-19-armbru@redhat.com> --- util/keyval.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/util/keyval.c b/util/keyval.c index 7f625ad33c..be34928813 100644 --- a/util/keyval.c +++ b/util/keyval.c @@ -189,7 +189,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params, QDict *cur; int ret; QObject *next; - QString *val; + GString *val; key = params; val_end = NULL; @@ -263,7 +263,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params, if (key == implied_key) { assert(!*s); - val = qstring_from_substr(params, 0, val_end - params); + val = g_string_new_len(params, val_end - params); s = val_end; if (*s == ',') { s++; @@ -276,7 +276,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params, } s++; - val = qstring_new(); + val = g_string_new(NULL); for (;;) { if (!*s) { break; @@ -286,11 +286,12 @@ static const char *keyval_parse_one(QDict *qdict, const char *params, break; } } - qstring_append_chr(val, *s++); + g_string_append_c(val, *s++); } } - if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) { + if (!keyval_parse_put(cur, key_in_cur, qstring_from_gstring(val), + key, key_end, errp)) { return NULL; } return s; From 18cf67c5e1d6e23042b752831aa6f9e4e478787b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:51 +0100 Subject: [PATCH 32/33] block: Use GString instead of QString to build filenames QString supports modifying its string, but it's quite limited: you can only append. Just one caller remains: bdrv_parse_filename_strip_prefix() uses it just for building an initial string. Change it to do build the initial string with GString. This is another step towards making QString immutable. Cc: Kevin Wolf Cc: Max Reitz Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-20-armbru@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 934d1bcc8f..27b2c3ba15 100644 --- a/block.c +++ b/block.c @@ -217,7 +217,7 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, /* Stripping the explicit protocol prefix may result in a protocol * prefix being (wrongly) detected (if the filename contains a colon) */ if (path_has_protocol(filename)) { - QString *fat_filename; + GString *fat_filename; /* This means there is some colon before the first slash; therefore, * this cannot be an absolute path */ @@ -225,12 +225,13 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, /* And we can thus fix the protocol detection issue by prefixing it * by "./" */ - fat_filename = qstring_from_str("./"); - qstring_append(fat_filename, filename); + fat_filename = g_string_new("./"); + g_string_append(fat_filename, filename); - assert(!path_has_protocol(qstring_get_str(fat_filename))); + assert(!path_has_protocol(fat_filename->str)); - qdict_put(options, "filename", fat_filename); + qdict_put(options, "filename", + qstring_from_gstring(fat_filename)); } else { /* If no protocol prefix was detected, we can use the shortened * filename as-is */ From 4ac76ba414ecb94f086d73621775d8b38b6f0a43 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Dec 2020 18:11:52 +0100 Subject: [PATCH 33/33] qobject: Make QString immutable The functions to modify a QString's string are all unused now. Drop them, and make the string immutable. Saves 16 bytes per QString on my system. Signed-off-by: Markus Armbruster Message-Id: <20201211171152.146877-21-armbru@redhat.com> --- include/qapi/qmp/qstring.h | 8 +---- qobject/qstring.c | 65 ++------------------------------------ tests/check-qobject.c | 3 +- tests/check-qstring.c | 16 ---------- 4 files changed, 4 insertions(+), 88 deletions(-) diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index 53567db6c0..1d8ba46936 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -17,19 +17,13 @@ struct QString { struct QObjectBase_ base; - char *string; - size_t length; - size_t capacity; + const char *string; }; QString *qstring_new(void); QString *qstring_from_str(const char *str); QString *qstring_from_substr(const char *str, size_t start, size_t end); QString *qstring_from_gstring(GString *gstr); -size_t qstring_get_length(const QString *qstring); const char *qstring_get_str(const QString *qstring); -void qstring_append_int(QString *qstring, int64_t value); -void qstring_append(QString *qstring, const char *str); -void qstring_append_chr(QString *qstring, int c); #endif /* QSTRING_H */ diff --git a/qobject/qstring.c b/qobject/qstring.c index ea86d80cf0..b4613899b9 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -24,14 +24,6 @@ QString *qstring_new(void) return qstring_from_str(""); } -/** - * qstring_get_length(): Get the length of a QString - */ -size_t qstring_get_length(const QString *qstring) -{ - return qstring->length; -} - /** * qstring_from_substr(): Create a new QString from a C string substring * @@ -42,18 +34,9 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end) QString *qstring; assert(start <= end); - qstring = g_malloc(sizeof(*qstring)); qobject_init(QOBJECT(qstring), QTYPE_QSTRING); - - qstring->length = end - start; - qstring->capacity = qstring->length; - - assert(qstring->capacity < SIZE_MAX); - qstring->string = g_malloc(qstring->capacity + 1); - memcpy(qstring->string, str + start, qstring->length); - qstring->string[qstring->length] = 0; - + qstring->string = g_strndup(str + start, end - start); return qstring; } @@ -79,55 +62,11 @@ QString *qstring_from_gstring(GString *gstr) qstring = g_malloc(sizeof(*qstring)); qobject_init(QOBJECT(qstring), QTYPE_QSTRING); - qstring->length = gstr->len; - qstring->capacity = gstr->allocated_len; qstring->string = g_string_free(gstr, false); return qstring; } -static void capacity_increase(QString *qstring, size_t len) -{ - if (qstring->capacity < (qstring->length + len)) { - assert(len <= SIZE_MAX - qstring->capacity); - qstring->capacity += len; - assert(qstring->capacity <= SIZE_MAX / 2); - qstring->capacity *= 2; /* use exponential growth */ - - qstring->string = g_realloc(qstring->string, qstring->capacity + 1); - } -} - -/* qstring_append(): Append a C string to a QString - */ -void qstring_append(QString *qstring, const char *str) -{ - size_t len = strlen(str); - - capacity_increase(qstring, len); - memcpy(qstring->string + qstring->length, str, len); - qstring->length += len; - qstring->string[qstring->length] = 0; -} - -void qstring_append_int(QString *qstring, int64_t value) -{ - char num[32]; - - snprintf(num, sizeof(num), "%" PRId64, value); - qstring_append(qstring, num); -} - -/** - * qstring_append_chr(): Append a C char to a QString - */ -void qstring_append_chr(QString *qstring, int c) -{ - capacity_increase(qstring, 1); - qstring->string[qstring->length++] = c; - qstring->string[qstring->length] = 0; -} - /** * qstring_get_str(): Return a pointer to the stored string * @@ -158,6 +97,6 @@ void qstring_destroy_obj(QObject *obj) assert(obj != NULL); qs = qobject_to(QString, obj); - g_free(qs->string); + g_free((char *)qs->string); g_free(qs); } diff --git a/tests/check-qobject.c b/tests/check-qobject.c index 6b6deaeb8b..c1713d15af 100644 --- a/tests/check-qobject.c +++ b/tests/check-qobject.c @@ -155,8 +155,7 @@ static void qobject_is_equal_string_test(void) str_case = qstring_from_str("Foo"); /* Should yield "foo" */ - str_built = qstring_from_substr("form", 0, 2); - qstring_append_chr(str_built, 'o'); + str_built = qstring_from_substr("buffoon", 3, 6); check_unequal(str_base, str_whitespace_0, str_whitespace_1, str_whitespace_2, str_whitespace_3, str_case); diff --git a/tests/check-qstring.c b/tests/check-qstring.c index 2d079921e3..4bf9772093 100644 --- a/tests/check-qstring.c +++ b/tests/check-qstring.c @@ -47,21 +47,6 @@ static void qstring_get_str_test(void) qobject_unref(qstring); } -static void qstring_append_chr_test(void) -{ - int i; - QString *qstring; - const char *str = "qstring append char unit-test"; - - qstring = qstring_new(); - - for (i = 0; str[i]; i++) - qstring_append_chr(qstring, str[i]); - - g_assert(strcmp(str, qstring_get_str(qstring)) == 0); - qobject_unref(qstring); -} - static void qstring_from_substr_test(void) { QString *qs; @@ -90,7 +75,6 @@ int main(int argc, char **argv) g_test_add_func("/public/from_str", qstring_from_str_test); g_test_add_func("/public/get_str", qstring_get_str_test); - g_test_add_func("/public/append_chr", qstring_append_chr_test); g_test_add_func("/public/from_substr", qstring_from_substr_test); g_test_add_func("/public/to_qstring", qobject_to_qstring_test);