[PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD()

[PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD()

[Markus Armbruster]

Markus Armbruster (10):
  error: Drop some obviously superfluous error_propagate()
  error: Drop a few superfluous ERRP_GUARD()
  error: Move ERRP_GUARD() to the beginning of the function
  monitor: Simplify monitor_fd_param()'s error handling
  monitor: Use ERRP_GUARD() in monitor_init()
  qemu-config: Make config_parse_qdict() return bool
  qemu-config: Use ERRP_GUARD() where obviously appropriate
  sockets: Use ERRP_GUARD() where obviously appropriate
  qapi: Use returned bool to check for failure (again)
  io: Tidy up fat-fingered parameter name

 include/io/channel.h             |  2 +-
 include/qemu/config-file.h       |  2 +-
 accel/kvm/kvm-all.c              |  5 +---
 block/blkdebug.c                 |  4 +--
 block/copy-before-write.c        |  1 -
 dump/dump.c                      |  2 --
 hw/arm/armsse.c                  |  3 +--
 hw/arm/virt.c                    | 14 ++++------
 hw/core/machine.c                |  3 +--
 hw/core/qdev-properties-system.c |  5 +---
 hw/core/qdev.c                   |  2 --
 hw/hyperv/vmbus.c                |  8 +++---
 hw/i386/pc.c                     |  5 +---
 hw/pci/msi.c                     |  1 -
 hw/remote/vfio-user-obj.c        |  1 -
 hw/virtio/vhost-vdpa.c           |  2 +-
 hw/virtio/virtio-balloon.c       | 20 +++++++-------
 hw/virtio/virtio-mem.c           | 10 ++-----
 iothread.c                       |  2 +-
 monitor/misc.c                   | 14 +++-------
 monitor/monitor.c                | 12 +++------
 monitor/qmp-cmds.c               |  4 +--
 net/colo-compare.c               | 13 +++------
 qga/commands-win32.c             |  8 +++---
 target/i386/kvm/kvm.c            |  5 +---
 ui/util.c                        |  1 -
 util/qemu-config.c               | 46 +++++++++++++++-----------------
 util/qemu-sockets.c              | 26 +++++++-----------
 util/thread-context.c            | 10 ++-----
 29 files changed, 80 insertions(+), 151 deletions(-)

-- 
2.37.3

[PATCH 01/10] error: Drop some obviously superfluous error_propagate()

[Markus Armbruster]

When error_propagate(errp, local_err) is the only reader of
@local_err, we can just as well change its writers to write @errp
directly, and drop the error_propagate() along with @local_err.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/virt.c        | 14 +++++---------
 hw/hyperv/vmbus.c    |  8 +++-----
 qga/commands-win32.c |  8 +++-----
 3 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b871350856..02d627a5ab 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2771,24 +2771,20 @@ static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev,
                                      DeviceState *dev, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
-    Error *local_err = NULL;
 
     if (!vms->acpi_dev) {
-        error_setg(&local_err,
+        error_setg(errp,
                    "memory hotplug is not enabled: missing acpi-ged device");
-        goto out;
+        return;
     }
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
-        error_setg(&local_err,
-                   "nvdimm device hot unplug is not supported yet.");
-        goto out;
+        error_setg(errp, "nvdimm device hot unplug is not supported yet.");
+        return;
     }
 
     hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev,
-                                   &local_err);
-out:
-    error_propagate(errp, local_err);
+                                   errp);
 }
 
 static void virt_dimm_unplug(HotplugHandler *hotplug_dev,
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 30bc04e1c4..5db3fc1192 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -2404,7 +2404,6 @@ static const TypeInfo vmbus_dev_type_info = {
 static void vmbus_realize(BusState *bus, Error **errp)
 {
     int ret = 0;
-    Error *local_err = NULL;
     VMBus *vmbus = VMBUS(bus);
 
     qemu_mutex_init(&vmbus->rx_queue_lock);
@@ -2415,13 +2414,13 @@ static void vmbus_realize(BusState *bus, Error **errp)
     ret = hyperv_set_msg_handler(VMBUS_MESSAGE_CONNECTION_ID,
                                  vmbus_recv_message, vmbus);
     if (ret != 0) {
-        error_setg(&local_err, "hyperv set message handler failed: %d", ret);
+        error_setg(errp, "hyperv set message handler failed: %d", ret);
         goto error_out;
     }
 
     ret = event_notifier_init(&vmbus->notifier, 0);
     if (ret != 0) {
-        error_setg(&local_err, "event notifier failed to init with %d", ret);
+        error_setg(errp, "event notifier failed to init with %d", ret);
         goto remove_msg_handler;
     }
 
@@ -2429,7 +2428,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
     ret = hyperv_set_event_flag_handler(VMBUS_EVENT_CONNECTION_ID,
                                         &vmbus->notifier);
     if (ret != 0) {
-        error_setg(&local_err, "hyperv set event handler failed with %d", ret);
+        error_setg(errp, "hyperv set event handler failed with %d", ret);
         goto clear_event_notifier;
     }
 
@@ -2441,7 +2440,6 @@ remove_msg_handler:
     hyperv_set_msg_handler(VMBUS_MESSAGE_CONNECTION_ID, NULL, NULL);
 error_out:
     qemu_mutex_destroy(&vmbus->rx_queue_lock);
-    error_propagate(errp, local_err);
 }
 
 static void vmbus_unrealize(BusState *bus)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index ec9f55b453..962db8e543 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -275,13 +275,12 @@ static void acquire_privilege(const char *name, Error **errp)
 {
     HANDLE token = NULL;
     TOKEN_PRIVILEGES priv;
-    Error *local_err = NULL;
 
     if (OpenProcessToken(GetCurrentProcess(),
         TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &token))
     {
         if (!LookupPrivilegeValue(NULL, name, &priv.Privileges[0].Luid)) {
-            error_setg(&local_err, QERR_QGA_COMMAND_FAILED,
+            error_setg(errp, QERR_QGA_COMMAND_FAILED,
                        "no luid for requested privilege");
             goto out;
         }
@@ -290,13 +289,13 @@ static void acquire_privilege(const char *name, Error **errp)
         priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
 
         if (!AdjustTokenPrivileges(token, FALSE, &priv, 0, NULL, 0)) {
-            error_setg(&local_err, QERR_QGA_COMMAND_FAILED,
+            error_setg(errp, QERR_QGA_COMMAND_FAILED,
                        "unable to acquire requested privilege");
             goto out;
         }
 
     } else {
-        error_setg(&local_err, QERR_QGA_COMMAND_FAILED,
+        error_setg(errp, QERR_QGA_COMMAND_FAILED,
                    "failed to open privilege token");
     }
 
@@ -304,7 +303,6 @@ out:
     if (token) {
         CloseHandle(token);
     }
-    error_propagate(errp, local_err);
 }
 
 static void execute_async(DWORD WINAPI (*func)(LPVOID), LPVOID opaque,
-- 
2.37.3

[PATCH 02/10] error: Drop a few superfluous ERRP_GUARD()

[Markus Armbruster]

include/qapi/error.h on ERRP_GUARD():

 * It must be used when the function dereferences @errp or passes
 * @errp to error_prepend(), error_vprepend(), or error_append_hint().
 * It is safe to use even when it's not needed, but please avoid
 * cluttering the source with useless code.

Clean up some of this clutter.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/copy-before-write.c | 1 -
 dump/dump.c               | 2 --
 hw/core/qdev.c            | 2 --
 hw/pci/msi.c              | 1 -
 hw/remote/vfio-user-obj.c | 1 -
 ui/util.c                 | 1 -
 6 files changed, 8 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 4abaa7339e..6f0157244f 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -522,7 +522,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockCopyState **bcs,
                                   Error **errp)
 {
-    ERRP_GUARD();
     BDRVCopyBeforeWriteState *state;
     BlockDriverState *top;
     QDict *opts;
diff --git a/dump/dump.c b/dump/dump.c
index df117c847f..c9afc30ce2 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -357,7 +357,6 @@ static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
 
 static void write_elf_phdr_note(DumpState *s, Error **errp)
 {
-    ERRP_GUARD();
     Elf32_Phdr phdr32;
     Elf64_Phdr phdr64;
     void *phdr;
@@ -773,7 +772,6 @@ static void dump_iterate(DumpState *s, Error **errp)
 static void dump_end(DumpState *s, Error **errp)
 {
     int rc;
-    ERRP_GUARD();
 
     if (s->elf_section_data_size) {
         s->elf_section_data = g_malloc0(s->elf_section_data_size);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0145501904..67be2feaf3 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -493,8 +493,6 @@ void qdev_del_unplug_blocker(DeviceState *dev, Error *reason)
 
 bool qdev_unplug_blocked(DeviceState *dev, Error **errp)
 {
-    ERRP_GUARD();
-
     if (dev->unplug_blockers) {
         error_propagate(errp, error_copy(dev->unplug_blockers->data));
         return true;
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 058d1d1ef1..1cadf150bc 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -317,7 +317,6 @@ bool msi_is_masked(const PCIDevice *dev, unsigned int vector)
 
 void msi_set_mask(PCIDevice *dev, int vector, bool mask, Error **errp)
 {
-    ERRP_GUARD();
     uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
     bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
     uint32_t irq_state, vector_mask, pending;
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 4e36bb8bcf..6d0310cec9 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -719,7 +719,6 @@ static void vfu_object_machine_done(Notifier *notifier, void *data)
  */
 static void vfu_object_init_ctx(VfuObject *o, Error **errp)
 {
-    ERRP_GUARD();
     DeviceState *dev = NULL;
     vfu_pci_type_t pci_type = VFU_PCI_TYPE_CONVENTIONAL;
     int ret;
diff --git a/ui/util.c b/ui/util.c
index 7e8fc1ea53..907d60e032 100644
--- a/ui/util.c
+++ b/ui/util.c
@@ -51,7 +51,6 @@ bool qemu_console_fill_device_address(QemuConsole *con,
                                       size_t size,
                                       Error **errp)
 {
-    ERRP_GUARD();
     DeviceState *dev = DEVICE(object_property_get_link(OBJECT(con),
                                                        "device",
                                                        &error_abort));
-- 
2.37.3

[PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function

[Markus Armbruster]

include/qapi/error.h advises to put ERRP_GUARD() right at the
beginning of the function, because only then can it guard the whole
function.  Clean up the few spots disregarding the advice.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/armsse.c        | 3 +--
 hw/core/machine.c      | 3 +--
 hw/virtio/vhost-vdpa.c | 2 +-
 iothread.c             | 2 +-
 monitor/qmp-cmds.c     | 4 ++--
 5 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index aecdeb9815..0202bad787 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -900,6 +900,7 @@ static qemu_irq armsse_get_common_irq_in(ARMSSE *s, int irqno)
 
 static void armsse_realize(DeviceState *dev, Error **errp)
 {
+    ERRP_GUARD();
     ARMSSE *s = ARM_SSE(dev);
     ARMSSEClass *asc = ARM_SSE_GET_CLASS(dev);
     const ARMSSEInfo *info = asc->info;
@@ -914,8 +915,6 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     DeviceState *dev_splitter;
     uint32_t addr_width_max;
 
-    ERRP_GUARD();
-
     if (!s->board_memory) {
         error_setg(errp, "memory property was not set");
         return;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8d34caa31d..2352861240 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -554,12 +554,11 @@ static void machine_get_mem(Object *obj, Visitor *v, const char *name,
 static void machine_set_mem(Object *obj, Visitor *v, const char *name,
                             void *opaque, Error **errp)
 {
+    ERRP_GUARD();
     MachineState *ms = MACHINE(obj);
     MachineClass *mc = MACHINE_GET_CLASS(obj);
     MemorySizeConfiguration *mem;
 
-    ERRP_GUARD();
-
     if (!visit_type_MemorySizeConfiguration(v, name, &mem, errp)) {
         return;
     }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7468e44b87..bc1c79b325 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -963,6 +963,7 @@ static bool vhost_vdpa_svq_map_rings(struct vhost_dev *dev,
                                      struct vhost_vring_addr *addr,
                                      Error **errp)
 {
+    ERRP_GUARD();
     DMAMap device_region, driver_region;
     struct vhost_vring_addr svq_addr;
     struct vhost_vdpa *v = dev->opaque;
@@ -971,7 +972,6 @@ static bool vhost_vdpa_svq_map_rings(struct vhost_dev *dev,
     size_t avail_offset;
     bool ok;
 
-    ERRP_GUARD();
     vhost_svq_get_vring_addr(svq, &svq_addr);
 
     driver_region = (DMAMap) {
diff --git a/iothread.c b/iothread.c
index 529194a566..3862a64471 100644
--- a/iothread.c
+++ b/iothread.c
@@ -155,8 +155,8 @@ static void iothread_init_gcontext(IOThread *iothread)
 
 static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
 {
-    IOThread *iothread = IOTHREAD(base);
     ERRP_GUARD();
+    IOThread *iothread = IOTHREAD(base);
 
     if (!iothread->ctx) {
         return;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 81c8fdadf8..686d562cad 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -474,9 +474,9 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
                             StatsFilter *filter, StatsRequest *request,
                             Error **errp)
 {
+    ERRP_GUARD();
     strList *targets = NULL;
     strList *names = NULL;
-    ERRP_GUARD();
 
     if (request) {
         if (request->provider != entry->provider) {
@@ -541,9 +541,9 @@ StatsSchemaList *qmp_query_stats_schemas(bool has_provider,
                                          StatsProvider provider,
                                          Error **errp)
 {
+    ERRP_GUARD();
     StatsSchemaList *stats_results = NULL;
     StatsCallbacks *entry;
-    ERRP_GUARD();
 
     QTAILQ_FOREACH(entry, &stats_callbacks, next) {
         if (!has_provider || provider == entry->provider) {
-- 
2.37.3

[PATCH 04/10] monitor: Simplify monitor_fd_param()'s error handling

[Markus Armbruster]

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

diff --git a/monitor/misc.c b/monitor/misc.c
index 205487e2b9..83d7f4ffde 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1086,6 +1086,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
         }
 
         fd = monfd->fd;
+        assert(fd >= 0);
 
         /* caller takes ownership of fd */
         QLIST_REMOVE(monfd, next);
@@ -1403,23 +1404,16 @@ void monitor_fdset_dup_fd_remove(int dup_fd)
 int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp)
 {
     int fd;
-    Error *local_err = NULL;
 
     if (!qemu_isdigit(fdname[0]) && mon) {
-        fd = monitor_get_fd(mon, fdname, &local_err);
+        fd = monitor_get_fd(mon, fdname, errp);
     } else {
         fd = qemu_parse_fd(fdname);
-        if (fd == -1) {
-            error_setg(&local_err, "Invalid file descriptor number '%s'",
+        if (fd < 0) {
+            error_setg(errp, "Invalid file descriptor number '%s'",
                        fdname);
         }
     }
-    if (local_err) {
-        error_propagate(errp, local_err);
-        assert(fd == -1);
-    } else {
-        assert(fd != -1);
-    }
 
     return fd;
 }
-- 
2.37.3

[PATCH 05/10] monitor: Use ERRP_GUARD() in monitor_init()

[Markus Armbruster]

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

diff --git a/monitor/monitor.c b/monitor/monitor.c
index 86949024f6..7ed7bd5342 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -711,8 +711,8 @@ void monitor_init_globals_core(void)
 
 int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
 {
+    ERRP_GUARD();
     Chardev *chr;
-    Error *local_err = NULL;
 
     chr = qemu_chr_find(opts->chardev);
     if (chr == NULL) {
@@ -726,7 +726,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
 
     switch (opts->mode) {
     case MONITOR_MODE_CONTROL:
-        monitor_init_qmp(chr, opts->pretty, &local_err);
+        monitor_init_qmp(chr, opts->pretty, errp);
         break;
     case MONITOR_MODE_READLINE:
         if (!allow_hmp) {
@@ -737,17 +737,13 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
             error_setg(errp, "'pretty' is not compatible with HMP monitors");
             return -1;
         }
-        monitor_init_hmp(chr, true, &local_err);
+        monitor_init_hmp(chr, true, errp);
         break;
     default:
         g_assert_not_reached();
     }
 
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return -1;
-    }
-    return 0;
+    return *errp ? -1 : 0;
 }
 
 int monitor_init_opts(QemuOpts *opts, Error **errp)
-- 
2.37.3

[PATCH 06/10] qemu-config: Make config_parse_qdict() return bool

[Markus Armbruster]

This simplifies error checking.

Cc: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/config-file.h |  2 +-
 block/blkdebug.c           |  4 +---
 util/qemu-config.c         | 39 ++++++++++++++++++--------------------
 3 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 321e7c7c03..b82a778123 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -22,7 +22,7 @@ int qemu_read_config_file(const char *filename, QEMUConfigCB *f, Error **errp);
 
 /* Parse QDict options as a replacement for a config file (allowing multiple
    enumerated (0..(n-1)) configuration "sections") */
-void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
+bool qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
                              Error **errp);
 
 #endif /* QEMU_CONFIG_FILE_H */
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 4265ca125e..ca65b043f0 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -297,9 +297,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
         }
     }
 
-    qemu_config_parse_qdict(options, config_groups, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (!qemu_config_parse_qdict(options, config_groups, errp)) {
         ret = -EINVAL;
         goto fail;
     }
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 433488aa56..e983607b46 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -423,12 +423,12 @@ int qemu_read_config_file(const char *filename, QEMUConfigCB *cb, Error **errp)
     return ret;
 }
 
-static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
+static bool config_parse_qdict_section(QDict *options, QemuOptsList *opts,
                                        Error **errp)
 {
     QemuOpts *subopts;
-    QDict *subqdict;
-    QList *list = NULL;
+    g_autoptr(QDict) subqdict = NULL;
+    g_autoptr(QList) list = NULL;
     size_t orig_size, enum_size;
     char *prefix;
 
@@ -437,23 +437,23 @@ static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
     g_free(prefix);
     orig_size = qdict_size(subqdict);
     if (!orig_size) {
-        goto out;
+        return true;
     }
 
     subopts = qemu_opts_create(opts, NULL, 0, errp);
     if (!subopts) {
-        goto out;
+        return false;
     }
 
     if (!qemu_opts_absorb_qdict(subopts, subqdict, errp)) {
-        goto out;
+        return false;
     }
 
     enum_size = qdict_size(subqdict);
     if (enum_size < orig_size && enum_size) {
         error_setg(errp, "Unknown option '%s' for [%s]",
                    qdict_first(subqdict)->key, opts->name);
-        goto out;
+        return false;
     }
 
     if (enum_size) {
@@ -468,7 +468,7 @@ static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
         if (qdict_size(subqdict)) {
             error_setg(errp, "Unused option '%s' for [%s]",
                        qdict_first(subqdict)->key, opts->name);
-            goto out;
+            return false;
         }
 
         QLIST_FOREACH_ENTRY(list, list_entry) {
@@ -478,46 +478,43 @@ static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
             if (!section) {
                 error_setg(errp, "[%s] section (index %u) does not consist of "
                            "keys", opts->name, i);
-                goto out;
+                return false;
             }
 
             opt_name = g_strdup_printf("%s.%u", opts->name, i++);
             subopts = qemu_opts_create(opts, opt_name, 1, errp);
             g_free(opt_name);
             if (!subopts) {
-                goto out;
+                return false;
             }
 
             if (!qemu_opts_absorb_qdict(subopts, section, errp)) {
                 qemu_opts_del(subopts);
-                goto out;
+                return false;
             }
 
             if (qdict_size(section)) {
                 error_setg(errp, "[%s] section doesn't support the option '%s'",
                            opts->name, qdict_first(section)->key);
                 qemu_opts_del(subopts);
-                goto out;
+                return false;
             }
         }
     }
 
-out:
-    qobject_unref(subqdict);
-    qobject_unref(list);
+    return true;
 }
 
-void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
+bool qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
                              Error **errp)
 {
     int i;
-    Error *local_err = NULL;
 
     for (i = 0; lists[i]; i++) {
-        config_parse_qdict_section(options, lists[i], &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
+        if (!config_parse_qdict_section(options, lists[i], errp)) {
+            return false;
         }
     }
+
+    return true;
 }
-- 
2.37.3

[PATCH 07/10] qemu-config: Use ERRP_GUARD() where obviously appropriate

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/qemu-config.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index e983607b46..8c907fa83b 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -318,9 +318,9 @@ void qemu_add_opts(QemuOptsList *list)
 static int qemu_config_foreach(FILE *fp, QEMUConfigCB *cb, void *opaque,
                                const char *fname, Error **errp)
 {
+    ERRP_GUARD();
     char line[1024], prev_group[64], group[64], arg[64], value[1024];
     Location loc;
-    Error *local_err = NULL;
     QDict *qdict = NULL;
     int res = -EINVAL, lno = 0;
     int count = 0;
@@ -348,10 +348,9 @@ static int qemu_config_foreach(FILE *fp, QEMUConfigCB *cb, void *opaque,
             }
             if (qdict != prev) {
                 if (prev) {
-                    cb(prev_group, prev, opaque, &local_err);
+                    cb(prev_group, prev, opaque, errp);
                     qobject_unref(prev);
-                    if (local_err) {
-                        error_propagate(errp, local_err);
+                    if (*errp) {
                         goto out;
                     }
                 }
-- 
2.37.3

[PATCH 08/10] sockets: Use ERRP_GUARD() where obviously appropriate

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/qemu-sockets.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index d185245023..6538859b87 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -210,7 +210,8 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
                              int num,
                              Error **errp)
 {
-    struct addrinfo ai,*res,*e;
+    ERRP_GUARD();
+    struct addrinfo ai, *res, *e;
     char port[33];
     char uaddr[INET6_ADDRSTRLEN+1];
     char uport[33];
@@ -218,7 +219,6 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
     int slisten = -1;
     int saved_errno = 0;
     bool socket_created = false;
-    Error *err = NULL;
 
     if (saddr->keep_alive) {
         error_setg(errp, "keep-alive option is not supported for passive "
@@ -231,11 +231,9 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
     if (saddr->has_numeric && saddr->numeric) {
         ai.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
     }
-    ai.ai_family = inet_ai_family_from_address(saddr, &err);
     ai.ai_socktype = SOCK_STREAM;
-
-    if (err) {
-        error_propagate(errp, err);
+    ai.ai_family = inet_ai_family_from_address(saddr, errp);
+    if (*errp) {
         return -1;
     }
 
@@ -392,9 +390,9 @@ static int inet_connect_addr(const InetSocketAddress *saddr,
 static struct addrinfo *inet_parse_connect_saddr(InetSocketAddress *saddr,
                                                  Error **errp)
 {
+    ERRP_GUARD();
     struct addrinfo ai, *res;
     int rc;
-    Error *err = NULL;
     static int useV4Mapped = 1;
 
     memset(&ai, 0, sizeof(ai));
@@ -403,11 +401,9 @@ static struct addrinfo *inet_parse_connect_saddr(InetSocketAddress *saddr,
     if (qatomic_read(&useV4Mapped)) {
         ai.ai_flags |= AI_V4MAPPED;
     }
-    ai.ai_family = inet_ai_family_from_address(saddr, &err);
     ai.ai_socktype = SOCK_STREAM;
-
-    if (err) {
-        error_propagate(errp, err);
+    ai.ai_family = inet_ai_family_from_address(saddr, errp);
+    if (*errp) {
         return NULL;
     }
 
@@ -499,20 +495,18 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
                             InetSocketAddress *sladdr,
                             Error **errp)
 {
+    ERRP_GUARD();
     struct addrinfo ai, *peer = NULL, *local = NULL;
     const char *addr;
     const char *port;
     int sock = -1, rc;
-    Error *err = NULL;
 
     /* lookup peer addr */
     memset(&ai,0, sizeof(ai));
     ai.ai_flags = AI_CANONNAME | AI_V4MAPPED | AI_ADDRCONFIG;
-    ai.ai_family = inet_ai_family_from_address(sraddr, &err);
     ai.ai_socktype = SOCK_DGRAM;
-
-    if (err) {
-        error_propagate(errp, err);
+    ai.ai_family = inet_ai_family_from_address(sraddr, errp);
+    if (*errp) {
         goto err;
     }
 
-- 
2.37.3

[PATCH 09/10] qapi: Use returned bool to check for failure (again)

[Markus Armbruster]

Commit 012d4c96e2 changed the visitor functions taking Error ** to
return bool instead of void, and the commits following it used the new
return value to simplify error checking.  Since then a few more uses
in need of the same treatment crept in.  Do that.  All pretty
mechanical except for

* balloon_stats_get_all()

  This is basically the same transformation commit 012d4c96e2 applied
  to the virtual walk example in include/qapi/visitor.h.

* set_max_queue_size()

  Additionally replace "goto end of function" by return.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 accel/kvm/kvm-all.c              |  5 +----
 hw/core/qdev-properties-system.c |  5 +----
 hw/i386/pc.c                     |  5 +----
 hw/virtio/virtio-balloon.c       | 20 +++++++++-----------
 hw/virtio/virtio-mem.c           | 10 ++--------
 net/colo-compare.c               | 13 ++++---------
 target/i386/kvm/kvm.c            |  5 +----
 util/thread-context.c            | 10 ++--------
 8 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f99b0becd8..e86c33e0e6 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3586,7 +3586,6 @@ static void kvm_set_dirty_ring_size(Object *obj, Visitor *v,
                                     Error **errp)
 {
     KVMState *s = KVM_STATE(obj);
-    Error *error = NULL;
     uint32_t value;
 
     if (s->fd != -1) {
@@ -3594,9 +3593,7 @@ static void kvm_set_dirty_ring_size(Object *obj, Visitor *v,
         return;
     }
 
-    visit_type_uint32(v, name, &value, &error);
-    if (error) {
-        error_propagate(errp, error);
+    if (!visit_type_uint32(v, name, &value, errp)) {
         return;
     }
     if (value & (value - 1)) {
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index a91f60567a..97a968f477 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -679,14 +679,11 @@ static void set_reserved_region(Object *obj, Visitor *v, const char *name,
 {
     Property *prop = opaque;
     ReservedRegion *rr = object_field_prop_ptr(obj, prop);
-    Error *local_err = NULL;
     const char *endptr;
     char *str;
     int ret;
 
-    visit_type_str(v, name, &str, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (!visit_type_str(v, name, &str, errp)) {
         return;
     }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 546b703cb4..fa69b6f43e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1782,12 +1782,9 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
                                        Error **errp)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
-    Error *error = NULL;
     uint64_t value;
 
-    visit_type_size(v, name, &value, &error);
-    if (error) {
-        error_propagate(errp, error);
+    if (!visit_type_size(v, name, &value, errp)) {
         return;
     }
 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 73ac5eb675..746f07c4d2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -241,36 +241,34 @@ static void balloon_stats_poll_cb(void *opaque)
 static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
-    Error *err = NULL;
     VirtIOBalloon *s = VIRTIO_BALLOON(obj);
+    bool ok = false;
     int i;
 
-    if (!visit_start_struct(v, name, NULL, 0, &err)) {
-        goto out;
+    if (!visit_start_struct(v, name, NULL, 0, errp)) {
+        return;
     }
-    if (!visit_type_int(v, "last-update", &s->stats_last_update, &err)) {
+    if (!visit_type_int(v, "last-update", &s->stats_last_update, errp)) {
         goto out_end;
     }
 
-    if (!visit_start_struct(v, "stats", NULL, 0, &err)) {
+    if (!visit_start_struct(v, "stats", NULL, 0, errp)) {
         goto out_end;
     }
     for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
-        if (!visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err)) {
+        if (!visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], errp)) {
             goto out_nested;
         }
     }
-    visit_check_struct(v, &err);
+    ok = visit_check_struct(v, errp);
 out_nested:
     visit_end_struct(v, NULL);
 
-    if (!err) {
-        visit_check_struct(v, &err);
+    if (ok) {
+        visit_check_struct(v, errp);
     }
 out_end:
     visit_end_struct(v, NULL);
-out:
-    error_propagate(errp, err);
 }
 
 static void balloon_stats_get_poll_interval(Object *obj, Visitor *v,
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ed170def48..d96bde1fab 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -1094,12 +1094,9 @@ static void virtio_mem_set_requested_size(Object *obj, Visitor *v,
                                           Error **errp)
 {
     VirtIOMEM *vmem = VIRTIO_MEM(obj);
-    Error *err = NULL;
     uint64_t value;
 
-    visit_type_size(v, name, &value, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!visit_type_size(v, name, &value, errp)) {
         return;
     }
 
@@ -1159,7 +1156,6 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name,
                                       void *opaque, Error **errp)
 {
     VirtIOMEM *vmem = VIRTIO_MEM(obj);
-    Error *err = NULL;
     uint64_t value;
 
     if (DEVICE(obj)->realized) {
@@ -1167,9 +1163,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name,
         return;
     }
 
-    visit_type_size(v, name, &value, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!visit_type_size(v, name, &value, errp)) {
         return;
     }
 
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 787c740f14..7f9e6f89ce 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1135,22 +1135,17 @@ static void set_max_queue_size(Object *obj, Visitor *v,
                                const char *name, void *opaque,
                                Error **errp)
 {
-    Error *local_err = NULL;
     uint64_t value;
 
-    visit_type_uint64(v, name, &value, &local_err);
-    if (local_err) {
-        goto out;
+    if (!visit_type_uint64(v, name, &value, errp)) {
+        return;
     }
     if (!value) {
-        error_setg(&local_err, "Property '%s.%s' requires a positive value",
+        error_setg(errp, "Property '%s.%s' requires a positive value",
                    object_get_typename(obj), name);
-        goto out;
+        return;
     }
     max_queue_size = value;
-
-out:
-    error_propagate(errp, local_err);
 }
 
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a213209379..0ab4e0734a 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -5689,7 +5689,6 @@ static void kvm_arch_set_notify_window(Object *obj, Visitor *v,
                                        Error **errp)
 {
     KVMState *s = KVM_STATE(obj);
-    Error *error = NULL;
     uint32_t value;
 
     if (s->fd != -1) {
@@ -5697,9 +5696,7 @@ static void kvm_arch_set_notify_window(Object *obj, Visitor *v,
         return;
     }
 
-    visit_type_uint32(v, name, &value, &error);
-    if (error) {
-        error_propagate(errp, error);
+    if (!visit_type_uint32(v, name, &value, errp)) {
         return;
     }
 
diff --git a/util/thread-context.c b/util/thread-context.c
index 4138245332..2bc7883b9e 100644
--- a/util/thread-context.c
+++ b/util/thread-context.c
@@ -90,16 +90,13 @@ static void thread_context_set_cpu_affinity(Object *obj, Visitor *v,
     uint16List *l, *host_cpus = NULL;
     unsigned long *bitmap = NULL;
     int nbits = 0, ret;
-    Error *err = NULL;
 
     if (tc->init_cpu_bitmap) {
         error_setg(errp, "Mixing CPU and node affinity not supported");
         return;
     }
 
-    visit_type_uint16List(v, name, &host_cpus, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!visit_type_uint16List(v, name, &host_cpus, errp)) {
         return;
     }
 
@@ -178,7 +175,6 @@ static void thread_context_set_node_affinity(Object *obj, Visitor *v,
     uint16List *l, *host_nodes = NULL;
     unsigned long *bitmap = NULL;
     struct bitmask *tmp_cpus;
-    Error *err = NULL;
     int ret, i;
 
     if (tc->init_cpu_bitmap) {
@@ -186,9 +182,7 @@ static void thread_context_set_node_affinity(Object *obj, Visitor *v,
         return;
     }
 
-    visit_type_uint16List(v, name, &host_nodes, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!visit_type_uint16List(v, name, &host_nodes, errp)) {
         return;
     }
 
-- 
2.37.3

[PATCH 10/10] io: Tidy up fat-fingered parameter name

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/io/channel.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee7480..f1b7e05f81 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -350,7 +350,7 @@ int qio_channel_readv_all(QIOChannel *ioc,
 int qio_channel_writev_all(QIOChannel *ioc,
                            const struct iovec *iov,
                            size_t niov,
-                           Error **erp);
+                           Error **errp);
 
 /**
  * qio_channel_readv:
-- 
2.37.3

Re: [PATCH 01/10] error: Drop some obviously superfluous error_propagate()

[Philippe Mathieu-Daudé]

On 21/11/22 09:50, Markus Armbruster wrote:
> When error_propagate(errp, local_err) is the only reader of
> @local_err, we can just as well change its writers to write @errp
> directly, and drop the error_propagate() along with @local_err.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/arm/virt.c        | 14 +++++---------
>   hw/hyperv/vmbus.c    |  8 +++-----
>   qga/commands-win32.c |  8 +++-----
>   3 files changed, 11 insertions(+), 19 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 09/10] qapi: Use returned bool to check for failure (again)

[Philippe Mathieu-Daudé]

On 21/11/22 09:50, Markus Armbruster wrote:
> Commit 012d4c96e2 changed the visitor functions taking Error ** to
> return bool instead of void, and the commits following it used the new
> return value to simplify error checking.  Since then a few more uses
> in need of the same treatment crept in.  Do that.  All pretty
> mechanical except for
> 
> * balloon_stats_get_all()
> 
>    This is basically the same transformation commit 012d4c96e2 applied
>    to the virtual walk example in include/qapi/visitor.h.
> 
> * set_max_queue_size()
> 
>    Additionally replace "goto end of function" by return.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   accel/kvm/kvm-all.c              |  5 +----
>   hw/core/qdev-properties-system.c |  5 +----
>   hw/i386/pc.c                     |  5 +----
>   hw/virtio/virtio-balloon.c       | 20 +++++++++-----------
>   hw/virtio/virtio-mem.c           | 10 ++--------
>   net/colo-compare.c               | 13 ++++---------
>   target/i386/kvm/kvm.c            |  5 +----
>   util/thread-context.c            | 10 ++--------
>   8 files changed, 21 insertions(+), 52 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 10/10] io: Tidy up fat-fingered parameter name

[Philippe Mathieu-Daudé]

On 21/11/22 09:50, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/io/channel.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function

[Philippe Mathieu-Daudé]

On 21/11/22 09:50, Markus Armbruster wrote:
> include/qapi/error.h advises to put ERRP_GUARD() right at the
> beginning of the function, because only then can it guard the whole
> function.  Clean up the few spots disregarding the advice.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/arm/armsse.c        | 3 +--
>   hw/core/machine.c      | 3 +--
>   hw/virtio/vhost-vdpa.c | 2 +-
>   iothread.c             | 2 +-
>   monitor/qmp-cmds.c     | 4 ++--
>   5 files changed, 6 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>