Re: [PATCH v2 03/44] qdev: Use returned bool to check for qdev_realize() etc. failure
02.07.2020 18:49, Markus Armbruster wrote: 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 Sorry me, reviewing this patch with help of script: #!/usr/bin/env python3 import sys import re with open(sys.argv[1]) as f: patch = f.read() regex = re.compile(r'^- *(?P(?P\w+)\(.*, &(?P\w+)\));\n' r'^- *if \((?P=err)( != NULL)?\) \{\n' r'^\+ *if \(!(?P=func_call)\) \{$', flags=re.MULTILINE) for chunk in re.split('^@', patch, flags=re.MULTILINE): filtered = regex.sub('OK', chunk) if re.search('^[+-][^+-]', filtered, flags=re.MULTILINE): print(re.sub('^', ' ', '@' + chunk, flags=re.MULTILINE)) funcs = set() for m in regex.finditer(patch): funcs.add(m.group('func')) print() for func in funcs: print(func) output: @@ -34,9 +34,7 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp) Error *local_error = NULL; virtio_pci_force_virtio_1(vpci_dev); -qdev_realize(vdev, BUS(&vpci_dev->bus), &local_error); - -if (local_error) { +if (!qdev_realize(vdev, BUS(&vpci_dev->bus), &local_error)) { error_propagate(errp, local_error); return; } diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index 67f409e106..0fc00fee1f 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -444,15 +444,13 @@ static void realize_event_facility(DeviceState *dev, Error **errp) SCLPEventFacility *event_facility = EVENT_FACILITY(dev); Error *local_err = NULL; -qdev_realize(DEVICE(&event_facility->quiesce), - BUS(&event_facility->sbus), &local_err); -if (local_err) { +if (!qdev_realize(DEVICE(&event_facility->quiesce), + BUS(&event_facility->sbus), &local_err)) { error_propagate(errp, local_err); return; } -qdev_realize(DEVICE(&event_facility->cpu_hotplug), - BUS(&event_facility->sbus), &local_err); -if (local_err) { +if (!qdev_realize(DEVICE(&event_facility->cpu_hotplug), + BUS(&event_facility->sbus), &local_err)) { error_propagate(errp, local_err); qdev_unrealize(DEVICE(&event_facility->quiesce)); return; diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 142e52a8ff..0517901024 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c usb_realize_and_unref sysbus_realize sysbus_realize_and_unref qdev_realize qdev_realize_and_unref === So, the remaning non-matching seems correct, and all found functions seems to have corresponding semantics: Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH v2 03/44] qdev: Use returned bool to check for qdev_realize() etc. failure
On 7/2/20 10:49 AM, Markus Armbruster wrote: 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 --- The conversion is quite straight-forward. The patch is big, but correct, and you documented where it is not mechanical. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH v2 03/44] 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 --- hw/arm/allwinner-a10.c | 15 +++ hw/arm/armsse.c | 78 +++-- hw/arm/armv7m.c | 9 ++-- hw/arm/aspeed_ast2600.c | 51 +++-- hw/arm/aspeed_soc.c | 45 +++ hw/arm/bcm2835_peripherals.c| 45 +++ hw/arm/bcm2836.c| 9 ++-- hw/arm/cubieboard.c | 3 +- hw/arm/digic.c | 9 ++-- hw/arm/digic_boards.c | 3 +- hw/arm/fsl-imx25.c | 33 +- hw/arm/fsl-imx31.c | 24 -- hw/arm/fsl-imx6.c | 36 +-- hw/arm/msf2-soc.c | 15 +++ hw/arm/nrf51_soc.c | 18 +++- hw/arm/stm32f205_soc.c | 21 +++-- hw/arm/stm32f405_soc.c | 24 -- hw/arm/xlnx-zynqmp.c| 45 +++ hw/block/fdc.c | 3 +- hw/block/xen-block.c| 3 +- hw/char/serial-pci-multi.c | 3 +- hw/char/serial-pci.c| 3 +- hw/char/serial.c| 6 +-- hw/core/cpu.c | 3 +- hw/cpu/a15mpcore.c | 3 +- hw/cpu/a9mpcore.c | 15 +++ hw/cpu/arm11mpcore.c| 12 ++--- hw/cpu/realview_mpcore.c| 6 +-- hw/display/virtio-gpu-pci.c | 4 +- hw/display/virtio-vga.c | 3 +- hw/intc/armv7m_nvic.c | 6 +-- hw/intc/pnv_xive.c | 6 +-- hw/intc/realview_gic.c | 3 +- hw/intc/spapr_xive.c| 6 +-- hw/intc/xics.c | 3 +- hw/intc/xive.c | 3 +- hw/isa/piix4.c | 3 +- hw/microblaze/xlnx-zynqmp-pmu.c | 6 +-- hw/mips/cps.c | 12 ++--- hw/misc/macio/cuda.c| 3 +- hw/misc/macio/macio.c | 18 +++- hw/misc/macio/pmu.c | 3 +- hw/pci-host/pnv_phb3.c | 9 ++-- hw/pci-host/pnv_phb4.c | 3 +- hw/pci-host/pnv_phb4_pec.c | 3 +- hw/ppc/e500.c | 3 +- hw/ppc/pnv.c| 39 ++--- hw/ppc/pnv_core.c | 3 +- hw/ppc/pnv_psi.c| 6 +-- hw/ppc/spapr_cpu_core.c | 3 +- hw/ppc/spapr_irq.c | 3 +- hw/riscv/opentitan.c| 6 +-- hw/riscv/sifive_e.c | 3 +- hw/riscv/sifive_u.c | 3 +- hw/s390x/event-facility.c | 10 ++--- hw/s390x/s390-pci-bus.c | 3 +- hw/s390x/sclp.c | 3 +- hw/s390x/virtio-ccw-crypto.c| 3 +- hw/s390x/virtio-ccw-rng.c | 3 +- hw/scsi/scsi-bus.c | 3 +- hw/sd/aspeed_sdhci.c| 3 +- hw/sd/ssi-sd.c | 3 +- hw/usb/bus.c| 3 +- hw/virtio/virtio-rng-pci.c | 3 +- qdev-monitor.c | 3 +- 65 files changed, 248 insertions(+), 495 deletions(-) diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c index 52e0d83760..e1acffe5f6 100644 --- a/hw/arm/allwinner-a10.c +++ b/hw/arm/allwinner-a10.c @@ -74,14 +74,12 @@ static void aw_a10_realize(DeviceState *dev, Error **errp) SysBusDevice *sysbusdev; Error *err = NULL; -qdev_realize(DEVICE(&s->cpu), NULL, &err); -if (err != NULL) { +if (!qdev_realize(DEVICE(&s->cpu), NULL, &err)) { error_propagate(errp, err); return; } -sysbus_realize(SYS_BUS_DEVICE(&s->intc), &err); -if (err != NULL) { +if (!sysbus_realize(SYS_BUS_DEVICE(&s->intc), &err)) { error_propagate(errp, err); return; } @@ -93,8 +91,7 @@ static void aw_a10_realize(DeviceState *dev, Error **errp) qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ)); qdev_pass_gpios(DEVICE(&s->intc), dev, NULL); -sysbus_realize(SYS_BUS_DEVICE(&s->timer), &err); -if (err != NULL