* [PATCH v2 00/25] Error handling fixes & cleanups
@ 2020-06-24 8:37 Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 01/25] net/virtio: Fix failover_replug_primary() return value regression Markus Armbruster
` (24 more replies)
0 siblings, 25 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
This series addresses a number of error handling issues I found while
working on error handling improvements. It's based on my "[PULL
00/16] Qdev patches for 2020-06-23" merged into current master.
Based-on: <20200623142326.2349416-1-armbru@redhat.com>
v2:
* PATCH 04: Two more instances, in tests/check-qom-proplist
* PATCH 12: Dropped; PATCH 04 takes care of it
* PATCH 17: One more instance due to rebase; R-by kept
* PATCH 22-25: New
Markus Armbruster (25):
net/virtio: Fix failover_replug_primary() return value regression
pci: Delete useless error_propagate()
Clean up some calls to ignore Error objects the right way
tests: Use &error_abort where appropriate
tests: Use error_free_or_abort() where appropriate
usb/dev-mtp: Fix Error double free after inotify failure
spapr: Plug minor memory leak in spapr_machine_init()
qga: Plug unlikely memory leak in guest-set-memory-blocks
sd/milkymist-memcard: Plug minor memory leak in realize
test-util-filemonitor: Plug unlikely memory leak
vnc: Plug minor memory leak in vnc_display_open()
aspeed: Clean up roundabout error propagation
qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp
qdev: Drop qbus_set_hotplug_handler() parameter @errp
hw: Fix error API violation around object_property_set_link()
hw/arm: Drop useless object_property_set_link() error handling
riscv/sifive_u: Fix sifive_u_soc_realize() error API violations
riscv_hart: Fix riscv_harts_realize() error API violations
mips/cps: Fix mips_cps_realize() error API violations
x86: Fix x86_cpu_new() error API violations
amd_iommu: Fix amdvi_realize() error API violation
arm/stm32f205 arm/stm32f405: Fix realize error API violation
aspeed: Fix realize error API violation
hw/arm/armsse: Fix armsse_realize() error API violation
arm/{bcm2835,fsl-imx25,fsl-imx6}: Fix realize error API violations
include/hw/qdev-core.h | 5 +-
chardev/char-socket.c | 6 +-
hw/9pfs/9p.c | 6 +-
hw/acpi/pcihp.c | 3 +-
hw/acpi/piix4.c | 2 +-
hw/arm/armsse.c | 61 +++++-----------
hw/arm/armv7m.c | 7 +-
hw/arm/aspeed_ast2600.c | 35 ++++-----
hw/arm/aspeed_soc.c | 29 +++-----
hw/arm/bcm2835_peripherals.c | 12 ++--
hw/arm/fsl-imx25.c | 12 ++--
hw/arm/fsl-imx6.c | 12 ++--
hw/arm/nrf51_soc.c | 6 +-
hw/arm/stm32f205_soc.c | 2 +-
hw/arm/stm32f405_soc.c | 2 +-
hw/arm/virt.c | 4 +-
hw/char/virtio-serial-bus.c | 4 +-
hw/core/bus.c | 8 +--
hw/display/virtio-gpu-pci.c | 2 +-
hw/display/virtio-vga.c | 2 +-
hw/dma/sparc32_dma.c | 6 +-
hw/dma/xilinx_axidma.c | 12 +---
hw/i386/amd_iommu.c | 6 +-
hw/i386/x86.c | 12 +---
hw/mips/cps.c | 35 +++++----
hw/misc/macio/macio.c | 3 +-
hw/net/virtio-net.c | 2 +-
hw/net/xilinx_axienet.c | 12 +---
hw/pci/pci.c | 3 -
hw/pci/pcie.c | 2 +-
hw/pci/shpc.c | 2 +-
hw/ppc/spapr.c | 4 +-
hw/ppc/spapr_drc.c | 4 +-
hw/ppc/spapr_pci.c | 4 +-
hw/riscv/riscv_hart.c | 14 ++--
hw/riscv/sifive_u.c | 12 +++-
hw/s390x/ap-bridge.c | 2 +-
hw/s390x/css-bridge.c | 2 +-
hw/s390x/s390-pci-bus.c | 14 +---
hw/scsi/scsi-bus.c | 2 +-
hw/scsi/virtio-scsi.c | 4 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/sd/milkymist-memcard.c | 5 +-
hw/usb/bus.c | 2 +-
hw/usb/dev-mtp.c | 2 -
hw/usb/dev-smartcard-reader.c | 2 +-
hw/virtio/virtio-iommu-pci.c | 2 +-
hw/xen/xen-bus.c | 2 +-
hw/xen/xen-legacy-backend.c | 2 +-
qga/commands-posix.c | 1 +
tests/check-block-qdict.c | 24 ++-----
tests/check-qobject.c | 5 +-
tests/check-qom-proplist.c | 14 ++--
tests/test-base64.c | 3 +-
tests/test-bdrv-graph-mod.c | 4 +-
tests/test-block-iothread.c | 3 +-
tests/test-crypto-cipher.c | 8 +--
tests/test-io-task.c | 4 +-
tests/test-logging.c | 12 +---
tests/test-qemu-opts.c | 22 ++----
tests/test-replication.c | 109 +++++++++--------------------
tests/test-string-input-visitor.c | 33 +++------
tests/test-string-output-visitor.c | 16 ++---
tests/test-util-filemonitor.c | 1 +
ui/vnc.c | 6 +-
65 files changed, 229 insertions(+), 432 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 01/25] net/virtio: Fix failover_replug_primary() return value regression
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 02/25] pci: Delete useless error_propagate() Markus Armbruster
` (23 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Jens Freimann, qemu-stable, Michael S . Tsirkin
Commit 150ab54aa6 "net/virtio: fix re-plugging of primary device"
fixed failover_replug_primary() to return false on failure. Commit
5a0948d36c "net/virtio: Fix failover error handling crash bugs" broke
it again for hotplug_handler_plug() failure. Unbreak it.
Commit 5a0948d36c4cbc1c5534afac6fee99de55245d12
Fixes: 5a0948d36c4cbc1c5534afac6fee99de55245d12
Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
---
hw/net/virtio-net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index aff67a92df..9bb5578e5d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3129,7 +3129,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
if (err) {
goto out;
}
- hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
+ hotplug_handler_plug(hotplug_ctrl, n->primary_dev, &err);
}
out:
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 02/25] pci: Delete useless error_propagate()
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 01/25] net/virtio: Fix failover_replug_primary() return value regression Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way Markus Armbruster
` (22 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Jens Freimann, Michael S . Tsirkin
Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
---
hw/pci/pci.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b22dedc88c..de0fae10ab 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2123,7 +2123,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
error_setg(errp, "failover primary device must be on "
"PCIExpress bus");
- error_propagate(errp, local_err);
pci_qdev_unrealize(DEVICE(pci_dev));
return;
}
@@ -2131,7 +2130,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
if (class_id != PCI_CLASS_NETWORK_ETHERNET) {
error_setg(errp, "failover primary device is not an "
"Ethernet device");
- error_propagate(errp, local_err);
pci_qdev_unrealize(DEVICE(pci_dev));
return;
}
@@ -2141,7 +2139,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
} else {
error_setg(errp, "failover: primary device must be in its own "
"PCI slot");
- error_propagate(errp, local_err);
pci_qdev_unrealize(DEVICE(pci_dev));
return;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 01/25] net/virtio: Fix failover_replug_primary() return value regression Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 02/25] pci: Delete useless error_propagate() Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 9:02 ` Greg Kurz
2020-06-24 8:37 ` [PATCH v2 04/25] tests: Use &error_abort where appropriate Markus Armbruster
` (21 subsequent siblings)
24 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Jerome Forissier, Daniel P . Berrange, Greg Kurz
Receiving the error in a local variable only to free it is less clear
(and also less efficient) than passing NULL. Clean up.
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Jerome Forissier <jerome@forissier.org>
CC: Greg Kurz <groug@kaod.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
chardev/char-socket.c | 6 ++----
hw/9pfs/9p.c | 6 ++----
hw/arm/virt.c | 4 +---
hw/ppc/spapr_drc.c | 4 +---
ui/vnc.c | 3 +--
5 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index afebeec5c3..b0cae97960 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -815,22 +815,20 @@ static void tcp_chr_tls_init(Chardev *chr)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
QIOChannelTLS *tioc;
- Error *err = NULL;
gchar *name;
if (s->is_listen) {
tioc = qio_channel_tls_new_server(
s->ioc, s->tls_creds,
s->tls_authz,
- &err);
+ NULL);
} else {
tioc = qio_channel_tls_new_client(
s->ioc, s->tls_creds,
s->addr->u.inet.host,
- &err);
+ NULL);
}
if (tioc == NULL) {
- error_free(err);
tcp_chr_disconnect(chr);
return;
}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 45a788f6e6..9755fba9a9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1399,7 +1399,6 @@ static void coroutine_fn v9fs_attach(void *opaque)
size_t offset = 7;
V9fsQID qid;
ssize_t err;
- Error *local_err = NULL;
v9fs_string_init(&uname);
v9fs_string_init(&aname);
@@ -1437,9 +1436,8 @@ static void coroutine_fn v9fs_attach(void *opaque)
error_setg(&s->migration_blocker,
"Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
- err = migrate_add_blocker(s->migration_blocker, &local_err);
- if (local_err) {
- error_free(local_err);
+ err = migrate_add_blocker(s->migration_blocker, NULL);
+ if (err < 0) {
error_free(s->migration_blocker);
s->migration_blocker = NULL;
clunk_fid(s, fid);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index caceb1e4a0..29b9d5b2e6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -217,11 +217,9 @@ static bool cpu_type_valid(const char *cpu)
static void create_kaslr_seed(VirtMachineState *vms, const char *node)
{
- Error *err = NULL;
uint64_t seed;
- if (qemu_guest_getrandom(&seed, sizeof(seed), &err)) {
- error_free(err);
+ if (qemu_guest_getrandom(&seed, sizeof(seed), NULL)) {
return;
}
qemu_fdt_setprop_u64(vms->fdt, node, "kaslr-seed", seed);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 2689104295..951bcdf2c0 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -1163,16 +1163,14 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
if (!drc->fdt) {
- Error *local_err = NULL;
void *fdt;
int fdt_size;
fdt = create_device_tree(&fdt_size);
if (drck->dt_populate(drc, spapr, fdt, &drc->fdt_start_offset,
- &local_err)) {
+ NULL)) {
g_free(fdt);
- error_free(local_err);
rc = SPAPR_DR_CC_RESPONSE_ERROR;
goto out;
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 12a12714e1..0702a76cce 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -458,9 +458,8 @@ static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
Error *err = NULL;
SocketAddress *addr;
- addr = qio_channel_socket_get_local_address(ioc, &err);
+ addr = qio_channel_socket_get_local_address(ioc, NULL);
if (!addr) {
- error_free(err);
return prev;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 04/25] tests: Use &error_abort where appropriate
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (2 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 05/25] tests: Use error_free_or_abort() " Markus Armbruster
` (20 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Huth
Receiving the error in a local variable only to assert there is none
is less clear than passing &error_abort. Clean up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
tests/check-qobject.c | 5 +-
tests/check-qom-proplist.c | 7 +-
tests/test-logging.c | 12 +---
tests/test-qemu-opts.c | 22 ++----
tests/test-replication.c | 109 +++++++++--------------------
tests/test-string-input-visitor.c | 33 +++------
tests/test-string-output-visitor.c | 16 ++---
7 files changed, 59 insertions(+), 145 deletions(-)
diff --git a/tests/check-qobject.c b/tests/check-qobject.c
index 593c3a0618..6b6deaeb8b 100644
--- a/tests/check-qobject.c
+++ b/tests/check-qobject.c
@@ -9,6 +9,7 @@
#include "qemu/osdep.h"
#include "block/qdict.h"
+#include "qapi/error.h"
#include "qapi/qmp/qbool.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qlist.h"
@@ -213,7 +214,6 @@ static void qobject_is_equal_list_test(void)
static void qobject_is_equal_dict_test(void)
{
- Error *local_err = NULL;
QDict *dict_0, *dict_1, *dict_cloned;
QDict *dict_different_key, *dict_different_value, *dict_different_null_key;
QDict *dict_longer, *dict_shorter, *dict_nested;
@@ -276,8 +276,7 @@ static void qobject_is_equal_dict_test(void)
dict_different_null_key, dict_longer, dict_shorter,
dict_nested);
- dict_crumpled = qobject_to(QDict, qdict_crumple(dict_1, &local_err));
- g_assert(!local_err);
+ dict_crumpled = qobject_to(QDict, qdict_crumple(dict_1, &error_abort));
check_equal(dict_crumpled, dict_nested);
qdict_flatten(dict_nested);
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 13a824cfae..8c71734e1a 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -419,9 +419,7 @@ static void test_dummy_createcmdl(void)
g_assert(dobj->bv == true);
g_assert(dobj->av == DUMMY_PLATYPUS);
- user_creatable_del("dev0", &err);
- g_assert(err == NULL);
- error_free(err);
+ user_creatable_del("dev0", &error_abort);
object_unref(OBJECT(dobj));
@@ -485,8 +483,7 @@ static void test_dummy_getenum(void)
val = object_property_get_enum(OBJECT(dobj),
"av",
"DummyAnimal",
- &err);
- g_assert(err == NULL);
+ &error_abort);
g_assert(val == DUMMY_PLATYPUS);
/* A bad enum type name */
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8580b82420..8a1161de1d 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -113,7 +113,6 @@ static void test_logfile_write(gconstpointer data)
QemuLogFile *logfile;
QemuLogFile *logfile2;
gchar const *dir = data;
- Error *err = NULL;
g_autofree gchar *file_path = NULL;
g_autofree gchar *file_path1 = NULL;
FILE *orig_fd;
@@ -132,8 +131,7 @@ static void test_logfile_write(gconstpointer data)
* Test that even if an open file handle is changed,
* our handle remains valid due to RCU.
*/
- qemu_set_log_filename(file_path, &err);
- g_assert(!err);
+ qemu_set_log_filename(file_path, &error_abort);
rcu_read_lock();
logfile = atomic_rcu_read(&qemu_logfile);
orig_fd = logfile->fd;
@@ -142,8 +140,7 @@ static void test_logfile_write(gconstpointer data)
fflush(logfile->fd);
/* Change the logfile and ensure that the handle is still valid. */
- qemu_set_log_filename(file_path1, &err);
- g_assert(!err);
+ qemu_set_log_filename(file_path1, &error_abort);
logfile2 = atomic_rcu_read(&qemu_logfile);
g_assert(logfile->fd == orig_fd);
g_assert(logfile2->fd != logfile->fd);
@@ -156,7 +153,6 @@ static void test_logfile_lock(gconstpointer data)
{
FILE *logfile;
gchar const *dir = data;
- Error *err = NULL;
g_autofree gchar *file_path = NULL;
file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);
@@ -166,7 +162,7 @@ static void test_logfile_lock(gconstpointer data)
* that even if an open file handle is closed,
* our handle remains valid for use due to RCU.
*/
- qemu_set_log_filename(file_path, &err);
+ qemu_set_log_filename(file_path, &error_abort);
logfile = qemu_log_lock();
g_assert(logfile);
fprintf(logfile, "%s 1st write to file\n", __func__);
@@ -180,8 +176,6 @@ static void test_logfile_lock(gconstpointer data)
fprintf(logfile, "%s 2nd write to file\n", __func__);
fflush(logfile);
qemu_log_unlock(logfile);
-
- g_assert(!err);
}
/* Remove a directory and all its entries (non-recursive). */
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 2a0f42a09b..297ffe79dd 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -187,7 +187,6 @@ static void test_qemu_opt_get(void)
static void test_qemu_opt_get_bool(void)
{
- Error *err = NULL;
QemuOptsList *list;
QemuOpts *opts;
bool opt;
@@ -210,16 +209,14 @@ static void test_qemu_opt_get_bool(void)
opt = qemu_opt_get_bool(opts, "bool1", false);
g_assert(opt == false);
- qemu_opt_set_bool(opts, "bool1", true, &err);
- g_assert(!err);
+ qemu_opt_set_bool(opts, "bool1", true, &error_abort);
/* now we have set bool1, should know about it */
opt = qemu_opt_get_bool(opts, "bool1", false);
g_assert(opt == true);
/* having reset the value, opt should be the reset one not defval */
- qemu_opt_set_bool(opts, "bool1", false, &err);
- g_assert(!err);
+ qemu_opt_set_bool(opts, "bool1", false, &error_abort);
opt = qemu_opt_get_bool(opts, "bool1", true);
g_assert(opt == false);
@@ -233,7 +230,6 @@ static void test_qemu_opt_get_bool(void)
static void test_qemu_opt_get_number(void)
{
- Error *err = NULL;
QemuOptsList *list;
QemuOpts *opts;
uint64_t opt;
@@ -256,16 +252,14 @@ static void test_qemu_opt_get_number(void)
opt = qemu_opt_get_number(opts, "number1", 5);
g_assert(opt == 5);
- qemu_opt_set_number(opts, "number1", 10, &err);
- g_assert(!err);
+ qemu_opt_set_number(opts, "number1", 10, &error_abort);
/* now we have set number1, should know about it */
opt = qemu_opt_get_number(opts, "number1", 5);
g_assert(opt == 10);
/* having reset it, the returned should be the reset one not defval */
- qemu_opt_set_number(opts, "number1", 15, &err);
- g_assert(!err);
+ qemu_opt_set_number(opts, "number1", 15, &error_abort);
opt = qemu_opt_get_number(opts, "number1", 5);
g_assert(opt == 15);
@@ -367,7 +361,6 @@ static void test_qemu_opt_unset(void)
static void test_qemu_opts_reset(void)
{
- Error *err = NULL;
QemuOptsList *list;
QemuOpts *opts;
uint64_t opt;
@@ -390,8 +383,7 @@ static void test_qemu_opts_reset(void)
opt = qemu_opt_get_number(opts, "number1", 5);
g_assert(opt == 5);
- qemu_opt_set_number(opts, "number1", 10, &err);
- g_assert(!err);
+ qemu_opt_set_number(opts, "number1", 10, &error_abort);
/* now we have set number1, should know about it */
opt = qemu_opt_get_number(opts, "number1", 5);
@@ -406,7 +398,6 @@ static void test_qemu_opts_reset(void)
static void test_qemu_opts_set(void)
{
- Error *err = NULL;
QemuOptsList *list;
QemuOpts *opts;
const char *opt;
@@ -421,8 +412,7 @@ static void test_qemu_opts_set(void)
g_assert(opts == NULL);
/* implicitly create opts and set str3 value */
- qemu_opts_set(list, NULL, "str3", "value", &err);
- g_assert(!err);
+ qemu_opts_set(list, NULL, "str3", "value", &error_abort);
g_assert(!QTAILQ_EMPTY(&list->head));
/* get the just created opts */
diff --git a/tests/test-replication.c b/tests/test-replication.c
index cbc37db2df..e0b03dafc2 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -139,8 +139,6 @@ static void make_temp(char *template)
static void prepare_imgs(void)
{
- Error *local_err = NULL;
-
make_temp(p_local_disk);
make_temp(s_local_disk);
make_temp(s_active_disk);
@@ -148,19 +146,15 @@ static void prepare_imgs(void)
/* Primary */
bdrv_img_create(p_local_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
- BDRV_O_RDWR, true, &local_err);
- g_assert(!local_err);
+ BDRV_O_RDWR, true, &error_abort);
/* Secondary */
bdrv_img_create(s_local_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
- BDRV_O_RDWR, true, &local_err);
- g_assert(!local_err);
+ BDRV_O_RDWR, true, &error_abort);
bdrv_img_create(s_active_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
- BDRV_O_RDWR, true, &local_err);
- g_assert(!local_err);
+ BDRV_O_RDWR, true, &error_abort);
bdrv_img_create(s_hidden_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
- BDRV_O_RDWR, true, &local_err);
- g_assert(!local_err);
+ BDRV_O_RDWR, true, &error_abort);
}
static void cleanup_imgs(void)
@@ -179,7 +173,6 @@ static BlockBackend *start_primary(void)
BlockBackend *blk;
QemuOpts *opts;
QDict *qdict;
- Error *local_err = NULL;
char *cmdline;
cmdline = g_strdup_printf("driver=replication,mode=primary,node-name=xxx,"
@@ -193,12 +186,10 @@ static BlockBackend *start_primary(void)
qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
- blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+ blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
g_assert(blk);
- g_assert(!local_err);
- monitor_add_blk(blk, P_ID, &local_err);
- g_assert(!local_err);
+ monitor_add_blk(blk, P_ID, &error_abort);
qemu_opts_del(opts);
@@ -248,12 +239,10 @@ static void test_primary_write(void)
static void test_primary_start(void)
{
BlockBackend *blk = NULL;
- Error *local_err = NULL;
blk = start_primary();
- replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
/* read from 0 to IMG_SIZE */
test_blk_read(blk, 0, 0, IMG_SIZE, 0, IMG_SIZE, true);
@@ -266,46 +255,35 @@ static void test_primary_start(void)
static void test_primary_stop(void)
{
- Error *local_err = NULL;
bool failover = true;
start_primary();
- replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
- replication_stop_all(failover, &local_err);
- g_assert(!local_err);
+ replication_stop_all(failover, &error_abort);
teardown_primary();
}
static void test_primary_do_checkpoint(void)
{
- Error *local_err = NULL;
-
start_primary();
- replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
- replication_do_checkpoint_all(&local_err);
- g_assert(!local_err);
+ replication_do_checkpoint_all(&error_abort);
teardown_primary();
}
static void test_primary_get_error_all(void)
{
- Error *local_err = NULL;
-
start_primary();
- replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
- replication_get_error_all(&local_err);
- g_assert(!local_err);
+ replication_get_error_all(&error_abort);
teardown_primary();
}
@@ -316,7 +294,6 @@ static BlockBackend *start_secondary(void)
QDict *qdict;
BlockBackend *blk;
char *cmdline;
- Error *local_err = NULL;
/* add s_local_disk and forge S_LOCAL_DISK_ID */
cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
@@ -329,10 +306,9 @@ static BlockBackend *start_secondary(void)
qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
- blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+ blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
assert(blk);
- monitor_add_blk(blk, S_LOCAL_DISK_ID, &local_err);
- g_assert(!local_err);
+ monitor_add_blk(blk, S_LOCAL_DISK_ID, &error_abort);
/* format s_local_disk with pattern "0x11" */
test_blk_write(blk, 0x11, 0, IMG_SIZE, false);
@@ -356,10 +332,9 @@ static BlockBackend *start_secondary(void)
qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
- blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+ blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
assert(blk);
- monitor_add_blk(blk, S_ID, &local_err);
- g_assert(!local_err);
+ monitor_add_blk(blk, S_ID, &error_abort);
qemu_opts_del(opts);
@@ -420,12 +395,10 @@ static void test_secondary_write(void)
static void test_secondary_start(void)
{
BlockBackend *top_blk, *local_blk;
- Error *local_err = NULL;
bool failover = true;
top_blk = start_secondary();
- replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
/* read from s_local_disk (0, IMG_SIZE) */
test_blk_read(top_blk, 0x11, 0, IMG_SIZE, 0, IMG_SIZE, false);
@@ -446,8 +419,7 @@ static void test_secondary_start(void)
0, IMG_SIZE / 2, false);
/* unblock top_bs */
- replication_stop_all(failover, &local_err);
- g_assert(!local_err);
+ replication_stop_all(failover, &error_abort);
teardown_secondary();
}
@@ -456,12 +428,10 @@ static void test_secondary_start(void)
static void test_secondary_stop(void)
{
BlockBackend *top_blk, *local_blk;
- Error *local_err = NULL;
bool failover = true;
top_blk = start_secondary();
- replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
/* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -475,8 +445,7 @@ static void test_secondary_stop(void)
test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
/* do active commit */
- replication_stop_all(failover, &local_err);
- g_assert(!local_err);
+ replication_stop_all(failover, &error_abort);
/* read from s_local_disk (0, IMG_SIZE / 2) */
test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
@@ -493,11 +462,9 @@ static void test_secondary_stop(void)
static void test_secondary_continuous_replication(void)
{
BlockBackend *top_blk, *local_blk;
- Error *local_err = NULL;
top_blk = start_secondary();
- replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
/* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -511,22 +478,18 @@ static void test_secondary_continuous_replication(void)
test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
/* do failover (active commit) */
- replication_stop_all(true, &local_err);
- g_assert(!local_err);
+ replication_stop_all(true, &error_abort);
/* it should ignore all requests from now on */
/* start after failover */
- replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
/* checkpoint */
- replication_do_checkpoint_all(&local_err);
- g_assert(!local_err);
+ replication_do_checkpoint_all(&error_abort);
/* stop */
- replication_stop_all(true, &local_err);
- g_assert(!local_err);
+ replication_stop_all(true, &error_abort);
/* read from s_local_disk (0, IMG_SIZE / 2) */
test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
@@ -543,12 +506,10 @@ static void test_secondary_continuous_replication(void)
static void test_secondary_do_checkpoint(void)
{
BlockBackend *top_blk, *local_blk;
- Error *local_err = NULL;
bool failover = true;
top_blk = start_secondary();
- replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
/* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -559,35 +520,29 @@ static void test_secondary_do_checkpoint(void)
test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
IMG_SIZE / 2, 0, IMG_SIZE, false);
- replication_do_checkpoint_all(&local_err);
- g_assert(!local_err);
+ replication_do_checkpoint_all(&error_abort);
/* after checkpoint, read pattern 0x22 from s_local_disk */
test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
IMG_SIZE / 2, 0, IMG_SIZE, false);
/* unblock top_bs */
- replication_stop_all(failover, &local_err);
- g_assert(!local_err);
+ replication_stop_all(failover, &error_abort);
teardown_secondary();
}
static void test_secondary_get_error_all(void)
{
- Error *local_err = NULL;
bool failover = true;
start_secondary();
- replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
- g_assert(!local_err);
+ replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
- replication_get_error_all(&local_err);
- g_assert(!local_err);
+ replication_get_error_all(&error_abort);
/* unblock top_bs */
- replication_stop_all(failover, &local_err);
- g_assert(!local_err);
+ replication_stop_all(failover, &error_abort);
teardown_secondary();
}
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 5418e085a4..249faafc9d 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -53,8 +53,7 @@ static void test_visitor_in_int(TestInputVisitorData *data,
v = visitor_input_test_init(data, "-42");
- visit_type_int(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_int(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, value);
v = visitor_input_test_init(data, "not an int");
@@ -327,44 +326,37 @@ static void test_visitor_in_uintList(TestInputVisitorData *data,
static void test_visitor_in_bool(TestInputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
bool res = false;
Visitor *v;
v = visitor_input_test_init(data, "true");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, true);
v = visitor_input_test_init(data, "yes");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, true);
v = visitor_input_test_init(data, "on");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, true);
v = visitor_input_test_init(data, "false");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, false);
v = visitor_input_test_init(data, "no");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, false);
v = visitor_input_test_init(data, "off");
- visit_type_bool(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_bool(v, NULL, &res, &error_abort);
g_assert_cmpint(res, ==, false);
}
@@ -377,8 +369,7 @@ static void test_visitor_in_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "3.14");
- visit_type_number(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_number(v, NULL, &res, &error_abort);
g_assert_cmpfloat(res, ==, value);
/* NaN and infinity has to be rejected */
@@ -399,13 +390,11 @@ static void test_visitor_in_string(TestInputVisitorData *data,
const void *unused)
{
char *res = NULL, *value = (char *) "Q E M U";
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, value);
- visit_type_str(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_str(v, NULL, &res, &error_abort);
g_assert_cmpstr(res, ==, value);
g_free(res);
@@ -414,7 +403,6 @@ static void test_visitor_in_string(TestInputVisitorData *data,
static void test_visitor_in_enum(TestInputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
Visitor *v;
EnumOne i;
@@ -423,8 +411,7 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
v = visitor_input_test_init(data, EnumOne_str(i));
- visit_type_EnumOne(v, NULL, &res, &err);
- g_assert(!err);
+ visit_type_EnumOne(v, NULL, &res, &error_abort);
g_assert_cmpint(i, ==, res);
}
}
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 3bd732222c..9f6581439a 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -71,11 +71,9 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
const void *unused)
{
int64_t value = 42;
- Error *err = NULL;
char *str;
- visit_type_int(data->ov, NULL, &value, &err);
- g_assert(!err);
+ visit_type_int(data->ov, NULL, &value, &error_abort);
str = visitor_get(data);
if (data->human) {
@@ -120,12 +118,10 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
static void test_visitor_out_bool(TestOutputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
bool value = true;
char *str;
- visit_type_bool(data->ov, NULL, &value, &err);
- g_assert(!err);
+ visit_type_bool(data->ov, NULL, &value, &error_abort);
str = visitor_get(data);
g_assert_cmpstr(str, ==, "true");
@@ -135,11 +131,9 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
const void *unused)
{
double value = 3.14;
- Error *err = NULL;
char *str;
- visit_type_number(data->ov, NULL, &value, &err);
- g_assert(!err);
+ visit_type_number(data->ov, NULL, &value, &error_abort);
str = visitor_get(data);
g_assert_cmpstr(str, ==, "3.140000");
@@ -150,11 +144,9 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
{
char *string = (char *) "Q E M U";
const char *string_human = "\"Q E M U\"";
- Error *err = NULL;
char *str;
- visit_type_str(data->ov, NULL, &string, &err);
- g_assert(!err);
+ visit_type_str(data->ov, NULL, &string, &error_abort);
str = visitor_get(data);
if (data->human) {
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 05/25] tests: Use error_free_or_abort() where appropriate
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (3 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 04/25] tests: Use &error_abort where appropriate Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 14:42 ` Eric Blake
2020-06-24 8:37 ` [PATCH v2 06/25] usb/dev-mtp: Fix Error double free after inotify failure Markus Armbruster
` (19 subsequent siblings)
24 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Replace
g_assert(err != NULL);
error_free(err);
err = NULL;
and variations thereof by
error_free_or_abort(&err);
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/check-block-qdict.c | 24 ++++++------------------
tests/check-qom-proplist.c | 7 ++-----
tests/test-base64.c | 3 +--
tests/test-bdrv-graph-mod.c | 4 +---
tests/test-block-iothread.c | 3 +--
tests/test-crypto-cipher.c | 8 ++------
tests/test-io-task.c | 4 +---
7 files changed, 14 insertions(+), 39 deletions(-)
diff --git a/tests/check-block-qdict.c b/tests/check-block-qdict.c
index 73d3e9f574..5a25825093 100644
--- a/tests/check-block-qdict.c
+++ b/tests/check-block-qdict.c
@@ -610,9 +610,7 @@ static void qdict_rename_keys_test(void)
copy = qdict_clone_shallow(dict);
qdict_rename_keys(copy, renames, &local_err);
- g_assert(local_err != NULL);
- error_free(local_err);
- local_err = NULL;
+ error_free_or_abort(&local_err);
g_assert_cmpstr(qdict_get_str(copy, "abc"), ==, "foo");
g_assert_cmpstr(qdict_get_str(copy, "abcdef"), ==, "bar");
@@ -649,9 +647,7 @@ static void qdict_crumple_test_bad_inputs(void)
qdict_put_str(src, "rule.0.policy", "allow");
g_assert(qdict_crumple(src, &error) == NULL);
- g_assert(error != NULL);
- error_free(error);
- error = NULL;
+ error_free_or_abort(&error);
qobject_unref(src);
src = qdict_new();
@@ -660,9 +656,7 @@ static void qdict_crumple_test_bad_inputs(void)
qdict_put_str(src, "rule.a", "allow");
g_assert(qdict_crumple(src, &error) == NULL);
- g_assert(error != NULL);
- error_free(error);
- error = NULL;
+ error_free_or_abort(&error);
qobject_unref(src);
src = qdict_new();
@@ -673,9 +667,7 @@ static void qdict_crumple_test_bad_inputs(void)
qdict_put_str(src, "rule.b", "allow");
g_assert(qdict_crumple(src, &error) == NULL);
- g_assert(error != NULL);
- error_free(error);
- error = NULL;
+ error_free_or_abort(&error);
qobject_unref(src);
src = qdict_new();
@@ -684,9 +676,7 @@ static void qdict_crumple_test_bad_inputs(void)
qdict_put_str(src, "rule.3", "allow");
g_assert(qdict_crumple(src, &error) == NULL);
- g_assert(error != NULL);
- error_free(error);
- error = NULL;
+ error_free_or_abort(&error);
qobject_unref(src);
src = qdict_new();
@@ -695,9 +685,7 @@ static void qdict_crumple_test_bad_inputs(void)
qdict_put_str(src, "rule.+1", "allow");
g_assert(qdict_crumple(src, &error) == NULL);
- g_assert(error != NULL);
- error_free(error);
- error = NULL;
+ error_free_or_abort(&error);
qobject_unref(src);
}
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 8c71734e1a..e1e0a96661 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -491,17 +491,14 @@ static void test_dummy_getenum(void)
"av",
"BadAnimal",
&err);
- g_assert(err != NULL);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
/* A non-enum property name */
val = object_property_get_enum(OBJECT(dobj),
"iv",
"DummyAnimal",
&err);
- g_assert(err != NULL);
- error_free(err);
+ error_free_or_abort(&err);
object_unparent(OBJECT(dobj));
}
diff --git a/tests/test-base64.c b/tests/test-base64.c
index ec122ceba5..a7f722c459 100644
--- a/tests/test-base64.c
+++ b/tests/test-base64.c
@@ -54,10 +54,9 @@ static void test_base64_bad(const char *input,
&len,
&err);
- g_assert(err != NULL);
+ error_free_or_abort(&err);
g_assert(actual == NULL);
g_assert_cmpint(len, ==, 0);
- error_free(err);
}
diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
index f93f3168b0..8cff13830e 100644
--- a/tests/test-bdrv-graph-mod.c
+++ b/tests/test-bdrv-graph-mod.c
@@ -115,9 +115,7 @@ static void test_update_perm_tree(void)
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort);
bdrv_append(filter, bs, &local_err);
-
- g_assert_nonnull(local_err);
- error_free(local_err);
+ error_free_or_abort(&local_err);
blk_unref(root);
}
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index a953794be2..3f866a35c6 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -650,8 +650,7 @@ static void test_propagate_mirror(void)
blk_insert_bs(blk, src, &error_abort);
bdrv_try_set_aio_context(target, ctx, &local_err);
- g_assert(local_err);
- error_free(local_err);
+ error_free_or_abort(&local_err);
g_assert(blk_get_aio_context(blk) == main_ctx);
g_assert(bdrv_get_aio_context(src) == main_ctx);
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 07fa2fa616..bebba1a4f4 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -761,10 +761,7 @@ static void test_cipher_short_plaintext(void)
sizeof(plaintext1),
&err);
g_assert(ret == -1);
- g_assert(err != NULL);
-
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
/* Should report an error as plaintext is larger than
* block size, but not a multiple of block size
@@ -775,9 +772,8 @@ static void test_cipher_short_plaintext(void)
sizeof(plaintext2),
&err);
g_assert(ret == -1);
- g_assert(err != NULL);
+ error_free_or_abort(&err);
- error_free(err);
qcrypto_cipher_free(cipher);
}
diff --git a/tests/test-io-task.c b/tests/test-io-task.c
index c8a3813d49..85e7a98da5 100644
--- a/tests/test-io-task.c
+++ b/tests/test-io-task.c
@@ -240,9 +240,7 @@ static void test_task_thread_failure(void)
object_unref(obj);
g_assert(data.source == obj);
- g_assert(data.err != NULL);
-
- error_free(data.err);
+ error_free_or_abort(&data.err);
self = g_thread_self();
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 06/25] usb/dev-mtp: Fix Error double free after inotify failure
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (4 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 05/25] tests: Use error_free_or_abort() " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
` (18 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrangé, Gerd Hoffmann, qemu-stable
error_report_err() frees its first argument. Freeing it again is
wrong. Don't.
Fixes: 47287c27d0c367a89f7b2851e23a7f8b2d499dd6
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/usb/dev-mtp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 168428156b..15a2243101 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -634,7 +634,6 @@ static void usb_mtp_object_readdir(MTPState *s, MTPObject *o)
error_reportf_err(err,
"usb-mtp: failed to add watch for %s: ",
o->path);
- error_free(err);
} else {
trace_usb_mtp_file_monitor_event(s->dev.addr, o->path,
"Watch Added");
@@ -1279,7 +1278,6 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
if (err) {
error_reportf_err(err,
"usb-mtp: file monitoring init failed: ");
- error_free(err);
} else {
QTAILQ_INIT(&s->events);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init()
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (5 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 06/25] usb/dev-mtp: Fix Error double free after inotify failure Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 9:04 ` Greg Kurz
2020-06-25 3:52 ` David Gibson
2020-06-24 8:37 ` [PATCH v2 08/25] qga: Plug unlikely memory leak in guest-set-memory-blocks Markus Armbruster
` (17 subsequent siblings)
24 siblings, 2 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: David Gibson, qemu-ppc
spapr_machine_init() leaks an Error object when
kvmppc_check_papr_resize_hpt() fails and spapr->resize_hpt is
SPAPR_RESIZE_HPT_DISABLED, i.e. when the host doesn't support hash
page table resizing, and the user didn't ask for it. As harmless as
memory leaks can possibly be. Plug it.
Fixes: 30f4b05bd090564181554d0890605eb2c143e4ea
Cc: David Gibson <dgibson@redhat.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ppc/spapr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bd9345cdac..9bd2183ead 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2731,6 +2731,7 @@ static void spapr_machine_init(MachineState *machine)
error_report_err(resize_hpt_err);
exit(1);
}
+ error_free(resize_hpt_err);
spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 08/25] qga: Plug unlikely memory leak in guest-set-memory-blocks
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (6 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 09/25] sd/milkymist-memcard: Plug minor memory leak in realize Markus Armbruster
` (16 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Hailiang Zhang
transfer_memory_block() leaks an Error object when reading file
/sys/devices/system/memory/memory<INDEX>/state fails with errno other
than ENOENT, and @sys2memblk is false, i.e. when the state file exists
but cannot be read (seems quite unlikely), and this is
guest-set-memory-blocks, not guest-get-memory-blocks.
Plug the leak.
Fixes: bd240fca42d5f072fb758a71720d9de9990ac553
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
qga/commands-posix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ae1348dc8f..cdbeb59dcc 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2421,6 +2421,7 @@ static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk,
if (sys2memblk) {
error_propagate(errp, local_err);
} else {
+ error_free(local_err);
result->response =
GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 09/25] sd/milkymist-memcard: Plug minor memory leak in realize
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (7 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 08/25] qga: Plug unlikely memory leak in guest-set-memory-blocks Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 10/25] test-util-filemonitor: Plug unlikely memory leak Markus Armbruster
` (15 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Philippe Mathieu-Daudé
milkymist_memcard_realize() leaks an Error object when realization of
its "sd-card" device fails. Quite harmless, since we only ever
realize this once, in milkymist_init() via milkymist_memcard_create().
Plug the leak.
Fixes: 3d0369ba499866cc6a839f71212d97876500762d
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/sd/milkymist-memcard.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 482e97191e..afdb8aa0c0 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -280,9 +280,8 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
carddev = qdev_new(TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk);
- qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err);
- if (err) {
- error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
+ if (!qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err)) {
+ error_propagate_prepend(errp, err, "failed to init SD card: %s");
return;
}
s->enabled = blk && blk_is_inserted(blk);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 10/25] test-util-filemonitor: Plug unlikely memory leak
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (8 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 09/25] sd/milkymist-memcard: Plug minor memory leak in realize Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 11/25] vnc: Plug minor memory leak in vnc_display_open() Markus Armbruster
` (14 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrangé
test_file_monitor_events() leaks an Error object when
qemu_file_monitor_add_watch() fails, which seems unlikely. Plug it.
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-util-filemonitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 45009c69f4..8f0eff3d03 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -495,6 +495,7 @@ test_file_monitor_events(void)
if (*op->watchid < 0) {
g_printerr("Unable to add watch %s",
error_get_pretty(local_err));
+ error_free(local_err);
goto cleanup;
}
if (debug) {
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 11/25] vnc: Plug minor memory leak in vnc_display_open()
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (9 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 10/25] test-util-filemonitor: Plug unlikely memory leak Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 12/25] aspeed: Clean up roundabout error propagation Markus Armbruster
` (13 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrange, Gerd Hoffmann
vnc_display_print_local_addr() leaks the Error object when
qio_channel_socket_get_local_address() fails. Seems unlikely. Called
when we create a VNC display with vnc_display_open(). Plug the leak
by passing NULL to ignore the error.
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
ui/vnc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 0702a76cce..527ad25124 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3274,13 +3274,12 @@ int vnc_display_pw_expire(const char *id, time_t expires)
static void vnc_display_print_local_addr(VncDisplay *vd)
{
SocketAddress *addr;
- Error *err = NULL;
if (!vd->listener || !vd->listener->nsioc) {
return;
}
- addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], &err);
+ addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], NULL);
if (!addr) {
return;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 12/25] aspeed: Clean up roundabout error propagation
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (10 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 11/25] vnc: Plug minor memory leak in vnc_display_open() Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 13/25] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp Markus Armbruster
` (12 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Cédric Le Goater
Replace
sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
error_propagate(&err, local_err);
if (err) {
error_propagate(errp, err);
return;
}
by
sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
error_propagate(errp, err);
return;
}
Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/aspeed_ast2600.c | 10 ++++------
hw/arm/aspeed_soc.c | 10 ++++------
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 6da687299f..08b3592e36 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -228,7 +228,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
int i;
AspeedSoCState *s = ASPEED_SOC(dev);
AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
- Error *err = NULL, *local_err = NULL;
+ Error *err = NULL;
qemu_irq irq;
/* IO space */
@@ -394,8 +394,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
return;
}
object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
- sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
- error_propagate(&err, local_err);
+ sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
error_propagate(errp, err);
return;
@@ -446,11 +445,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
&err);
- sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &local_err);
- error_propagate(&err, local_err);
+ sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
if (err) {
error_propagate(errp, err);
- return;
+ return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
sc->memmap[ASPEED_ETH1 + i]);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 810cf9b6cc..ec21de50ce 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -218,7 +218,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
int i;
AspeedSoCState *s = ASPEED_SOC(dev);
AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
- Error *err = NULL, *local_err = NULL;
+ Error *err = NULL;
/* IO space */
create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_IOMEM],
@@ -340,8 +340,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
/* SPI */
for (i = 0; i < sc->spis_num; i++) {
object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
- sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
- error_propagate(&err, local_err);
+ sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
error_propagate(errp, err);
return;
@@ -392,11 +391,10 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
&err);
- sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &local_err);
- error_propagate(&err, local_err);
+ sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
if (err) {
error_propagate(errp, err);
- return;
+ return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
sc->memmap[ASPEED_ETH1 + i]);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 13/25] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (11 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 12/25] aspeed: Clean up roundabout error propagation Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 14/25] qdev: Drop qbus_set_hotplug_handler() " Markus Armbruster
` (11 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost
All callers pass &error_abort. Drop the parameter.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/qdev-core.h | 2 +-
hw/core/bus.c | 4 ++--
hw/scsi/scsi-bus.c | 2 +-
hw/usb/bus.c | 2 +-
hw/xen/xen-bus.c | 2 +-
hw/xen/xen-legacy-backend.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7dc10be46f..78acdeaed6 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -537,7 +537,7 @@ char *qdev_get_dev_path(DeviceState *dev);
void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
+void qbus_set_bus_hotplug_handler(BusState *bus);
static inline bool qbus_is_hotpluggable(BusState *bus)
{
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 6cc28b334e..8d4e810d7f 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -29,9 +29,9 @@ void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
}
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+void qbus_set_bus_hotplug_handler(BusState *bus)
{
- qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
+ qbus_set_hotplug_handler(bus, OBJECT(bus), &error_abort);
}
int qbus_walk_children(BusState *bus,
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 27843bb04b..b878a08080 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -107,7 +107,7 @@ void scsi_bus_new(SCSIBus *bus, size_t bus_size, DeviceState *host,
qbus_create_inplace(bus, bus_size, TYPE_SCSI_BUS, host, bus_name);
bus->busnr = next_scsi_bus++;
bus->info = info;
- qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
+ qbus_set_bus_hotplug_handler(BUS(bus));
}
static void scsi_dma_restart_bh(void *opaque)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index a81aee2051..957559b18d 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -84,7 +84,7 @@ void usb_bus_new(USBBus *bus, size_t bus_size,
USBBusOps *ops, DeviceState *host)
{
qbus_create_inplace(bus, bus_size, TYPE_USB_BUS, host, NULL);
- qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
+ qbus_set_bus_hotplug_handler(BUS(bus));
bus->ops = ops;
bus->busnr = next_usb_bus++;
QTAILQ_INIT(&bus->free);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4b00320f1c..c4e2162ae9 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1391,5 +1391,5 @@ void xen_bus_init(void)
BusState *bus = qbus_create(TYPE_XEN_BUS, dev, NULL);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- qbus_set_bus_hotplug_handler(bus, &error_abort);
+ qbus_set_bus_hotplug_handler(bus);
}
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 2335ee2e65..7d4b13351e 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -705,7 +705,7 @@ int xen_be_init(void)
xen_sysdev = qdev_new(TYPE_XENSYSDEV);
sysbus_realize_and_unref(SYS_BUS_DEVICE(xen_sysdev), &error_fatal);
xen_sysbus = qbus_create(TYPE_XENSYSBUS, xen_sysdev, "xen-sysbus");
- qbus_set_bus_hotplug_handler(xen_sysbus, &error_abort);
+ qbus_set_bus_hotplug_handler(xen_sysbus);
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 14/25] qdev: Drop qbus_set_hotplug_handler() parameter @errp
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (12 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 13/25] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 15/25] hw: Fix error API violation around object_property_set_link() Markus Armbruster
` (10 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost
qbus_set_hotplug_handler() is a simple wrapper around
object_property_set_link().
object_property_set_link() fails when the property doesn't exist, is
not settable, or its .check() method fails. These are all programming
errors here, so passing &error_abort to qbus_set_hotplug_handler() is
appropriate.
Most of its callers do. Exceptions:
* pcie_cap_slot_init(), shpc_init(), spapr_phb_realize() pass NULL,
i.e. they ignore errors.
* spapr_machine_init() passes &error_fatal.
* s390_pcihost_realize(), virtio_serial_device_realize(),
s390_pcihost_plug() pass the error to their callers. The latter two
keep going after the error, which looks wrong.
Drop the @errp parameter, and instead pass &error_abort to
object_property_set_link().
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/qdev-core.h | 3 +--
hw/acpi/pcihp.c | 3 +--
hw/acpi/piix4.c | 2 +-
hw/char/virtio-serial-bus.c | 4 ++--
hw/core/bus.c | 6 +++---
hw/pci/pcie.c | 2 +-
hw/pci/shpc.c | 2 +-
hw/ppc/spapr.c | 3 +--
hw/ppc/spapr_pci.c | 4 ++--
hw/s390x/ap-bridge.c | 2 +-
hw/s390x/css-bridge.c | 2 +-
hw/s390x/s390-pci-bus.c | 14 +++-----------
hw/scsi/virtio-scsi.c | 4 ++--
hw/scsi/vmw_pvscsi.c | 2 +-
hw/usb/dev-smartcard-reader.c | 2 +-
15 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 78acdeaed6..fe78073c70 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -535,8 +535,7 @@ extern bool qdev_hot_removed;
char *qdev_get_dev_path(DeviceState *dev);
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
-
+void qbus_set_hotplug_handler(BusState *bus, Object *handler);
void qbus_set_bus_hotplug_handler(BusState *bus);
static inline bool qbus_is_hotpluggable(BusState *bus)
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 33ea2b76ae..9e31ab2da4 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -246,8 +246,7 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
- qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev),
- &error_abort);
+ qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev));
/* We don't have to overwrite any other hotplug handler yet */
assert(QLIST_EMPTY(&sec->child));
}
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 1262abc77a..52bcbd6414 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -505,7 +505,7 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
pci_get_bus(dev), s);
- qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
+ qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s));
piix4_pm_add_propeties(s);
}
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 262089c0c9..f9a4428bd6 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1056,7 +1056,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
/* Spawn a new virtio-serial bus on which the ports will ride as devices */
qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
dev, vdev->bus_name);
- qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser), errp);
+ qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser));
vser->bus.vser = vser;
QTAILQ_INIT(&vser->ports);
@@ -1147,7 +1147,7 @@ static void virtio_serial_device_unrealize(DeviceState *dev)
g_free(vser->post_load);
}
- qbus_set_hotplug_handler(BUS(&vser->bus), NULL, &error_abort);
+ qbus_set_hotplug_handler(BUS(&vser->bus), NULL);
virtio_cleanup(vdev);
}
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 8d4e810d7f..544dd8a6fa 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -23,15 +23,15 @@
#include "qemu/module.h"
#include "qapi/error.h"
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
+void qbus_set_hotplug_handler(BusState *bus, Object *handler)
{
object_property_set_link(OBJECT(bus), handler,
- QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+ QDEV_HOTPLUG_HANDLER_PROPERTY, &error_abort);
}
void qbus_set_bus_hotplug_handler(BusState *bus)
{
- qbus_set_hotplug_handler(bus, OBJECT(bus), &error_abort);
+ qbus_set_hotplug_handler(bus, OBJECT(bus));
}
int qbus_walk_children(BusState *bus,
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 086d0dfceb..5b48bae0f6 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -574,7 +574,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
dev->exp.hpev_notified = false;
qbus_set_hotplug_handler(BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev))),
- OBJECT(dev), NULL);
+ OBJECT(dev));
}
void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 99d65d5c4c..b00dce629c 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -649,7 +649,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar,
shpc_cap_update_dword(d);
memory_region_add_subregion(bar, offset, &shpc->mmio);
- qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d), NULL);
+ qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d));
d->cap_present |= QEMU_PCI_CAP_SHPC;
return 0;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9bd2183ead..83b974870c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3033,8 +3033,7 @@ static void spapr_machine_init(MachineState *machine)
register_savevm_live("spapr/htab", VMSTATE_INSTANCE_ID_ANY, 1,
&savevm_htab_handlers, spapr);
- qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine),
- &error_fatal);
+ qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine));
qemu_register_boot_set(spapr_boot_set, spapr);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 329002ac04..0f00e2421f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1719,7 +1719,7 @@ static void spapr_phb_unrealize(DeviceState *dev)
address_space_remove_listeners(&sphb->iommu_as);
address_space_destroy(&sphb->iommu_as);
- qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+ qbus_set_hotplug_handler(BUS(phb->bus), NULL);
pci_unregister_root_bus(phb->bus);
memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
@@ -1868,7 +1868,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
}
phb->bus = bus;
- qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL);
+ qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb));
/*
* Initialize PHB address space.
diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c
index c4e3188ad6..8bcf8ece9d 100644
--- a/hw/s390x/ap-bridge.c
+++ b/hw/s390x/ap-bridge.c
@@ -58,7 +58,7 @@ void s390_init_ap(void)
bus = qbus_create(TYPE_AP_BUS, dev, TYPE_AP_BUS);
/* Enable hotplugging */
- qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
+ qbus_set_hotplug_handler(bus, OBJECT(dev));
}
static void ap_bridge_class_init(ObjectClass *oc, void *data)
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index e37a54d3f2..9d793d671e 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -111,7 +111,7 @@ VirtualCssBus *virtual_css_bus_init(void)
cbus = VIRTUAL_CSS_BUS(bus);
/* Enable hotplugging */
- qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
+ qbus_set_hotplug_handler(bus, OBJECT(dev));
css_register_io_adapters(CSS_IO_ADAPTER_VIRTIO, true, false,
0, &error_abort);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index a13978bb37..142e52a8ff 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -751,19 +751,11 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
pci_setup_iommu(b, s390_pci_dma_iommu, s);
bus = BUS(b);
- qbus_set_hotplug_handler(bus, OBJECT(dev), &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
+ qbus_set_hotplug_handler(bus, OBJECT(dev));
phb->bus = b;
s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, dev, NULL));
- qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev), &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
+ qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev));
s->iommu_table = g_hash_table_new_full(g_int64_hash, g_int64_equal,
NULL, g_free);
@@ -921,7 +913,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
- qbus_set_hotplug_handler(BUS(&pb->sec_bus), OBJECT(s), errp);
+ qbus_set_hotplug_handler(BUS(&pb->sec_bus), OBJECT(s));
if (dev->hotplugged) {
pci_default_write_config(pdev, PCI_PRIMARY_BUS,
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9b72094a61..b49775269e 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -934,7 +934,7 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), dev,
&virtio_scsi_scsi_info, vdev->bus_name);
/* override default SCSI bus hotplug-handler, with virtio-scsi's one */
- qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev));
virtio_scsi_dataplane_setup(s, errp);
}
@@ -958,7 +958,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev)
{
VirtIOSCSI *s = VIRTIO_SCSI(dev);
- qbus_set_hotplug_handler(BUS(&s->bus), NULL, &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), NULL);
virtio_scsi_common_unrealize(dev);
}
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index ec5bf9ea34..df07ab6bfb 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1147,7 +1147,7 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
&pvscsi_scsi_info, NULL);
/* override default SCSI bus hotplug-handler, with pvscsi's one */
- qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s), &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s));
pvscsi_reset_state(s);
}
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index ada18c1983..fcfe216594 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1320,7 +1320,7 @@ static void ccid_realize(USBDevice *dev, Error **errp)
usb_desc_init(dev);
qbus_create_inplace(&s->bus, sizeof(s->bus), TYPE_CCID_BUS, DEVICE(dev),
NULL);
- qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev));
s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP);
s->card = NULL;
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 15/25] hw: Fix error API violation around object_property_set_link()
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (13 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 14/25] qdev: Drop qbus_set_hotplug_handler() " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 16/25] hw/arm: Drop useless object_property_set_link() error handling Markus Armbruster
` (9 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, Alistair Francis,
Mark Cave-Ayland, Eric Auger, Aleksandar Markovic, qemu-arm,
Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Aurelien Jarno
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
virtio_gpu_pci_base_realize(), virtio_vga_base_realize(),
sparc32_ledma_device_realize(), sparc32_dma_realize(),
sparc32_dma_realize() xilinx_axidma_realize(), mips_cps_realize(),
macio_realize_ide(), xilinx_enet_realize(), and
virtio_iommu_pci_realize() are wrong that way: they reuse the argument
they pass to object_property_set_link() for another call.
Harmless, because object_property_set_link() can't actually fail for
them: it fails when the property doesn't exist, is not settable, or
its .check() method fails. Fix by passing &error_abort instead.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alistair Francis <alistair@alistair23.me>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
Cc: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/display/virtio-gpu-pci.c | 2 +-
hw/display/virtio-vga.c | 2 +-
hw/dma/sparc32_dma.c | 6 +++---
hw/dma/xilinx_axidma.c | 12 ++----------
hw/mips/cps.c | 6 ++++--
hw/misc/macio/macio.c | 3 ++-
hw/net/xilinx_axienet.c | 12 ++----------
hw/virtio/virtio-iommu-pci.c | 2 +-
8 files changed, 16 insertions(+), 29 deletions(-)
diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index b532fe8b5f..41b88b878d 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -44,7 +44,7 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
for (i = 0; i < g->conf.max_outputs; i++) {
object_property_set_link(OBJECT(g->scanout[i].con),
OBJECT(vpci_dev),
- "device", errp);
+ "device", &error_abort);
}
}
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 68a062ece6..67f409e106 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -154,7 +154,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
for (i = 0; i < g->conf.max_outputs; i++) {
object_property_set_link(OBJECT(g->scanout[i].con),
OBJECT(vpci_dev),
- "device", errp);
+ "device", &error_abort);
}
}
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index f02aca6f40..2d7dbbb92d 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -346,7 +346,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
d = qdev_new(TYPE_LANCE);
object_property_add_child(OBJECT(dev), "lance", OBJECT(d));
qdev_set_nic_properties(d, nd);
- object_property_set_link(OBJECT(d), OBJECT(dev), "dma", errp);
+ object_property_set_link(OBJECT(d), OBJECT(dev), "dma", &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
}
@@ -379,7 +379,7 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
}
espdma = qdev_new(TYPE_SPARC32_ESPDMA_DEVICE);
- object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
+ object_property_set_link(OBJECT(espdma), iommu, "iommu", &error_abort);
object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma));
sysbus_realize_and_unref(SYS_BUS_DEVICE(espdma), &error_fatal);
@@ -394,7 +394,7 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
sysbus_mmio_get_region(sbd, 0));
ledma = qdev_new(TYPE_SPARC32_LEDMA_DEVICE);
- object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
+ object_property_set_link(OBJECT(ledma), iommu, "iommu", &error_abort);
object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma));
sysbus_realize_and_unref(SYS_BUS_DEVICE(ledma), &error_fatal);
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 6a9df2c4db..a069637bf2 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -537,7 +537,6 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(&s->rx_data_dev);
XilinxAXIDMAStreamSlave *cs = XILINX_AXI_DMA_CONTROL_STREAM(
&s->rx_control_dev);
- Error *local_err = NULL;
int i;
object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,
@@ -548,11 +547,8 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
(Object **)&cs->dma,
object_property_allow_set_link,
OBJ_PROP_LINK_STRONG);
- object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &local_err);
- object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &local_err);
- if (local_err) {
- goto xilinx_axidma_realize_fail;
- }
+ object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &error_abort);
+ object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &error_abort);
for (i = 0; i < 2; i++) {
struct Stream *st = &s->streams[i];
@@ -567,10 +563,6 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
address_space_init(&s->as,
s->dma_mr ? s->dma_mr : get_system_memory(), "dma");
- return;
-
-xilinx_axidma_realize_fail:
- error_propagate(errp, local_err);
}
static void xilinx_axidma_init(Object *obj)
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index cdfab19826..5382bc86f7 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -150,8 +150,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", &err);
object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", &err);
object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", &err);
- object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic", &err);
- object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc", &err);
+ object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic",
+ &error_abort);
+ object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err);
if (err != NULL) {
error_propagate(errp, err);
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 8ba7af073c..3251c79f46 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -136,7 +136,8 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
sysbus_connect_irq(sysbus_dev, 0, irq0);
sysbus_connect_irq(sysbus_dev, 1, irq1);
qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
- object_property_set_link(OBJECT(ide), OBJECT(&s->dbdma), "dbdma", errp);
+ object_property_set_link(OBJECT(ide), OBJECT(&s->dbdma), "dbdma",
+ &error_abort);
macio_ide_register_dma(ide);
qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index c2f40b8ea9..679a359f9a 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -980,7 +980,6 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(&s->rx_data_dev);
XilinxAXIEnetStreamSlave *cs = XILINX_AXI_ENET_CONTROL_STREAM(
&s->rx_control_dev);
- Error *local_err = NULL;
object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet",
(Object **) &ds->enet,
@@ -990,11 +989,8 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
(Object **) &cs->enet,
object_property_allow_set_link,
OBJ_PROP_LINK_STRONG);
- object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &local_err);
- object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &local_err);
- if (local_err) {
- goto xilinx_enet_realize_fail;
- }
+ object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &error_abort);
+ object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &error_abort);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
s->nic = qemu_new_nic(&net_xilinx_enet_info, &s->conf,
@@ -1008,10 +1004,6 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
s->rxmem = g_malloc(s->c_rxmem);
s->txmem = g_malloc(s->c_txmem);
- return;
-
-xilinx_enet_realize_fail:
- error_propagate(errp, local_err);
}
static void xilinx_enet_init(Object *obj)
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 632533abaf..bd61d6e2f8 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -56,7 +56,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
}
object_property_set_link(OBJECT(dev),
OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
- "primary-bus", errp);
+ "primary-bus", &error_abort);
qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 16/25] hw/arm: Drop useless object_property_set_link() error handling
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (14 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 15/25] hw: Fix error API violation around object_property_set_link() Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` Markus Armbruster
` (8 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Cédric Le Goater,
Joel Stanley
object_property_set_link() fails when the property doesn't exist, is
not settable, or its .check() method fails. These are all programming
errors here, so passing it &error_abort is appropriate.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/armsse.c | 53 ++++++++++-------------------------------
hw/arm/armv7m.c | 7 ++----
hw/arm/aspeed_ast2600.c | 20 ++++------------
hw/arm/aspeed_soc.c | 14 ++++-------
hw/arm/nrf51_soc.c | 6 +----
5 files changed, 24 insertions(+), 76 deletions(-)
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index e8f8f60abc..c73cc6badf 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -564,16 +564,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
&s->container, -1);
}
object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
- "memory", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
- object_property_set_link(cpuobj, OBJECT(s), "idau", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ "memory", &error_abort);
+ object_property_set_link(cpuobj, OBJECT(s), "idau", &error_abort);
sysbus_realize(SYS_BUS_DEVICE(cpuobj), &err);
if (err) {
error_propagate(errp, err);
@@ -700,11 +692,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
return;
}
object_property_set_link(OBJECT(&s->mpc[i]), OBJECT(&s->sram[i]),
- "downstream", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ "downstream", &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->mpc[i]), &err);
if (err) {
error_propagate(errp, err);
@@ -755,11 +743,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer0), 0,
armsse_get_common_irq_in(s, 3));
mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer0), 0);
- object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]",
+ &error_abort);
qdev_prop_set_uint32(DEVICE(&s->timer1), "pclk-frq", s->mainclk_frq);
sysbus_realize(SYS_BUS_DEVICE(&s->timer1), &err);
@@ -770,12 +755,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer1), 0,
armsse_get_common_irq_in(s, 4));
mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer1), 0);
- object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
-
+ object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]",
+ &error_abort);
qdev_prop_set_uint32(DEVICE(&s->dualtimer), "pclk-frq", s->mainclk_frq);
sysbus_realize(SYS_BUS_DEVICE(&s->dualtimer), &err);
@@ -786,11 +767,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->dualtimer), 0,
armsse_get_common_irq_in(s, 5));
mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dualtimer), 0);
- object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]",
+ &error_abort);
if (info->has_mhus) {
/*
@@ -815,12 +793,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
port = g_strdup_printf("port[%d]", i + 3);
mr = sysbus_mmio_get_region(mhu_sbd, 0);
object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
- port, &err);
+ port, &error_abort);
g_free(port);
- if (err) {
- error_propagate(errp, err);
- return;
- }
/*
* Each MHU has an irq line for each CPU:
@@ -967,11 +941,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->s32ktimer), 0,
armsse_get_common_irq_in(s, 2));
mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->s32ktimer), 0);
- object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->apb_ppc1), &err);
if (err) {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index ce83586e03..3308211e9c 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -170,11 +170,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
&error_abort);
if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
- object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(s->cpu), s->idau, "idau",
+ &error_abort);
}
if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 08b3592e36..4efac02e2b 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -341,11 +341,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
}
/* I2C */
- object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
if (err) {
error_propagate(errp, err);
@@ -363,11 +360,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
}
/* FMC, The number of CS is set at the board level */
- object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
+ &error_abort);
object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
"sdram-base", &err);
if (err) {
@@ -388,11 +382,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
/* SPI */
for (i = 0; i < sc->spis_num; i++) {
object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),
- "dram", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ "dram", &error_abort);
object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index ec21de50ce..03b91bade6 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -300,11 +300,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
}
/* I2C */
- object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
if (err) {
error_propagate(errp, err);
@@ -315,11 +312,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
aspeed_soc_get_irq(s, ASPEED_I2C));
/* FMC, The number of CS is set at the board level */
- object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
+ &error_abort);
object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
"sdram-base", &err);
if (err) {
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 5a8961ddbb..20dd8b5897 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -66,11 +66,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
}
object_property_set_link(OBJECT(&s->cpu), OBJECT(&s->container), "memory",
- &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->cpu), &err);
if (err) {
error_propagate(errp, err);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 17/25] riscv/sifive_u: Fix sifive_u_soc_realize() error API violations
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 02/25] pci: Delete useless error_propagate() Markus Armbruster
` (23 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann,
Alistair Francis, Palmer Dabbelt, Bin Meng
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
sifive_u_soc_realize() is wrong that way: it passes &err to
sysbus_realize() four times before checking it. Harmless, because the
first three can't actually fail (I think).
Fix by checking for failure right away.
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/riscv/sifive_u.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7d051e7c92..a1d2edfe13 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -677,11 +677,15 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
memmap[SIFIVE_U_CLINT].size, ms->smp.cpus,
SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE, false);
- sysbus_realize(SYS_BUS_DEVICE(&s->prci), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->prci), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->prci), 0, memmap[SIFIVE_U_PRCI].base);
qdev_prop_set_uint32(DEVICE(&s->gpio), "ngpio", 16);
- sysbus_realize(SYS_BUS_DEVICE(&s->gpio), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, memmap[SIFIVE_U_GPIO].base);
/* Pass all GPIOs to the SOC layer so they are available to the board */
@@ -695,7 +699,9 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
}
qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
- sysbus_realize(SYS_BUS_DEVICE(&s->otp), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->otp), 0, memmap[SIFIVE_U_OTP].base);
if (nd->used) {
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 17/25] riscv/sifive_u: Fix sifive_u_soc_realize() error API violations
@ 2020-06-24 8:37 ` Markus Armbruster
0 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
Bastian Koppelmann, Bin Meng, qemu-riscv, Alistair Francis
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
sifive_u_soc_realize() is wrong that way: it passes &err to
sysbus_realize() four times before checking it. Harmless, because the
first three can't actually fail (I think).
Fix by checking for failure right away.
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/riscv/sifive_u.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7d051e7c92..a1d2edfe13 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -677,11 +677,15 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
memmap[SIFIVE_U_CLINT].size, ms->smp.cpus,
SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE, false);
- sysbus_realize(SYS_BUS_DEVICE(&s->prci), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->prci), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->prci), 0, memmap[SIFIVE_U_PRCI].base);
qdev_prop_set_uint32(DEVICE(&s->gpio), "ngpio", 16);
- sysbus_realize(SYS_BUS_DEVICE(&s->gpio), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, memmap[SIFIVE_U_GPIO].base);
/* Pass all GPIOs to the SOC layer so they are available to the board */
@@ -695,7 +699,9 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
}
qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
- sysbus_realize(SYS_BUS_DEVICE(&s->otp), &err);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
+ return;
+ }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->otp), 0, memmap[SIFIVE_U_OTP].base);
if (nd->used) {
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 18/25] riscv_hart: Fix riscv_harts_realize() error API violations
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 02/25] pci: Delete useless error_propagate() Markus Armbruster
` (23 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann,
Alistair Francis, Palmer Dabbelt, Bin Meng
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
riscv_harts_realize() is wrong that way: it passes @errp to
riscv_hart_realize() in a loop. I can't tell offhand whether this can
fail.
Fix by checking for failure in each iteration.
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/riscv/riscv_hart.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index e26c382259..f59fe52f0f 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -40,19 +40,13 @@ static void riscv_harts_cpu_reset(void *opaque)
cpu_reset(CPU(cpu));
}
-static void riscv_hart_realize(RISCVHartArrayState *s, int idx,
+static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
char *cpu_type, Error **errp)
{
- Error *err = NULL;
-
object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
s->harts[idx].env.mhartid = s->hartid_base + idx;
qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
- qdev_realize(DEVICE(&s->harts[idx]), NULL, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
}
static void riscv_harts_realize(DeviceState *dev, Error **errp)
@@ -63,7 +57,9 @@ static void riscv_harts_realize(DeviceState *dev, Error **errp)
s->harts = g_new0(RISCVCPU, s->num_harts);
for (n = 0; n < s->num_harts; n++) {
- riscv_hart_realize(s, n, s->cpu_type, errp);
+ if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
+ return;
+ }
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 18/25] riscv_hart: Fix riscv_harts_realize() error API violations
@ 2020-06-24 8:37 ` Markus Armbruster
0 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
Bastian Koppelmann, Bin Meng, qemu-riscv, Alistair Francis
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
riscv_harts_realize() is wrong that way: it passes @errp to
riscv_hart_realize() in a loop. I can't tell offhand whether this can
fail.
Fix by checking for failure in each iteration.
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/riscv/riscv_hart.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index e26c382259..f59fe52f0f 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -40,19 +40,13 @@ static void riscv_harts_cpu_reset(void *opaque)
cpu_reset(CPU(cpu));
}
-static void riscv_hart_realize(RISCVHartArrayState *s, int idx,
+static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
char *cpu_type, Error **errp)
{
- Error *err = NULL;
-
object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
s->harts[idx].env.mhartid = s->hartid_base + idx;
qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
- qdev_realize(DEVICE(&s->harts[idx]), NULL, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
}
static void riscv_harts_realize(DeviceState *dev, Error **errp)
@@ -63,7 +57,9 @@ static void riscv_harts_realize(DeviceState *dev, Error **errp)
s->harts = g_new0(RISCVCPU, s->num_harts);
for (n = 0; n < s->num_harts; n++) {
- riscv_hart_realize(s, n, s->cpu_type, errp);
+ if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
+ return;
+ }
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 19/25] mips/cps: Fix mips_cps_realize() error API violations
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (17 preceding siblings ...)
2020-06-24 8:37 ` Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 20/25] x86: Fix x86_cpu_new() " Markus Armbruster
` (5 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Aleksandar Markovic, Aleksandar Rikalo, Aurelien Jarno
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
mips_cps_realize() is wrong that way: it passes &err to multiple
object_property_set_FOO() without checking for failure, and then to
sysbus_realize(). Harmless, because the object_property_set_FOO()
can't actually fail here.
Fix by passing &error_abort instead.
Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/mips/cps.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 5382bc86f7..0d7f3cf673 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -100,10 +100,12 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
/* Inter-Thread Communication Unit */
if (itu_present) {
object_initialize_child(OBJECT(dev), "itu", &s->itu, TYPE_MIPS_ITU);
- object_property_set_int(OBJECT(&s->itu), 16, "num-fifo", &err);
- object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores", &err);
+ object_property_set_int(OBJECT(&s->itu), 16, "num-fifo",
+ &error_abort);
+ object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores",
+ &error_abort);
object_property_set_bool(OBJECT(&s->itu), saar_present, "saar-present",
- &err);
+ &error_abort);
if (saar_present) {
s->itu.saar = &env->CP0_SAAR;
}
@@ -119,8 +121,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
/* Cluster Power Controller */
object_initialize_child(OBJECT(dev), "cpc", &s->cpc, TYPE_MIPS_CPC);
- object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp", &err);
- object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running", &err);
+ object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp",
+ &error_abort);
+ object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err);
if (err != NULL) {
error_propagate(errp, err);
@@ -132,8 +136,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
/* Global Interrupt Controller */
object_initialize_child(OBJECT(dev), "gic", &s->gic, TYPE_MIPS_GIC);
- object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp", &err);
- object_property_set_int(OBJECT(&s->gic), 128, "num-irq", &err);
+ object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp",
+ &error_abort);
+ object_property_set_int(OBJECT(&s->gic), 128, "num-irq",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err);
if (err != NULL) {
error_propagate(errp, err);
@@ -147,9 +153,12 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
gcr_base = env->CP0_CMGCRBase << 4;
object_initialize_child(OBJECT(dev), "gcr", &s->gcr, TYPE_MIPS_GCR);
- object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", &err);
- object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", &err);
- object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", &err);
+ object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp",
+ &error_abort);
+ object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev",
+ &error_abort);
+ object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base",
+ &error_abort);
object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic",
&error_abort);
object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc",
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (18 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 19/25] mips/cps: Fix mips_cps_realize() " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 14:17 ` Igor Mammedov
2020-06-24 8:37 ` [PATCH v2 21/25] amd_iommu: Fix amdvi_realize() error API violation Markus Armbruster
` (4 subsequent siblings)
24 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost, Richard Henderson
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
x86_cpu_new() is wrong that way: it passes &local_err to
object_property_set_uint() without checking it, and then to
qdev_realize(). Harmless, because the former can't actually fail
here.
Fix by checking for failure right away. While there, replace
qdev_realize(); object_unref() by qdev_realize_and_unref().
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/i386/x86.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 34229b45c7..3a7029e6db 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -118,16 +118,10 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
{
- Object *cpu = NULL;
- Error *local_err = NULL;
+ Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
- cpu = object_new(MACHINE(x86ms)->cpu_type);
-
- object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
- qdev_realize(DEVICE(cpu), NULL, &local_err);
-
- object_unref(cpu);
- error_propagate(errp, local_err);
+ object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
+ qdev_realize_and_unref(DEVICE(cpu), NULL, errp);
}
void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 21/25] amd_iommu: Fix amdvi_realize() error API violation
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (19 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 20/25] x86: Fix x86_cpu_new() " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize " Markus Armbruster
` (3 subsequent siblings)
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost, Richard Henderson
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
amdvi_realize() is wrong that way: it passes @errp to qdev_realize(),
object_property_get_int(), and msi_init() without checking it. I
can't tell offhand whether qdev_realize() can fail here. Fix by
checking it for failure. object_property_get_int() can't. Fix by
passing &error_abort instead.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/i386/amd_iommu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index b26d30da57..087f601666 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1549,7 +1549,9 @@ static void amdvi_realize(DeviceState *dev, Error **errp)
/* This device should take care of IOMMU PCI properties */
x86_iommu->type = TYPE_AMD;
- qdev_realize(DEVICE(&s->pci), &bus->qbus, errp);
+ if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) {
+ return;
+ }
ret = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
AMDVI_CAPAB_SIZE, errp);
if (ret < 0) {
@@ -1578,7 +1580,7 @@ static void amdvi_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->mmio);
sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, AMDVI_BASE_ADDR);
pci_setup_iommu(bus, amdvi_host_dma_iommu, s);
- s->devid = object_property_get_int(OBJECT(&s->pci), "addr", errp);
+ s->devid = object_property_get_int(OBJECT(&s->pci), "addr", &error_abort);
msi_init(&s->pci.dev, 0, 1, true, false, errp);
amdvi_init(s);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize error API violation
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (20 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 21/25] amd_iommu: Fix amdvi_realize() error API violation Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 19:17 ` Alistair Francis
2020-06-24 8:37 ` [PATCH v2 23/25] aspeed: " Markus Armbruster
` (2 subsequent siblings)
24 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Alistair Francis, qemu-arm
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
stm32f205_soc_realize() and stm32f405_soc_realize() are wrong that
way: they pass &err to object_property_set_int() without checking it,
and then to qdev_realize(). Harmless, because the former can't
actually fail here.
Fix by passing &error_abort instead.
Cc: Alistair Francis <alistair@alistair23.me>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/arm/stm32f205_soc.c | 2 +-
hw/arm/stm32f405_soc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 19487544f0..56aef686c9 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -154,7 +154,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
/* ADC 1 to 3 */
object_property_set_int(OBJECT(s->adc_irqs), STM_NUM_ADCS,
- "num-lines", &err);
+ "num-lines", &error_abort);
qdev_realize(DEVICE(s->adc_irqs), NULL, &err);
if (err != NULL) {
error_propagate(errp, err);
diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index c12d9f999d..cf9228d8e7 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -172,7 +172,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
return;
}
object_property_set_int(OBJECT(&s->adc_irqs), STM_NUM_ADCS,
- "num-lines", &err);
+ "num-lines", &error_abort);
qdev_realize(DEVICE(&s->adc_irqs), NULL, &err);
if (err != NULL) {
error_propagate(errp, err);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 23/25] aspeed: Fix realize error API violation
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (21 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 24/25] hw/arm/armsse: Fix armsse_realize() " Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 25/25] arm/{bcm2835, fsl-imx25, fsl-imx6}: Fix realize error API violations Markus Armbruster
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Cédric Le Goater,
Joel Stanley
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
aspeed_soc_ast2600_realize() and aspeed_soc_realize() are wrong that
way: they pass &err to object_property_set_int() and
object_property_set_bool() without checking it, and then to
sysbus_realize(). Harmless, because the former can't actually fail
here.
Fix by passing &error_abort instead.
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/arm/aspeed_ast2600.c | 5 +++--
hw/arm/aspeed_soc.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 4efac02e2b..59a7a1370b 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -383,7 +383,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
for (i = 0; i < sc->spis_num; i++) {
object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),
"dram", &error_abort);
- object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
+ object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
error_propagate(errp, err);
@@ -434,7 +435,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
/* Net */
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
- &err);
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 03b91bade6..311458aa76 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -333,7 +333,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
/* SPI */
for (i = 0; i < sc->spis_num; i++) {
- object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
+ object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
if (err) {
error_propagate(errp, err);
@@ -384,7 +385,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
/* Net */
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
- &err);
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
if (err) {
error_propagate(errp, err);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 24/25] hw/arm/armsse: Fix armsse_realize() error API violation
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (22 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 23/25] aspeed: " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 25/25] arm/{bcm2835, fsl-imx25, fsl-imx6}: Fix realize error API violations Markus Armbruster
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
armsse_realize() is wrong that way: it passes &err to
object_property_set_int() multiple times without checking it, and then
to sysbus_realize(). Harmless, because the former can't actually fail
here.
Fix by passing &error_abort instead.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/arm/armsse.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index c73cc6badf..e2cf43ee0b 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -991,13 +991,13 @@ static void armsse_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysinfo), 0, 0x40020000);
/* System control registers */
object_property_set_int(OBJECT(&s->sysctl), info->sys_version,
- "SYS_VERSION", &err);
+ "SYS_VERSION", &error_abort);
object_property_set_int(OBJECT(&s->sysctl), info->cpuwait_rst,
- "CPUWAIT_RST", &err);
+ "CPUWAIT_RST", &error_abort);
object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
- "INITSVTOR0_RST", &err);
+ "INITSVTOR0_RST", &error_abort);
object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
- "INITSVTOR1_RST", &err);
+ "INITSVTOR1_RST", &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->sysctl), &err);
if (err) {
error_propagate(errp, err);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 25/25] arm/{bcm2835, fsl-imx25, fsl-imx6}: Fix realize error API violations
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
` (23 preceding siblings ...)
2020-06-24 8:37 ` [PATCH v2 24/25] hw/arm/armsse: Fix armsse_realize() " Markus Armbruster
@ 2020-06-24 8:37 ` Markus Armbruster
24 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-24 8:37 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé,
Andrew Baumann, Jean-Christophe Dubois
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
bcm2835_peripherals_realize(), fsl_imx25_realize() and
fsl_imx6_realize() are wrong that way: they pass &err to
object_property_set_uint() and object_property_set_bool() without
checking it, and then to sysbus_realize(). Harmless, because the
former can't actually fail here.
Fix by passing &error_abort instead.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/arm/bcm2835_peripherals.c | 12 ++++--------
hw/arm/fsl-imx25.c | 12 +++++-------
hw/arm/fsl-imx6.c | 12 +++++-------
3 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 1e975d7eec..7ffdf62067 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -283,16 +283,12 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
* For the exact details please refer to the Arasan documentation:
* SD3.0_Host_AHB_eMMC4.4_Usersguide_ver5.9_jan11_10.pdf
*/
- object_property_set_uint(OBJECT(&s->sdhci), 3, "sd-spec-version", &err);
+ object_property_set_uint(OBJECT(&s->sdhci), 3, "sd-spec-version",
+ &error_abort);
object_property_set_uint(OBJECT(&s->sdhci), BCM2835_SDHC_CAPAREG, "capareg",
- &err);
+ &error_abort);
object_property_set_bool(OBJECT(&s->sdhci), true, "pending-insert-quirk",
- &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
-
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index f32f9bce0f..7ab5c98fbe 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -260,15 +260,13 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
};
object_property_set_uint(OBJECT(&s->esdhc[i]), 2, "sd-spec-version",
- &err);
+ &error_abort);
object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES,
- "capareg", &err);
+ "capareg",
+ &error_abort);
object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
- "vendor", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ "vendor",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index d4bc4fae93..4ae3c3efc2 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -336,15 +336,13 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
/* UHS-I SDIO3.0 SDR104 1.8V ADMA */
object_property_set_uint(OBJECT(&s->esdhc[i]), 3, "sd-spec-version",
- &err);
+ &error_abort);
object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES,
- "capareg", &err);
+ "capareg",
+ &error_abort);
object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
- "vendor", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
+ "vendor",
+ &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), &err);
if (err) {
error_propagate(errp, err);
--
2.26.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way
2020-06-24 8:37 ` [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way Markus Armbruster
@ 2020-06-24 9:02 ` Greg Kurz
2020-06-25 6:42 ` Markus Armbruster
0 siblings, 1 reply; 41+ messages in thread
From: Greg Kurz @ 2020-06-24 9:02 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Jerome Forissier, Daniel P . Berrange, qemu-devel
On Wed, 24 Jun 2020 10:37:15 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Receiving the error in a local variable only to free it is less clear
> (and also less efficient) than passing NULL. Clean up.
>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Cc: Jerome Forissier <jerome@forissier.org>
> CC: Greg Kurz <groug@kaod.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
It doesn't seem to be different from v1, so:
Reviewed-by: Greg Kurz <groug@kaod.org>
and
Acked-by: Greg Kurz <groug@kaod.org> # for 9pfs
> chardev/char-socket.c | 6 ++----
> hw/9pfs/9p.c | 6 ++----
> hw/arm/virt.c | 4 +---
> hw/ppc/spapr_drc.c | 4 +---
> ui/vnc.c | 3 +--
> 5 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index afebeec5c3..b0cae97960 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -815,22 +815,20 @@ static void tcp_chr_tls_init(Chardev *chr)
> {
> SocketChardev *s = SOCKET_CHARDEV(chr);
> QIOChannelTLS *tioc;
> - Error *err = NULL;
> gchar *name;
>
> if (s->is_listen) {
> tioc = qio_channel_tls_new_server(
> s->ioc, s->tls_creds,
> s->tls_authz,
> - &err);
> + NULL);
> } else {
> tioc = qio_channel_tls_new_client(
> s->ioc, s->tls_creds,
> s->addr->u.inet.host,
> - &err);
> + NULL);
> }
> if (tioc == NULL) {
> - error_free(err);
> tcp_chr_disconnect(chr);
> return;
> }
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 45a788f6e6..9755fba9a9 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1399,7 +1399,6 @@ static void coroutine_fn v9fs_attach(void *opaque)
> size_t offset = 7;
> V9fsQID qid;
> ssize_t err;
> - Error *local_err = NULL;
>
> v9fs_string_init(&uname);
> v9fs_string_init(&aname);
> @@ -1437,9 +1436,8 @@ static void coroutine_fn v9fs_attach(void *opaque)
> error_setg(&s->migration_blocker,
> "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
> s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> - err = migrate_add_blocker(s->migration_blocker, &local_err);
> - if (local_err) {
> - error_free(local_err);
> + err = migrate_add_blocker(s->migration_blocker, NULL);
> + if (err < 0) {
> error_free(s->migration_blocker);
> s->migration_blocker = NULL;
> clunk_fid(s, fid);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index caceb1e4a0..29b9d5b2e6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -217,11 +217,9 @@ static bool cpu_type_valid(const char *cpu)
>
> static void create_kaslr_seed(VirtMachineState *vms, const char *node)
> {
> - Error *err = NULL;
> uint64_t seed;
>
> - if (qemu_guest_getrandom(&seed, sizeof(seed), &err)) {
> - error_free(err);
> + if (qemu_guest_getrandom(&seed, sizeof(seed), NULL)) {
> return;
> }
> qemu_fdt_setprop_u64(vms->fdt, node, "kaslr-seed", seed);
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 2689104295..951bcdf2c0 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -1163,16 +1163,14 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>
> if (!drc->fdt) {
> - Error *local_err = NULL;
> void *fdt;
> int fdt_size;
>
> fdt = create_device_tree(&fdt_size);
>
> if (drck->dt_populate(drc, spapr, fdt, &drc->fdt_start_offset,
> - &local_err)) {
> + NULL)) {
> g_free(fdt);
> - error_free(local_err);
> rc = SPAPR_DR_CC_RESPONSE_ERROR;
> goto out;
> }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 12a12714e1..0702a76cce 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -458,9 +458,8 @@ static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
> Error *err = NULL;
> SocketAddress *addr;
>
> - addr = qio_channel_socket_get_local_address(ioc, &err);
> + addr = qio_channel_socket_get_local_address(ioc, NULL);
> if (!addr) {
> - error_free(err);
> return prev;
> }
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init()
2020-06-24 8:37 ` [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
@ 2020-06-24 9:04 ` Greg Kurz
2020-06-25 3:52 ` David Gibson
1 sibling, 0 replies; 41+ messages in thread
From: Greg Kurz @ 2020-06-24 9:04 UTC (permalink / raw)
To: Markus Armbruster; +Cc: David Gibson, qemu-ppc, qemu-devel
On Wed, 24 Jun 2020 10:37:19 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> spapr_machine_init() leaks an Error object when
> kvmppc_check_papr_resize_hpt() fails and spapr->resize_hpt is
> SPAPR_RESIZE_HPT_DISABLED, i.e. when the host doesn't support hash
> page table resizing, and the user didn't ask for it. As harmless as
> memory leaks can possibly be. Plug it.
>
> Fixes: 30f4b05bd090564181554d0890605eb2c143e4ea
> Cc: David Gibson <dgibson@redhat.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Ditto.
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/spapr.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bd9345cdac..9bd2183ead 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2731,6 +2731,7 @@ static void spapr_machine_init(MachineState *machine)
> error_report_err(resize_hpt_err);
> exit(1);
> }
> + error_free(resize_hpt_err);
>
> spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 8:37 ` [PATCH v2 20/25] x86: Fix x86_cpu_new() " Markus Armbruster
@ 2020-06-24 14:17 ` Igor Mammedov
2020-06-24 14:20 ` Paolo Bonzini
2020-06-26 12:54 ` Markus Armbruster
0 siblings, 2 replies; 41+ messages in thread
From: Igor Mammedov @ 2020-06-24 14:17 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Richard Henderson, qemu-devel, Eduardo Habkost
On Wed, 24 Jun 2020 10:37:32 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL. Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> x86_cpu_new() is wrong that way: it passes &local_err to
> object_property_set_uint() without checking it, and then to
> qdev_realize(). Harmless, because the former can't actually fail
> here.
>
> Fix by checking for failure right away. While there, replace
> qdev_realize(); object_unref() by qdev_realize_and_unref().
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/i386/x86.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 34229b45c7..3a7029e6db 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -118,16 +118,10 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
>
> void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> {
> - Object *cpu = NULL;
> - Error *local_err = NULL;
> + Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>
> - cpu = object_new(MACHINE(x86ms)->cpu_type);
> -
> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
> - qdev_realize(DEVICE(cpu), NULL, &local_err);
> -
> - object_unref(cpu);
> - error_propagate(errp, local_err);
> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
perhaps error_fatal would suit this case better?
> + qdev_realize_and_unref(DEVICE(cpu), NULL, errp);
> }
>
> void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 14:17 ` Igor Mammedov
@ 2020-06-24 14:20 ` Paolo Bonzini
2020-06-25 6:44 ` Markus Armbruster
2020-06-25 19:29 ` Igor Mammedov
2020-06-26 12:54 ` Markus Armbruster
1 sibling, 2 replies; 41+ messages in thread
From: Paolo Bonzini @ 2020-06-24 14:20 UTC (permalink / raw)
To: Igor Mammedov, Markus Armbruster
Cc: Richard Henderson, qemu-devel, Eduardo Habkost
On 24/06/20 16:17, Igor Mammedov wrote:
>> - cpu = object_new(MACHINE(x86ms)->cpu_type);
>> -
>> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
>> - qdev_realize(DEVICE(cpu), NULL, &local_err);
>> -
>> - object_unref(cpu);
>> - error_propagate(errp, local_err);
>> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
> it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
> perhaps error_fatal would suit this case better?
No, we need to add the error_propagate dance instead.
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 05/25] tests: Use error_free_or_abort() where appropriate
2020-06-24 8:37 ` [PATCH v2 05/25] tests: Use error_free_or_abort() " Markus Armbruster
@ 2020-06-24 14:42 ` Eric Blake
0 siblings, 0 replies; 41+ messages in thread
From: Eric Blake @ 2020-06-24 14:42 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
On 6/24/20 3:37 AM, Markus Armbruster wrote:
> Replace
>
> g_assert(err != NULL);
> error_free(err);
> err = NULL;
>
> and variations thereof by
>
> error_free_or_abort(&err);
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize error API violation
2020-06-24 8:37 ` [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize " Markus Armbruster
@ 2020-06-24 19:17 ` Alistair Francis
0 siblings, 0 replies; 41+ messages in thread
From: Alistair Francis @ 2020-06-24 19:17 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Alistair Francis,
qemu-devel@nongnu.org Developers, qemu-arm
On Wed, Jun 24, 2020 at 1:44 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL. Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> stm32f205_soc_realize() and stm32f405_soc_realize() are wrong that
> way: they pass &err to object_property_set_int() without checking it,
> and then to qdev_realize(). Harmless, because the former can't
> actually fail here.
>
> Fix by passing &error_abort instead.
>
> Cc: Alistair Francis <alistair@alistair23.me>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/stm32f205_soc.c | 2 +-
> hw/arm/stm32f405_soc.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 19487544f0..56aef686c9 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -154,7 +154,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>
> /* ADC 1 to 3 */
> object_property_set_int(OBJECT(s->adc_irqs), STM_NUM_ADCS,
> - "num-lines", &err);
> + "num-lines", &error_abort);
> qdev_realize(DEVICE(s->adc_irqs), NULL, &err);
> if (err != NULL) {
> error_propagate(errp, err);
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index c12d9f999d..cf9228d8e7 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -172,7 +172,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
> return;
> }
> object_property_set_int(OBJECT(&s->adc_irqs), STM_NUM_ADCS,
> - "num-lines", &err);
> + "num-lines", &error_abort);
> qdev_realize(DEVICE(&s->adc_irqs), NULL, &err);
> if (err != NULL) {
> error_propagate(errp, err);
> --
> 2.26.2
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init()
2020-06-24 8:37 ` [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
2020-06-24 9:04 ` Greg Kurz
@ 2020-06-25 3:52 ` David Gibson
1 sibling, 0 replies; 41+ messages in thread
From: David Gibson @ 2020-06-25 3:52 UTC (permalink / raw)
To: Markus Armbruster; +Cc: David Gibson, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]
On Wed, Jun 24, 2020 at 10:37:19AM +0200, Markus Armbruster wrote:
> spapr_machine_init() leaks an Error object when
> kvmppc_check_papr_resize_hpt() fails and spapr->resize_hpt is
> SPAPR_RESIZE_HPT_DISABLED, i.e. when the host doesn't support hash
> page table resizing, and the user didn't ask for it. As harmless as
> memory leaks can possibly be. Plug it.
>
> Fixes: 30f4b05bd090564181554d0890605eb2c143e4ea
> Cc: David Gibson <dgibson@redhat.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bd9345cdac..9bd2183ead 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2731,6 +2731,7 @@ static void spapr_machine_init(MachineState *machine)
> error_report_err(resize_hpt_err);
> exit(1);
> }
> + error_free(resize_hpt_err);
>
> spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way
2020-06-24 9:02 ` Greg Kurz
@ 2020-06-25 6:42 ` Markus Armbruster
0 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-25 6:42 UTC (permalink / raw)
To: Greg Kurz; +Cc: Daniel P . Berrange, Jerome Forissier, qemu-devel
Greg Kurz <groug@kaod.org> writes:
> On Wed, 24 Jun 2020 10:37:15 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Receiving the error in a local variable only to free it is less clear
>> (and also less efficient) than passing NULL. Clean up.
>>
>> Cc: Daniel P. Berrange <berrange@redhat.com>
>> Cc: Jerome Forissier <jerome@forissier.org>
>> CC: Greg Kurz <groug@kaod.org>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
> It doesn't seem to be different from v1, so:
I missed your reply to v1. My apologies!
> Reviewed-by: Greg Kurz <groug@kaod.org>
>
> and
>
> Acked-by: Greg Kurz <groug@kaod.org> # for 9pfs
Thanks!
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 14:20 ` Paolo Bonzini
@ 2020-06-25 6:44 ` Markus Armbruster
2020-06-25 19:29 ` Igor Mammedov
1 sibling, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-06-25 6:44 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Igor Mammedov, qemu-devel, Eduardo Habkost, Richard Henderson
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 24/06/20 16:17, Igor Mammedov wrote:
>>> - cpu = object_new(MACHINE(x86ms)->cpu_type);
>>> -
>>> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
>>> - qdev_realize(DEVICE(cpu), NULL, &local_err);
>>> -
>>> - object_unref(cpu);
>>> - error_propagate(errp, local_err);
>>> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
>> it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
>> perhaps error_fatal would suit this case better?
>
> No, we need to add the error_propagate dance instead.
Thanks, will dance!
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 14:20 ` Paolo Bonzini
2020-06-25 6:44 ` Markus Armbruster
@ 2020-06-25 19:29 ` Igor Mammedov
1 sibling, 0 replies; 41+ messages in thread
From: Igor Mammedov @ 2020-06-25 19:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Richard Henderson, Markus Armbruster, Eduardo Habkost, qemu-devel
On Wed, 24 Jun 2020 16:20:16 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 24/06/20 16:17, Igor Mammedov wrote:
> >> - cpu = object_new(MACHINE(x86ms)->cpu_type);
> >> -
> >> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
> >> - qdev_realize(DEVICE(cpu), NULL, &local_err);
> >> -
> >> - object_unref(cpu);
> >> - error_propagate(errp, local_err);
> >> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
> > it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
> > perhaps error_fatal would suit this case better?
>
> No, we need to add the error_propagate dance instead.
yep, it cam be used by legacy cpu-add, so just dying isn't an option.
we need deprecate cpu-add since device-add is supported buy all interested
boards for quite a bit and once it's gone, we can use error_fatal here.
>
> Paolo
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-24 14:17 ` Igor Mammedov
2020-06-24 14:20 ` Paolo Bonzini
@ 2020-06-26 12:54 ` Markus Armbruster
2020-07-13 15:50 ` Igor Mammedov
1 sibling, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2020-06-26 12:54 UTC (permalink / raw)
To: Igor Mammedov
Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson
Igor Mammedov <imammedo@redhat.com> writes:
> On Wed, 24 Jun 2020 10:37:32 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
>> pointer to a variable containing NULL. Passing an argument of the
>> latter kind twice without clearing it in between is wrong: if the
>> first call sets an error, it no longer points to NULL for the second
>> call.
>>
>> x86_cpu_new() is wrong that way: it passes &local_err to
>> object_property_set_uint() without checking it, and then to
>> qdev_realize(). Harmless, because the former can't actually fail
>> here.
>>
>> Fix by checking for failure right away. While there, replace
>> qdev_realize(); object_unref() by qdev_realize_and_unref().
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/i386/x86.c | 12 +++---------
>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index 34229b45c7..3a7029e6db 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -118,16 +118,10 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
>>
>> void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
>> {
>> - Object *cpu = NULL;
>> - Error *local_err = NULL;
>> + Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>>
>> - cpu = object_new(MACHINE(x86ms)->cpu_type);
>> -
>> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
>> - qdev_realize(DEVICE(cpu), NULL, &local_err);
>> -
>> - object_unref(cpu);
>> - error_propagate(errp, local_err);
>> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
> it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
Really?
object_property_set_uint() fails when property "apic-id" doesn't exist,
has no ->set() method, or its ->set() fails.
Property "apic-id" is defined in x86_cpu_properties[] as
DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
This means "apic-id" exists, and its ->set() is set_uint32(). That
leaves only set_uint32() as possible source of failure.
It fails when
* the device is already realized: programming error
* the value to be stored is not an integer: object_property_set_uint()
makes it one, can't fail
* the value is not representable as uint32_t: @api_id is declared as
int64_t, but:
- pc_hot_add_cpu() passes x86_cpu_apic_id_from_index(), which is
uint32_t, converted to int64_t. Can't fail.
- x86_cpus_init() passes possible_cpus->cpus[i].arch_id, which is
uint64_t. Is this the "if user specified wrong cpu flags" case?
Aside: should the integer types be cleaned up?
To assess the bug's impact, we need to know when the other call in this
error pileup fails. If we can make both fail, we have a crash bug.
Else, we have a harmless API violation.
Any ideas on how to make the qdev_realize() fail?
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-06-26 12:54 ` Markus Armbruster
@ 2020-07-13 15:50 ` Igor Mammedov
2020-07-14 9:49 ` Markus Armbruster
0 siblings, 1 reply; 41+ messages in thread
From: Igor Mammedov @ 2020-07-13 15:50 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Richard Henderson, qemu-devel, Eduardo Habkost
On Fri, 26 Jun 2020 14:54:38 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > On Wed, 24 Jun 2020 10:37:32 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> >> pointer to a variable containing NULL. Passing an argument of the
> >> latter kind twice without clearing it in between is wrong: if the
> >> first call sets an error, it no longer points to NULL for the second
> >> call.
> >>
> >> x86_cpu_new() is wrong that way: it passes &local_err to
> >> object_property_set_uint() without checking it, and then to
> >> qdev_realize(). Harmless, because the former can't actually fail
> >> here.
> >>
> >> Fix by checking for failure right away. While there, replace
> >> qdev_realize(); object_unref() by qdev_realize_and_unref().
> >>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Richard Henderson <rth@twiddle.net>
> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> hw/i386/x86.c | 12 +++---------
> >> 1 file changed, 3 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> >> index 34229b45c7..3a7029e6db 100644
> >> --- a/hw/i386/x86.c
> >> +++ b/hw/i386/x86.c
> >> @@ -118,16 +118,10 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
> >>
> >> void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> >> {
> >> - Object *cpu = NULL;
> >> - Error *local_err = NULL;
> >> + Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
> >>
> >> - cpu = object_new(MACHINE(x86ms)->cpu_type);
> >> -
> >> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
> >> - qdev_realize(DEVICE(cpu), NULL, &local_err);
> >> -
> >> - object_unref(cpu);
> >> - error_propagate(errp, local_err);
> >> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
> > it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
>
> Really?
>
> object_property_set_uint() fails when property "apic-id" doesn't exist,
> has no ->set() method, or its ->set() fails.
>
> Property "apic-id" is defined in x86_cpu_properties[] as
>
> DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
>
> This means "apic-id" exists, and its ->set() is set_uint32(). That
> leaves only set_uint32() as possible source of failure.
>
> It fails when
>
> * the device is already realized: programming error
>
> * the value to be stored is not an integer: object_property_set_uint()
> makes it one, can't fail
>
> * the value is not representable as uint32_t: @api_id is declared as
> int64_t, but:
>
> - pc_hot_add_cpu() passes x86_cpu_apic_id_from_index(), which is
> uint32_t, converted to int64_t. Can't fail.
>
> - x86_cpus_init() passes possible_cpus->cpus[i].arch_id, which is
> uint64_t. Is this the "if user specified wrong cpu flags" case?
looking more on it, object_property_set_uint() can't really fail
> Aside: should the integer types be cleaned up?
apic_id is x86 specific subset of .arch_id.
The later is used by other targets which may need larger than 32bit integer
(if I recall correctly virt-arm uses 64bit id).
> To assess the bug's impact, we need to know when the other call in this
> error pileup fails. If we can make both fail, we have a crash bug.
> Else, we have a harmless API violation.
>
> Any ideas on how to make the qdev_realize() fail?
qemu CLI case
QEMU -cpu qemu64,enforce,topoext
legacy hotplug case:
QEMU -smp 1,maxcpus=2
(monitor) cpu-add 1
(monitor) cpu-add 1 <= fail
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 20/25] x86: Fix x86_cpu_new() error API violations
2020-07-13 15:50 ` Igor Mammedov
@ 2020-07-14 9:49 ` Markus Armbruster
0 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2020-07-14 9:49 UTC (permalink / raw)
To: Igor Mammedov
Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson
Igor Mammedov <imammedo@redhat.com> writes:
> On Fri, 26 Jun 2020 14:54:38 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
>>
>> > On Wed, 24 Jun 2020 10:37:32 +0200
>> > Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
>> >> pointer to a variable containing NULL. Passing an argument of the
>> >> latter kind twice without clearing it in between is wrong: if the
>> >> first call sets an error, it no longer points to NULL for the second
>> >> call.
>> >>
>> >> x86_cpu_new() is wrong that way: it passes &local_err to
>> >> object_property_set_uint() without checking it, and then to
>> >> qdev_realize(). Harmless, because the former can't actually fail
>> >> here.
>> >>
>> >> Fix by checking for failure right away. While there, replace
>> >> qdev_realize(); object_unref() by qdev_realize_and_unref().
>> >>
>> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> >> Cc: Richard Henderson <rth@twiddle.net>
>> >> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> >> hw/i386/x86.c | 12 +++---------
>> >> 1 file changed, 3 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> >> index 34229b45c7..3a7029e6db 100644
>> >> --- a/hw/i386/x86.c
>> >> +++ b/hw/i386/x86.c
>> >> @@ -118,16 +118,10 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
>> >>
>> >> void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
>> >> {
>> >> - Object *cpu = NULL;
>> >> - Error *local_err = NULL;
>> >> + Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>> >>
>> >> - cpu = object_new(MACHINE(x86ms)->cpu_type);
>> >> -
>> >> - object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
>> >> - qdev_realize(DEVICE(cpu), NULL, &local_err);
>> >> -
>> >> - object_unref(cpu);
>> >> - error_propagate(errp, local_err);
>> >> + object_property_set_uint(cpu, apic_id, "apic-id", &error_abort);
>> > it may fail here if user specified wrong cpu flags, but there is nothing we can do to fix it.
>>
>> Really?
>>
>> object_property_set_uint() fails when property "apic-id" doesn't exist,
>> has no ->set() method, or its ->set() fails.
>>
>> Property "apic-id" is defined in x86_cpu_properties[] as
>>
>> DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
>>
>> This means "apic-id" exists, and its ->set() is set_uint32(). That
>> leaves only set_uint32() as possible source of failure.
>>
>> It fails when
>>
>> * the device is already realized: programming error
>>
>> * the value to be stored is not an integer: object_property_set_uint()
>> makes it one, can't fail
>>
>> * the value is not representable as uint32_t: @api_id is declared as
>> int64_t, but:
>>
>> - pc_hot_add_cpu() passes x86_cpu_apic_id_from_index(), which is
>> uint32_t, converted to int64_t. Can't fail.
>>
>> - x86_cpus_init() passes possible_cpus->cpus[i].arch_id, which is
>> uint64_t. Is this the "if user specified wrong cpu flags" case?
>
> looking more on it, object_property_set_uint() can't really fail
Correct.
>> Aside: should the integer types be cleaned up?
>
> apic_id is x86 specific subset of .arch_id.
> The later is used by other targets which may need larger than 32bit integer
> (if I recall correctly virt-arm uses 64bit id).
I trust this works and makes sense, but the implicit conversions still
give me an uneasy feeling.
>> To assess the bug's impact, we need to know when the other call in this
>> error pileup fails. If we can make both fail, we have a crash bug.
>> Else, we have a harmless API violation.
>>
>> Any ideas on how to make the qdev_realize() fail?
> qemu CLI case
> QEMU -cpu qemu64,enforce,topoext
>
> legacy hotplug case:
> QEMU -smp 1,maxcpus=2
> (monitor) cpu-add 1
> (monitor) cpu-add 1 <= fail
Testing:
$ qemu-system-x86_64 -nodefaults -display none -S -monitor stdio -smp 1,maxcpus=2
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) cpu-add 1
cpu_add is deprecated, please use device_add instead
(qemu) cpu-add 1
cpu_add is deprecated, please use device_add instead
Error: CPU[1] with APIC ID 1 exists
(qemu)
We're good.
$ qemu-system-x86_64 -cpu qemu64,enforce,topoext
qemu-system-x86_64: warning: TCG doesn't support requested feature: CPUID.80000001H:ECX.topoext [bit 22]
qemu-system-x86_64: TCG doesn't support requested features
[Exit 1 ]
Are we good?
To finish the job in time for the freeze, I made do with this
non-assessment (commit 18d588fe1e1):
To assess the bug's impact, we'd need to figure out how to make both
calls fail. Too much work for ignorant me, sorry.
Thanks!
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2020-07-14 9:50 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 8:37 [PATCH v2 00/25] Error handling fixes & cleanups Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 01/25] net/virtio: Fix failover_replug_primary() return value regression Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 02/25] pci: Delete useless error_propagate() Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 03/25] Clean up some calls to ignore Error objects the right way Markus Armbruster
2020-06-24 9:02 ` Greg Kurz
2020-06-25 6:42 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 04/25] tests: Use &error_abort where appropriate Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 05/25] tests: Use error_free_or_abort() " Markus Armbruster
2020-06-24 14:42 ` Eric Blake
2020-06-24 8:37 ` [PATCH v2 06/25] usb/dev-mtp: Fix Error double free after inotify failure Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 07/25] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
2020-06-24 9:04 ` Greg Kurz
2020-06-25 3:52 ` David Gibson
2020-06-24 8:37 ` [PATCH v2 08/25] qga: Plug unlikely memory leak in guest-set-memory-blocks Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 09/25] sd/milkymist-memcard: Plug minor memory leak in realize Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 10/25] test-util-filemonitor: Plug unlikely memory leak Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 11/25] vnc: Plug minor memory leak in vnc_display_open() Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 12/25] aspeed: Clean up roundabout error propagation Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 13/25] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 14/25] qdev: Drop qbus_set_hotplug_handler() " Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 15/25] hw: Fix error API violation around object_property_set_link() Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 16/25] hw/arm: Drop useless object_property_set_link() error handling Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 17/25] riscv/sifive_u: Fix sifive_u_soc_realize() error API violations Markus Armbruster
2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 18/25] riscv_hart: Fix riscv_harts_realize() " Markus Armbruster
2020-06-24 8:37 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 19/25] mips/cps: Fix mips_cps_realize() " Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 20/25] x86: Fix x86_cpu_new() " Markus Armbruster
2020-06-24 14:17 ` Igor Mammedov
2020-06-24 14:20 ` Paolo Bonzini
2020-06-25 6:44 ` Markus Armbruster
2020-06-25 19:29 ` Igor Mammedov
2020-06-26 12:54 ` Markus Armbruster
2020-07-13 15:50 ` Igor Mammedov
2020-07-14 9:49 ` Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 21/25] amd_iommu: Fix amdvi_realize() error API violation Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 22/25] arm/stm32f205 arm/stm32f405: Fix realize " Markus Armbruster
2020-06-24 19:17 ` Alistair Francis
2020-06-24 8:37 ` [PATCH v2 23/25] aspeed: " Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 24/25] hw/arm/armsse: Fix armsse_realize() " Markus Armbruster
2020-06-24 8:37 ` [PATCH v2 25/25] arm/{bcm2835, fsl-imx25, fsl-imx6}: Fix realize error API violations Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.