* [PATCH v2 01/10] nvdimm: Plug memory leak in uuid property setter
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
@ 2020-05-05 10:18 ` Markus Armbruster
2020-05-06 14:23 ` Shivaprasad G Bhat
2020-05-05 10:19 ` [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors Markus Armbruster
` (8 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Xiao Guangrong, Shivaprasad G Bhat
nvdimm_set_uuid() leaks memory on qemu_uuid_parse() failure. Fix
that.
Fixes: 6c5627bb24dcd68c997857a8b671617333b1289f
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Cc: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/mem/nvdimm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 8e426d24bb..d5752f7bf6 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -97,7 +97,6 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name,
if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) {
error_setg(errp, "Property '%s.%s' has invalid value",
object_get_typename(obj), name);
- goto out;
}
g_free(value);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 01/10] nvdimm: Plug memory leak in uuid property setter
2020-05-05 10:18 ` [PATCH v2 01/10] nvdimm: Plug memory leak in uuid property setter Markus Armbruster
@ 2020-05-06 14:23 ` Shivaprasad G Bhat
0 siblings, 0 replies; 17+ messages in thread
From: Shivaprasad G Bhat @ 2020-05-06 14:23 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Xiao Guangrong
On 05/05/2020 03:48 PM, Markus Armbruster wrote:
> nvdimm_set_uuid() leaks memory on qemu_uuid_parse() failure. Fix
> that.
>
> Fixes: 6c5627bb24dcd68c997857a8b671617333b1289f
> Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
> Cc: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Thanks for finding and fixing this Markus.
Tested-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Reviewed-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Regards,
Shivaprasad
> ---
> hw/mem/nvdimm.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 8e426d24bb..d5752f7bf6 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -97,7 +97,6 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name,
> if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) {
> error_setg(errp, "Property '%s.%s' has invalid value",
> object_get_typename(obj), name);
> - goto out;
> }
> g_free(value);
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
2020-05-05 10:18 ` [PATCH v2 01/10] nvdimm: Plug memory leak in uuid property setter Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 13:59 ` Paul Durrant
2020-05-05 10:19 ` [PATCH v2 03/10] s390x/cpumodel: Fix harmless misuse of visit_check_struct() Markus Armbruster
` (7 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Perard, xen-devel, Stefano Stabellini, Gerd Hoffmann,
Paul Durrant
usbback_portid_add() leaks the error when qdev_device_add() fails.
Fix that. While there, use the error to improve the error message.
The qemu_opts_from_qdict() similarly leaks on failure. But any
failure there is a programming error. Pass &error_abort.
Fixes: 816ac92ef769f9ffc534e49a1bb6177bddce7aa2
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Paul Durrant <paul@xen.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: xen-devel@lists.xenproject.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/usb/xen-usb.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 961190d0f7..4d266d7bb4 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -30,6 +30,7 @@
#include "hw/usb.h"
#include "hw/xen/xen-legacy-backend.h"
#include "monitor/qdev.h"
+#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
@@ -755,13 +756,16 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
qdict_put_int(qdict, "port", port);
qdict_put_int(qdict, "hostbus", atoi(busid));
qdict_put_str(qdict, "hostport", portname);
- opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
- if (local_err) {
- goto err;
- }
+ opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict,
+ &error_abort);
usbif->ports[port - 1].dev = USB_DEVICE(qdev_device_add(opts, &local_err));
if (!usbif->ports[port - 1].dev) {
- goto err;
+ qobject_unref(qdict);
+ xen_pv_printf(&usbif->xendev, 0,
+ "device %s could not be opened: %s\n",
+ busid, error_get_pretty(local_err));
+ error_free(local_err);
+ return;
}
qobject_unref(qdict);
speed = usbif->ports[port - 1].dev->speed;
@@ -793,11 +797,6 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
usbback_hotplug_enq(usbif, port);
TR_BUS(&usbif->xendev, "port %d attached\n", port);
- return;
-
-err:
- qobject_unref(qdict);
- xen_pv_printf(&usbif->xendev, 0, "device %s could not be opened\n", busid);
}
static void usbback_process_port(struct usbback_info *usbif, unsigned port)
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors
2020-05-05 10:19 ` [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors Markus Armbruster
@ 2020-05-05 13:59 ` Paul Durrant
0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2020-05-05 13:59 UTC (permalink / raw)
To: 'Markus Armbruster', qemu-devel
Cc: 'Anthony Perard', xen-devel, 'Stefano Stabellini',
'Gerd Hoffmann'
> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: 05 May 2020 11:19
> To: qemu-devel@nongnu.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com>; Paul
> Durrant <paul@xen.org>; Gerd Hoffmann <kraxel@redhat.com>; xen-devel@lists.xenproject.org
> Subject: [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors
>
> usbback_portid_add() leaks the error when qdev_device_add() fails.
> Fix that. While there, use the error to improve the error message.
>
> The qemu_opts_from_qdict() similarly leaks on failure. But any
> failure there is a programming error. Pass &error_abort.
>
> Fixes: 816ac92ef769f9ffc534e49a1bb6177bddce7aa2
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Paul Durrant <paul@xen.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: xen-devel@lists.xenproject.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paul Durrant <paul@xen.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 03/10] s390x/cpumodel: Fix harmless misuse of visit_check_struct()
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
2020-05-05 10:18 ` [PATCH v2 01/10] nvdimm: Plug memory leak in uuid property setter Markus Armbruster
2020-05-05 10:19 ` [PATCH v2 02/10] xen: Fix and improve handling of device_add usb-host errors Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:19 ` [PATCH v2 04/10] tests/migration: Tighten error checking Markus Armbruster
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
Commit e47970f51d "s390x/cpumodel: Fix query-cpu-model-FOO error API
violations" neglected to change visit_check_struct()'s Error **
argument along with the others. If visit_check_struct() failed, we'd
take the success path. Fortunately, it can't fail here:
qobject_input_check_struct() checks we consumed the whole dictionary,
and to get here, we did. Fix it anyway.
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
target/s390x/cpu_models.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7c32180269..87a8028ad3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -524,7 +524,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
}
}
if (!err) {
- visit_check_struct(visitor, errp);
+ visit_check_struct(visitor, &err);
}
visit_end_struct(visitor, NULL);
visit_free(visitor);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 04/10] tests/migration: Tighten error checking
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (2 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 03/10] s390x/cpumodel: Fix harmless misuse of visit_check_struct() Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:37 ` Philippe Mathieu-Daudé
2020-05-05 10:19 ` [PATCH v2 05/10] error: Use error_reportf_err() where appropriate Markus Armbruster
` (5 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel
migrate_get_socket_address() neglects to check
visit_type_SocketAddressList() failure. This smells like a leak, but
it actually will crash dereferencing @addrs. Pass &error_abort to
remove the code smell.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qtest/migration-test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 2568c9529c..dc3490c9fa 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "libqtest.h"
+#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qemu/module.h"
#include "qemu/option.h"
@@ -301,7 +302,6 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
{
QDict *rsp;
char *result;
- Error *local_err = NULL;
SocketAddressList *addrs;
Visitor *iv = NULL;
QObject *object;
@@ -310,7 +310,7 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
object = qdict_get(rsp, parameter);
iv = qobject_input_visitor_new(object);
- visit_type_SocketAddressList(iv, NULL, &addrs, &local_err);
+ visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
visit_free(iv);
/* we are only using a single address */
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 04/10] tests/migration: Tighten error checking
2020-05-05 10:19 ` [PATCH v2 04/10] tests/migration: Tighten error checking Markus Armbruster
@ 2020-05-05 10:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-05 10:37 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
On 5/5/20 12:19 PM, Markus Armbruster wrote:
> migrate_get_socket_address() neglects to check
> visit_type_SocketAddressList() failure. This smells like a leak, but
> it actually will crash dereferencing @addrs. Pass &error_abort to
> remove the code smell.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/qtest/migration-test.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 2568c9529c..dc3490c9fa 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -13,6 +13,7 @@
> #include "qemu/osdep.h"
>
> #include "libqtest.h"
> +#include "qapi/error.h"
> #include "qapi/qmp/qdict.h"
> #include "qemu/module.h"
> #include "qemu/option.h"
> @@ -301,7 +302,6 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
> {
> QDict *rsp;
> char *result;
> - Error *local_err = NULL;
> SocketAddressList *addrs;
> Visitor *iv = NULL;
> QObject *object;
> @@ -310,7 +310,7 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
> object = qdict_get(rsp, parameter);
>
> iv = qobject_input_visitor_new(object);
> - visit_type_SocketAddressList(iv, NULL, &addrs, &local_err);
> + visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
> visit_free(iv);
>
> /* we are only using a single address */
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 05/10] error: Use error_reportf_err() where appropriate
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (3 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 04/10] tests/migration: Tighten error checking Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:19 ` [PATCH v2 06/10] mips/malta: Fix create_cps() error handling Markus Armbruster
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Replace
error_report("...: %s", ..., error_get_pretty(err));
by
error_reportf_err(err, "...: ", ...);
One of the replaced messages lacked a colon. Add it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
chardev/char-socket.c | 5 +++--
hw/sd/pxa2xx_mmci.c | 4 ++--
hw/sd/sd.c | 4 ++--
hw/usb/dev-mtp.c | 9 +++++----
qemu-nbd.c | 7 +++----
scsi/qemu-pr-helper.c | 4 ++--
6 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 185fe38dda..e5ee685f8c 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -138,8 +138,9 @@ static void check_report_connect_error(Chardev *chr,
SocketChardev *s = SOCKET_CHARDEV(chr);
if (!s->connect_err_reported) {
- error_report("Unable to connect character device %s: %s",
- chr->label, error_get_pretty(err));
+ error_reportf_err(err,
+ "Unable to connect character device %s: ",
+ chr->label);
s->connect_err_reported = true;
}
qemu_chr_socket_restart_timer(chr);
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 8f9ab0ec16..f9c50ddda5 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -497,12 +497,12 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk, &err);
if (err) {
- error_report("failed to init SD card: %s", error_get_pretty(err));
+ error_reportf_err(err, "failed to init SD card: ");
return NULL;
}
object_property_set_bool(OBJECT(carddev), true, "realized", &err);
if (err) {
- error_report("failed to init SD card: %s", error_get_pretty(err));
+ error_reportf_err(err, "failed to init SD card: ");
return NULL;
}
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 71a9af09ab..3c06a0ac6d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -703,13 +703,13 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
dev = DEVICE(obj);
qdev_prop_set_drive(dev, "drive", blk, &err);
if (err) {
- error_report("sd_init failed: %s", error_get_pretty(err));
+ error_reportf_err(err, "sd_init failed: ");
return NULL;
}
qdev_prop_set_bit(dev, "spi", is_spi);
object_property_set_bool(obj, true, "realized", &err);
if (err) {
- error_report("sd_init failed: %s", error_get_pretty(err));
+ error_reportf_err(err, "sd_init failed: ");
return NULL;
}
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 20717f026b..168428156b 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -631,8 +631,9 @@ static void usb_mtp_object_readdir(MTPState *s, MTPObject *o)
int64_t id = qemu_file_monitor_add_watch(s->file_monitor, o->path, NULL,
file_monitor_event, s, &err);
if (id == -1) {
- error_report("usb-mtp: failed to add watch for %s: %s", o->path,
- error_get_pretty(err));
+ 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,
@@ -1276,8 +1277,8 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
s->file_monitor = qemu_file_monitor_new(&err);
if (err) {
- error_report("usb-mtp: file monitoring init failed: %s",
- error_get_pretty(err));
+ error_reportf_err(err,
+ "usb-mtp: file monitoring init failed: ");
error_free(err);
} else {
QTAILQ_INIT(&s->events);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4aa005004e..fe8b0052a2 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -856,8 +856,7 @@ int main(int argc, char **argv)
}
tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err);
if (local_err) {
- error_report("Failed to get TLS creds %s",
- error_get_pretty(local_err));
+ error_reportf_err(local_err, "Failed to get TLS creds: ");
exit(EXIT_FAILURE);
}
} else {
@@ -979,8 +978,8 @@ int main(int argc, char **argv)
&local_err);
if (sioc == NULL) {
object_unref(OBJECT(server));
- error_report("Failed to use socket activation: %s",
- error_get_pretty(local_err));
+ error_reportf_err(local_err,
+ "Failed to use socket activation: ");
exit(EXIT_FAILURE);
}
qio_net_listener_add(server, sioc);
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 181ed4a186..57ad830d54 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -1030,8 +1030,8 @@ int main(int argc, char **argv)
server_ioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
&local_err);
if (server_ioc == NULL) {
- error_report("Failed to use socket activation: %s",
- error_get_pretty(local_err));
+ error_reportf_err(local_err,
+ "Failed to use socket activation: ");
exit(EXIT_FAILURE);
}
}
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 06/10] mips/malta: Fix create_cps() error handling
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (4 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 05/10] error: Use error_reportf_err() where appropriate Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:41 ` Aleksandar Markovic
2020-05-05 10:19 ` [PATCH v2 07/10] mips/boston: Fix boston_mach_init() " Markus Armbruster
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel
Cc: Aleksandar Markovic, Philippe Mathieu-Daudé,
Philippe Mathieu-Daudé,
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
create_cps() is wrong that way. The last calls treats an error as
fatal. Do that for the prior ones, too.
Fixes: bff384a4fbd5d0e86939092e74e766ef0f5f592c
Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/mips_malta.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index e4c4de1b4e..17bf41616b 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1185,17 +1185,14 @@ static void create_cpu_without_cps(MachineState *ms,
static void create_cps(MachineState *ms, MaltaState *s,
qemu_irq *cbus_irq, qemu_irq *i8259_irq)
{
- Error *err = NULL;
-
sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps),
TYPE_MIPS_CPS);
- object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", &err);
- object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", &err);
- object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
- if (err != NULL) {
- error_report("%s", error_get_pretty(err));
- exit(1);
- }
+ object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type",
+ &error_fatal);
+ object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp",
+ &error_fatal);
+ object_property_set_bool(OBJECT(&s->cps), true, "realized",
+ &error_fatal);
sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 06/10] mips/malta: Fix create_cps() error handling
2020-05-05 10:19 ` [PATCH v2 06/10] mips/malta: Fix create_cps() error handling Markus Armbruster
@ 2020-05-05 10:41 ` Aleksandar Markovic
0 siblings, 0 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2020-05-05 10:41 UTC (permalink / raw)
To: Markus Armbruster
Cc: Philippe Mathieu-Daudé,
qemu-devel, Aurelien Jarno, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]
уторак, 05. мај 2020., Markus Armbruster <armbru@redhat.com> је написао/ла:
> 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
>
> create_cps() is wrong that way. The last calls treats an error as
> fatal. Do that for the prior ones, too.
>
> Fixes: bff384a4fbd5d0e86939092e74e766ef0f5f592c
> Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/mips/mips_malta.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
>
Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index e4c4de1b4e..17bf41616b 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1185,17 +1185,14 @@ static void create_cpu_without_cps(MachineState
> *ms,
> static void create_cps(MachineState *ms, MaltaState *s,
> qemu_irq *cbus_irq, qemu_irq *i8259_irq)
> {
> - Error *err = NULL;
> -
> sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps),
> sizeof(s->cps),
> TYPE_MIPS_CPS);
> - object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type",
> &err);
> - object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp",
> &err);
> - object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
> - if (err != NULL) {
> - error_report("%s", error_get_pretty(err));
> - exit(1);
> - }
> + object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type",
> + &error_fatal);
> + object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp",
> + &error_fatal);
> + object_property_set_bool(OBJECT(&s->cps), true, "realized",
> + &error_fatal);
>
> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
>
> --
> 2.21.1
>
>
[-- Attachment #2: Type: text/html, Size: 3772 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 07/10] mips/boston: Fix boston_mach_init() error handling
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (5 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 06/10] mips/malta: Fix create_cps() error handling Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:40 ` Aleksandar Markovic
2020-05-05 10:19 ` [PATCH v2 08/10] mips/boston: Plug memory leak in boston_mach_init() Markus Armbruster
` (2 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Paul Burton, Aleksandar Rikalo
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.
boston_mach_init() is wrong that way. The last calls treats an error
as fatal. Do that for the prior ones, too.
Fixes: df1d8a1f29f567567b9d20be685a4241282e7005
Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/mips/boston.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 98ecd25e8e..2832dfa6ae 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -458,14 +458,11 @@ static void boston_mach_init(MachineState *machine)
sysbus_init_child_obj(OBJECT(machine), "cps", OBJECT(&s->cps),
sizeof(s->cps), TYPE_MIPS_CPS);
object_property_set_str(OBJECT(&s->cps), machine->cpu_type, "cpu-type",
- &err);
- object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp", &err);
- object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
-
- if (err != NULL) {
- error_report("%s", error_get_pretty(err));
- exit(1);
- }
+ &error_fatal);
+ object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp",
+ &error_fatal);
+ object_property_set_bool(OBJECT(&s->cps), true, "realized",
+ &error_fatal);
sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 07/10] mips/boston: Fix boston_mach_init() error handling
2020-05-05 10:19 ` [PATCH v2 07/10] mips/boston: Fix boston_mach_init() " Markus Armbruster
@ 2020-05-05 10:40 ` Aleksandar Markovic
0 siblings, 0 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2020-05-05 10:40 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paul Burton, Aleksandar Rikalo, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]
уторак, 05. мај 2020., Markus Armbruster <armbru@redhat.com> је написао/ла:
> 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.
>
> boston_mach_init() is wrong that way. The last calls treats an error
> as fatal. Do that for the prior ones, too.
>
> Fixes: df1d8a1f29f567567b9d20be685a4241282e7005
> Cc: Paul Burton <pburton@wavecomp.com>
> Cc: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> hw/mips/boston.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 98ecd25e8e..2832dfa6ae 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -458,14 +458,11 @@ static void boston_mach_init(MachineState *machine)
> sysbus_init_child_obj(OBJECT(machine), "cps", OBJECT(&s->cps),
> sizeof(s->cps), TYPE_MIPS_CPS);
> object_property_set_str(OBJECT(&s->cps), machine->cpu_type,
> "cpu-type",
> - &err);
> - object_property_set_int(OBJECT(&s->cps), machine->smp.cpus,
> "num-vp", &err);
> - object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
> -
> - if (err != NULL) {
> - error_report("%s", error_get_pretty(err));
> - exit(1);
> - }
> + &error_fatal);
> + object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp",
> + &error_fatal);
> + object_property_set_bool(OBJECT(&s->cps), true, "realized",
> + &error_fatal);
>
> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
>
> --
> 2.21.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 3210 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 08/10] mips/boston: Plug memory leak in boston_mach_init()
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (6 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 07/10] mips/boston: Fix boston_mach_init() " Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:39 ` Aleksandar Markovic
2020-05-05 10:19 ` [PATCH v2 09/10] arm/sabrelite: Consistently use &error_fatal in sabrelite_init() Markus Armbruster
2020-05-05 10:19 ` [PATCH v2 10/10] i386: Fix x86_cpu_load_model() error API violation Markus Armbruster
9 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Paul Burton, Aleksandar Rikalo
Fixes: df1d8a1f29f567567b9d20be685a4241282e7005
Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/mips/boston.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 2832dfa6ae..a896056be1 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -426,7 +426,6 @@ static void boston_mach_init(MachineState *machine)
{
DeviceState *dev;
BostonState *s;
- Error *err = NULL;
MemoryRegion *flash, *ddr_low_alias, *lcd, *platreg;
MemoryRegion *sys_mem = get_system_memory();
XilinxPCIEHost *pcie2;
@@ -467,7 +466,8 @@ static void boston_mach_init(MachineState *machine)
sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
flash = g_new(MemoryRegion, 1);
- memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, &err);
+ memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB,
+ &error_fatal);
memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
memory_region_add_subregion_overlap(sys_mem, 0x80000000, machine->ram, 0);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 08/10] mips/boston: Plug memory leak in boston_mach_init()
2020-05-05 10:19 ` [PATCH v2 08/10] mips/boston: Plug memory leak in boston_mach_init() Markus Armbruster
@ 2020-05-05 10:39 ` Aleksandar Markovic
0 siblings, 0 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2020-05-05 10:39 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paul Burton, Aleksandar Rikalo, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
уторак, 05. мај 2020., Markus Armbruster <armbru@redhat.com> је написао/ла:
> Fixes: df1d8a1f29f567567b9d20be685a4241282e7005
> Cc: Paul Burton <pburton@wavecomp.com>
> Cc: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/mips/boston.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>
Thank you, Markus, for spotting this error.
Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
I am fine with you selecting this and another mips-related patch in this
series in your pull request, that will be result of this series.
Truly yours,
Aleksandar
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 2832dfa6ae..a896056be1 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -426,7 +426,6 @@ static void boston_mach_init(MachineState *machine)
> {
> DeviceState *dev;
> BostonState *s;
> - Error *err = NULL;
> MemoryRegion *flash, *ddr_low_alias, *lcd, *platreg;
> MemoryRegion *sys_mem = get_system_memory();
> XilinxPCIEHost *pcie2;
> @@ -467,7 +466,8 @@ static void boston_mach_init(MachineState *machine)
> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
>
> flash = g_new(MemoryRegion, 1);
> - memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, &err);
> + memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB,
> + &error_fatal);
> memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
>
> memory_region_add_subregion_overlap(sys_mem, 0x80000000,
> machine->ram, 0);
> --
> 2.21.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2934 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 09/10] arm/sabrelite: Consistently use &error_fatal in sabrelite_init()
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (7 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 08/10] mips/boston: Plug memory leak in boston_mach_init() Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
2020-05-05 10:19 ` [PATCH v2 10/10] i386: Fix x86_cpu_load_model() error API violation Markus Armbruster
9 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Jean-Christophe Dubois
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/sabrelite.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index e31694bb92..04f4b96591 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -41,7 +41,6 @@ static void sabrelite_reset_secondary(ARMCPU *cpu,
static void sabrelite_init(MachineState *machine)
{
FslIMX6State *s;
- Error *err = NULL;
/* Check the amount of memory is compatible with the SOC */
if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
@@ -52,11 +51,7 @@ static void sabrelite_init(MachineState *machine)
s = FSL_IMX6(object_new(TYPE_FSL_IMX6));
object_property_add_child(OBJECT(machine), "soc", OBJECT(s), &error_fatal);
- object_property_set_bool(OBJECT(s), true, "realized", &err);
- if (err != NULL) {
- error_report("%s", error_get_pretty(err));
- exit(1);
- }
+ object_property_set_bool(OBJECT(s), true, "realized", &error_fatal);
memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR,
machine->ram);
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 10/10] i386: Fix x86_cpu_load_model() error API violation
2020-05-05 10:18 [PATCH v2 00/10] More miscellaneous error handling fixes Markus Armbruster
` (8 preceding siblings ...)
2020-05-05 10:19 ` [PATCH v2 09/10] arm/sabrelite: Consistently use &error_fatal in sabrelite_init() Markus Armbruster
@ 2020-05-05 10:19 ` Markus Armbruster
9 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-05-05 10:19 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_load_model() is wrong that way. Harmless, because its @errp
is always &error_abort. To fix, cut out the @errp middleman.
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>
---
target/i386/cpu.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9c256ab159..16ed95e8da 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5078,7 +5078,7 @@ static void x86_cpu_apply_version_props(X86CPU *cpu, X86CPUModel *model)
/* Load data from X86CPUDefinition into a X86CPU object
*/
-static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model, Error **errp)
+static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
{
X86CPUDefinition *def = model->cpudef;
CPUX86State *env = &cpu->env;
@@ -5092,13 +5092,19 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model, Error **errp)
*/
/* CPU models only set _minimum_ values for level/xlevel: */
- object_property_set_uint(OBJECT(cpu), def->level, "min-level", errp);
- object_property_set_uint(OBJECT(cpu), def->xlevel, "min-xlevel", errp);
+ object_property_set_uint(OBJECT(cpu), def->level, "min-level",
+ &error_abort);
+ object_property_set_uint(OBJECT(cpu), def->xlevel, "min-xlevel",
+ &error_abort);
- object_property_set_int(OBJECT(cpu), def->family, "family", errp);
- object_property_set_int(OBJECT(cpu), def->model, "model", errp);
- object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
- object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+ object_property_set_int(OBJECT(cpu), def->family, "family",
+ &error_abort);
+ object_property_set_int(OBJECT(cpu), def->model, "model",
+ &error_abort);
+ object_property_set_int(OBJECT(cpu), def->stepping, "stepping",
+ &error_abort);
+ object_property_set_str(OBJECT(cpu), def->model_id, "model-id",
+ &error_abort);
for (w = 0; w < FEATURE_WORDS; w++) {
env->features[w] = def->features[w];
}
@@ -5135,7 +5141,8 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model, Error **errp)
vendor = host_vendor;
}
- object_property_set_str(OBJECT(cpu), vendor, "vendor", errp);
+ object_property_set_str(OBJECT(cpu), vendor, "vendor",
+ &error_abort);
x86_cpu_apply_version_props(cpu, model);
}
@@ -6981,7 +6988,7 @@ static void x86_cpu_initfn(Object *obj)
object_property_add_alias(obj, "sse4_2", obj, "sse4.2", &error_abort);
if (xcc->model) {
- x86_cpu_load_model(cpu, xcc->model, &error_abort);
+ x86_cpu_load_model(cpu, xcc->model);
}
}
--
2.21.1
^ permalink raw reply related [flat|nested] 17+ messages in thread