Greg Kurz <gr...@kaod.org> writes: > On Mon, 6 Jul 2020 10:09:09 +0200 > Markus Armbruster <arm...@redhat.com> 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 <arm...@redhat.com> >> Reviewed-by: Eric Blake <ebl...@redhat.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- > > FWIW I had posted an R-b for this patch in v1 > (20200629124037.2b9a2...@bahia.lan).
When I sliced and diced my patches for v2, I dropped R-bys for patches substantially altered. This one was borderline: the patch does strictly less, and the work it no longer does us done by later patches. Example: v1's first hunk diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c index 52e0d83760..3e45aa4141 100644 --- a/hw/arm/allwinner-a10.c +++ b/hw/arm/allwinner-a10.c @@ -72,17 +72,12 @@ static void aw_a10_realize(DeviceState *dev, Error **errp) { AwA10State *s = AW_A10(dev); SysBusDevice *sysbusdev; - Error *err = NULL; - qdev_realize(DEVICE(&s->cpu), NULL, &err); - if (err != NULL) { - error_propagate(errp, err); + if (!qdev_realize(DEVICE(&s->cpu), NULL, errp)) { return; } - sysbus_realize(SYS_BUS_DEVICE(&s->intc), &err); - if (err != NULL) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->intc), errp)) { return; } sysbusdev = SYS_BUS_DEVICE(&s->intc); became 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; } in v2 and v3. The two error_propagate() and the local variable now go away only in PATCH v3 33. Would you like me to record your R-by for the patch's current version?