Greg Kurz <[email protected]> writes:
> On Mon, 6 Jul 2020 10:09:09 +0200
> Markus Armbruster <[email protected]> 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 <[email protected]>
>> Reviewed-by: Eric Blake <[email protected]>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
>> ---
>
> FWIW I had posted an R-b for this patch in v1
> ([email protected]).
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?