From 118bfd76c9c604588cb3f97811710576f58e5a76 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:05:32 +0200 Subject: qdev: Use returned bool to check for qdev_realize() etc. failure Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref(). Coccinelle script: @@ identifier fun = { isa_realize_and_unref, pci_realize_and_unref, qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, sysbus_realize_and_unref, usb_realize_and_unref }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Nothing to convert there; skipped. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. A few line breaks tidied up manually. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Message-Id: <20200707160613.848843-5-armbru@redhat.com> --- hw/misc/macio/cuda.c | 3 +-- hw/misc/macio/macio.c | 18 ++++++------------ hw/misc/macio/pmu.c | 3 +-- 3 files changed, 8 insertions(+), 16 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index 5bbc777..cc7c26d 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -526,8 +526,7 @@ static void cuda_realize(DeviceState *dev, Error **errp) ADBBusState *adb_bus = &s->adb_bus; struct tm tm; - sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_cuda), &err); - if (err) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_cuda), &err)) { error_propagate(errp, err); return; } diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index 3251c79..4241479 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -100,8 +100,7 @@ static void macio_common_realize(PCIDevice *d, Error **errp) SysBusDevice *sysbus_dev; Error *err = NULL; - qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } @@ -116,8 +115,7 @@ static void macio_common_realize(PCIDevice *d, Error **errp) qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1)); qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial); qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial); - qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } @@ -159,8 +157,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp) qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency", s->frequency); - qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } @@ -176,8 +173,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp) sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCA_IRQ)); - qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } @@ -345,8 +341,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) object_property_set_link(OBJECT(&s->pmu), OBJECT(sysbus_dev), "gpio", &error_abort); qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb); - qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } @@ -363,8 +358,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency", s->frequency); - qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err); - if (err) { + if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err)) { error_propagate(errp, err); return; } diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index 598d8e7..42ba963 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -740,8 +740,7 @@ static void pmu_realize(DeviceState *dev, Error **errp) ADBBusState *adb_bus = &s->adb_bus; struct tm tm; - sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_pmu), &err); - if (err) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_pmu), &err)) { error_propagate(errp, err); return; } -- cgit v1.1 From 0a15a73236cb068cfd28f8c6846f66ab007f81bb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:05:33 +0200 Subject: macio: Tidy up error handling in macio_newworld_realize() macio_newworld_realize() effectively ignores ns->gpio realization errors, leaking the Error object. Fortunately, macio_gpio_realize() can't actually fail. Tidy up. Cc: Mark Cave-Ayland Cc: David Gibson Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Acked-by: David Gibson Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Message-Id: <20200707160613.848843-6-armbru@redhat.com> --- hw/misc/macio/macio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'hw/misc') diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index 4241479..be66bb7 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -334,7 +334,9 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) &error_abort); memory_region_add_subregion(&s->bar, 0x50, sysbus_mmio_get_region(sysbus_dev, 0)); - qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), &err); + if (!qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), errp)) { + return; + } /* PMU */ object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU); -- cgit v1.1 From 62a35aaa310807fa161ca041ddb0f308faeb582b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:05:46 +0200 Subject: qapi: Use returned bool to check for failure, Coccinelle part The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*"; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200707160613.848843-19-armbru@redhat.com> --- hw/misc/aspeed_sdmc.c | 3 +-- hw/misc/pca9552.c | 3 +-- hw/misc/tmp105.c | 3 +-- hw/misc/tmp421.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 25e1e58..2887444 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -227,8 +227,7 @@ static void aspeed_sdmc_set_ram_size(Object *obj, Visitor *v, const char *name, AspeedSDMCState *s = ASPEED_SDMC(obj); AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s); - visit_type_int(v, name, &value, &local_err); - if (local_err) { + if (!visit_type_int(v, name, &value, &local_err)) { error_propagate(errp, local_err); return; } diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c index 68b574d..d2b99fc 100644 --- a/hw/misc/pca9552.c +++ b/hw/misc/pca9552.c @@ -296,8 +296,7 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name, uint8_t state; char *state_str; - visit_type_str(v, name, &state_str, &local_err); - if (local_err) { + if (!visit_type_str(v, name, &state_str, &local_err)) { error_propagate(errp, local_err); return; } diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c index 58dbebc..2ae0b89 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -75,8 +75,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, const char *name, Error *local_err = NULL; int64_t temp; - visit_type_int(v, name, &temp, &local_err); - if (local_err) { + if (!visit_type_int(v, name, &temp, &local_err)) { error_propagate(errp, local_err); return; } diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c index 74864cd..9473382 100644 --- a/hw/misc/tmp421.c +++ b/hw/misc/tmp421.c @@ -147,8 +147,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, int offset = ext_range * 64 * 256; int tempid; - visit_type_int(v, name, &temp, &local_err); - if (local_err) { + if (!visit_type_int(v, name, &temp, &local_err)) { error_propagate(errp, local_err); return; } -- cgit v1.1 From 4d21fcd52404ff4bac1f94d8054dfb745f1b2ad6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:05:52 +0200 Subject: qom: Don't handle impossible object_property_get_link() failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't handle object_property_get_link() failure that can't happen unless the programmer screwed up, pass &error_abort. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20200707160613.848843-25-armbru@redhat.com> --- hw/misc/bcm2835_mbox.c | 9 +-------- hw/misc/bcm2835_property.c | 17 ++--------------- 2 files changed, 3 insertions(+), 23 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/bcm2835_mbox.c b/hw/misc/bcm2835_mbox.c index 2afa06a..9f73cbd 100644 --- a/hw/misc/bcm2835_mbox.c +++ b/hw/misc/bcm2835_mbox.c @@ -308,15 +308,8 @@ static void bcm2835_mbox_realize(DeviceState *dev, Error **errp) { BCM2835MboxState *s = BCM2835_MBOX(dev); Object *obj; - Error *err = NULL; - - obj = object_property_get_link(OBJECT(dev), "mbox-mr", &err); - if (obj == NULL) { - error_setg(errp, "%s: required mbox-mr link not found: %s", - __func__, error_get_pretty(err)); - return; - } + obj = object_property_get_link(OBJECT(dev), "mbox-mr", &error_abort); s->mbox_mr = MEMORY_REGION(obj); address_space_init(&s->mbox_as, s->mbox_mr, TYPE_BCM2835_MBOX "-memory"); bcm2835_mbox_reset(dev); diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index 3e228ca..73941bd 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -392,24 +392,11 @@ static void bcm2835_property_realize(DeviceState *dev, Error **errp) { BCM2835PropertyState *s = BCM2835_PROPERTY(dev); Object *obj; - Error *err = NULL; - - obj = object_property_get_link(OBJECT(dev), "fb", &err); - if (obj == NULL) { - error_setg(errp, "%s: required fb link not found: %s", - __func__, error_get_pretty(err)); - return; - } + obj = object_property_get_link(OBJECT(dev), "fb", &error_abort); s->fbdev = BCM2835_FB(obj); - obj = object_property_get_link(OBJECT(dev), "dma-mr", &err); - if (obj == NULL) { - error_setg(errp, "%s: required dma-mr link not found: %s", - __func__, error_get_pretty(err)); - return; - } - + obj = object_property_get_link(OBJECT(dev), "dma-mr", &error_abort); s->dma_mr = MEMORY_REGION(obj); address_space_init(&s->dma_as, s->dma_mr, TYPE_BCM2835_PROPERTY "-memory"); -- cgit v1.1 From 5325cc34a2ca985283134c7e264be7851b112d4e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:05:54 +0200 Subject: qom: Put name parameter before value / visitor parameter The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved] --- hw/misc/iotkit-sysctl.c | 2 +- hw/misc/macio/macio.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/iotkit-sysctl.c b/hw/misc/iotkit-sysctl.c index ec1cc19..2697833 100644 --- a/hw/misc/iotkit-sysctl.c +++ b/hw/misc/iotkit-sysctl.c @@ -84,7 +84,7 @@ static void set_init_vtor(uint64_t cpuid, uint32_t vtor) if (cpuobj) { if (object_property_find(cpuobj, "init-svtor", NULL)) { - object_property_set_uint(cpuobj, vtor, "init-svtor", &error_abort); + object_property_set_uint(cpuobj, "init-svtor", vtor, &error_abort); } } } diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index be66bb7..9b6e3f1 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -134,7 +134,7 @@ 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", + object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma), &error_abort); macio_ide_register_dma(ide); @@ -330,7 +330,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) if (ns->has_pmu) { /* GPIOs */ sysbus_dev = SYS_BUS_DEVICE(&ns->gpio); - object_property_set_link(OBJECT(&ns->gpio), OBJECT(pic_dev), "pic", + object_property_set_link(OBJECT(&ns->gpio), "pic", OBJECT(pic_dev), &error_abort); memory_region_add_subregion(&s->bar, 0x50, sysbus_mmio_get_region(sysbus_dev, 0)); @@ -340,7 +340,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) /* PMU */ object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU); - object_property_set_link(OBJECT(&s->pmu), OBJECT(sysbus_dev), "gpio", + object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sysbus_dev), &error_abort); qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb); if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), &err)) { -- cgit v1.1 From dcfe480544eef72d666cb1695624449e2c22da2d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:06:01 +0200 Subject: error: Avoid unnecessary error_propagate() after error_setg() Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: if (...) { error_setg(&err, ...); goto out; } ... out: error_propagate(errp, err); return; When all paths to label out are that way, replace by if (...) { error_setg(errp, ...); return; } and delete the label along with the error_propagate(). When we have at most one other path that actually needs to propagate, and maybe one at the end that where propagation is unnecessary, e.g. foo(..., &err); if (err) { goto out; } ... bar(..., &err); out: error_propagate(errp, err); return; move the error_propagate() to where it's needed, like if (...) { foo(..., &err); error_propagate(errp, err); return; } ... bar(..., errp); return; and transform the error_setg() as above. In some places, the transformation results in obviously unnecessary error_propagate(). The next few commits will eliminate them. Bonus: the elimination of gotos will make later patches in this series easier to review. Candidates for conversion tracked down with this Coccinelle script: @@ identifier err, errp; expression list args; @@ - error_setg(&err, args); + error_setg(errp, args); ... when != err error_propagate(errp, err); Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200707160613.848843-34-armbru@redhat.com> --- hw/misc/aspeed_sdmc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 2887444..40682af 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -240,9 +240,8 @@ static void aspeed_sdmc_set_ram_size(Object *obj, Visitor *v, const char *name, } sz = size_to_str(value); - error_setg(&local_err, "Invalid RAM size %s", sz); + error_setg(errp, "Invalid RAM size %s", sz); g_free(sz); - error_propagate(errp, local_err); } static void aspeed_sdmc_initfn(Object *obj) -- cgit v1.1 From 668f62ec621e4e2919fb7d4caa5d805764c5852d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:06:02 +0200 Subject: error: Eliminate error_propagate() with Coccinelle, part 1 When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200707160613.848843-35-armbru@redhat.com> --- hw/misc/aspeed_sdmc.c | 4 +--- hw/misc/macio/cuda.c | 4 +--- hw/misc/macio/macio.c | 19 ++++++------------- hw/misc/macio/pmu.c | 4 +--- hw/misc/pca9552.c | 4 +--- hw/misc/tmp105.c | 4 +--- hw/misc/tmp421.c | 4 +--- 7 files changed, 12 insertions(+), 31 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 40682af..0737d8d 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -223,12 +223,10 @@ static void aspeed_sdmc_set_ram_size(Object *obj, Visitor *v, const char *name, int i; char *sz; int64_t value; - Error *local_err = NULL; AspeedSDMCState *s = ASPEED_SDMC(obj); AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s); - if (!visit_type_int(v, name, &value, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &value, errp)) { return; } diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index cc7c26d..286e7a5 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -521,13 +521,11 @@ static void cuda_reset(DeviceState *dev) static void cuda_realize(DeviceState *dev, Error **errp) { CUDAState *s = CUDA(dev); - Error *err = NULL; SysBusDevice *sbd; ADBBusState *adb_bus = &s->adb_bus; struct tm tm; - if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_cuda), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_cuda), errp)) { return; } diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index 9b6e3f1..6797226 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -98,10 +98,8 @@ static void macio_common_realize(PCIDevice *d, Error **errp) { MacIOState *s = MACIO(d); SysBusDevice *sysbus_dev; - Error *err = NULL; - if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) { return; } sysbus_dev = SYS_BUS_DEVICE(&s->dbdma); @@ -115,8 +113,7 @@ static void macio_common_realize(PCIDevice *d, Error **errp) qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1)); qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial); qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial); - if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) { return; } @@ -157,8 +154,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp) qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency", s->frequency); - if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) { return; } sysbus_dev = SYS_BUS_DEVICE(&s->cuda); @@ -173,8 +169,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp) sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCA_IRQ)); - if (!qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), errp)) { return; } sysbus_dev = SYS_BUS_DEVICE(&os->nvram); @@ -343,8 +338,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sysbus_dev), &error_abort); qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb); - if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), errp)) { return; } sysbus_dev = SYS_BUS_DEVICE(&s->pmu); @@ -360,8 +354,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp) qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency", s->frequency); - if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), &err)) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) { return; } sysbus_dev = SYS_BUS_DEVICE(&s->cuda); diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index 42ba963..0902299 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -735,13 +735,11 @@ static void pmu_reset(DeviceState *dev) static void pmu_realize(DeviceState *dev, Error **errp) { PMUState *s = VIA_PMU(dev); - Error *err = NULL; SysBusDevice *sbd; ADBBusState *adb_bus = &s->adb_bus; struct tm tm; - if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_pmu), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->mos6522_pmu), errp)) { return; } diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c index d2b99fc..e4ccdea 100644 --- a/hw/misc/pca9552.c +++ b/hw/misc/pca9552.c @@ -291,13 +291,11 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name, { PCA955xClass *k = PCA955X_GET_CLASS(obj); PCA955xState *s = PCA955X(obj); - Error *local_err = NULL; int led, rc, reg, val; uint8_t state; char *state_str; - if (!visit_type_str(v, name, &state_str, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_str(v, name, &state_str, errp)) { return; } rc = sscanf(name, "led%2d", &led); diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c index 2ae0b89..b471204 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -72,11 +72,9 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { TMP105State *s = TMP105(obj); - Error *local_err = NULL; int64_t temp; - if (!visit_type_int(v, name, &temp, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &temp, errp)) { return; } if (temp >= 128000 || temp < -128000) { diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c index 9473382..49abe2d 100644 --- a/hw/misc/tmp421.c +++ b/hw/misc/tmp421.c @@ -141,14 +141,12 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { TMP421State *s = TMP421(obj); - Error *local_err = NULL; int64_t temp; bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); int offset = ext_range * 64 * 256; int tempid; - if (!visit_type_int(v, name, &temp, &local_err)) { - error_propagate(errp, local_err); + if (!visit_type_int(v, name, &temp, errp)) { return; } -- cgit v1.1 From 386f6c07d28cf1b51e5d0f398faf67d396ff62a1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 7 Jul 2020 18:06:10 +0200 Subject: error: Avoid error_propagate() after migrate_add_blocker() When migrate_add_blocker(blocker, &errp) is followed by error_propagate(errp, err), we can often just as well do migrate_add_blocker(..., errp). Do that with this Coccinelle script: @@ expression blocker, err, errp; expression ret; @@ - ret = migrate_add_blocker(blocker, &err); - if (err) { + ret = migrate_add_blocker(blocker, errp); + if (ret < 0) { ... when != err; - error_propagate(errp, err); ... } @@ expression blocker, err, errp; @@ - migrate_add_blocker(blocker, &err); - if (err) { + if (migrate_add_blocker(blocker, errp) < 0) { ... when != err; - error_propagate(errp, err); ... } Double-check @err is not used afterwards. Dereferencing it would be use after free, but checking whether it's null would be legitimate. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200707160613.848843-43-armbru@redhat.com> --- hw/misc/ivshmem.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'hw/misc') diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index a8dc9b3..fc128b2 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -898,9 +898,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp) if (!ivshmem_is_master(s)) { error_setg(&s->migration_blocker, "Migration is disabled when using feature 'peer mode' in device 'ivshmem'"); - migrate_add_blocker(s->migration_blocker, &err); - if (err) { - error_propagate(errp, err); + if (migrate_add_blocker(s->migration_blocker, errp) < 0) { error_free(s->migration_blocker); return; } -- cgit v1.1