* [PATCH 01/10] error: Drop some obviously superfluous error_propagate()
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 11:14 ` Philippe Mathieu-Daudé
2022-11-21 8:50 ` [PATCH 02/10] error: Drop a few superfluous ERRP_GUARD() Markus Armbruster
` (8 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 01/10] error: Drop some obviously superfluous error_propagate()
2022-11-21 8:50 ` [PATCH 01/10] error: Drop some obviously superfluous error_propagate() Markus Armbruster
@ 2022-11-21 11:14 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 11:14 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: eblake, vsementsov
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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 02/10] error: Drop a few superfluous ERRP_GUARD()
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
2022-11-21 8:50 ` [PATCH 01/10] error: Drop some obviously superfluous error_propagate() Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function Markus Armbruster
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
2022-11-21 8:50 ` [PATCH 01/10] error: Drop some obviously superfluous error_propagate() Markus Armbruster
2022-11-21 8:50 ` [PATCH 02/10] error: Drop a few superfluous ERRP_GUARD() Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 11:19 ` Philippe Mathieu-Daudé
2022-11-21 8:50 ` [PATCH 04/10] monitor: Simplify monitor_fd_param()'s error handling Markus Armbruster
` (6 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function
2022-11-21 8:50 ` [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function Markus Armbruster
@ 2022-11-21 11:19 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 11:19 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: eblake, vsementsov
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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 04/10] monitor: Simplify monitor_fd_param()'s error handling
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (2 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 05/10] monitor: Use ERRP_GUARD() in monitor_init() Markus Armbruster
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov, Dr . David Alan Gilbert
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/10] monitor: Use ERRP_GUARD() in monitor_init()
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (3 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 04/10] monitor: Simplify monitor_fd_param()'s error handling Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 06/10] qemu-config: Make config_parse_qdict() return bool Markus Armbruster
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov, Dr . David Alan Gilbert
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/10] qemu-config: Make config_parse_qdict() return bool
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (4 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 05/10] monitor: Use ERRP_GUARD() in monitor_init() Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 07/10] qemu-config: Use ERRP_GUARD() where obviously appropriate Markus Armbruster
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov, Hanna Reitz
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/10] qemu-config: Use ERRP_GUARD() where obviously appropriate
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (5 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 06/10] qemu-config: Make config_parse_qdict() return bool Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 08/10] sockets: " Markus Armbruster
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/10] sockets: Use ERRP_GUARD() where obviously appropriate
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (6 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 07/10] qemu-config: Use ERRP_GUARD() where obviously appropriate Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 8:50 ` [PATCH 09/10] qapi: Use returned bool to check for failure (again) Markus Armbruster
2022-11-21 8:50 ` [PATCH 10/10] io: Tidy up fat-fingered parameter name Markus Armbruster
9 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/10] qapi: Use returned bool to check for failure (again)
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (7 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 08/10] sockets: " Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 11:19 ` Philippe Mathieu-Daudé
2022-11-21 8:50 ` [PATCH 10/10] io: Tidy up fat-fingered parameter name Markus Armbruster
9 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 09/10] qapi: Use returned bool to check for failure (again)
2022-11-21 8:50 ` [PATCH 09/10] qapi: Use returned bool to check for failure (again) Markus Armbruster
@ 2022-11-21 11:19 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 11:19 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: eblake, vsementsov
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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 10/10] io: Tidy up fat-fingered parameter name
2022-11-21 8:50 [PATCH 00/10] Cleanups around error_propagate() and ERRP_GUARD() Markus Armbruster
` (8 preceding siblings ...)
2022-11-21 8:50 ` [PATCH 09/10] qapi: Use returned bool to check for failure (again) Markus Armbruster
@ 2022-11-21 8:50 ` Markus Armbruster
2022-11-21 11:19 ` Philippe Mathieu-Daudé
9 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2022-11-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, vsementsov
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
^ permalink raw reply related [flat|nested] 15+ messages in thread