[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 03/44] qdev: Use returned bool to check for qdev_realize()
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 03/44] qdev: Use returned bool to check for qdev_realize() etc. failure |
Date: |
Mon, 06 Jul 2020 13:35:19 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Greg Kurz <groug@kaod.org> writes:
> On Mon, 6 Jul 2020 10:09:09 +0200
> Markus Armbruster <armbru@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 <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>
> FWIW I had posted an R-b for this patch in v1
> (20200629124037.2b9a269e@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?
[PATCH v3 18/44] qapi: Use returned bool to check for failure, manual part, Markus Armbruster, 2020/07/06
[PATCH v3 30/44] qdev: Make functions taking Error ** return bool, not void, Markus Armbruster, 2020/07/06
[PATCH v3 13/44] qemu-option: Use returned bool to check for failure, Markus Armbruster, 2020/07/06
[PATCH v3 17/44] qapi: Use returned bool to check for failure, Coccinelle part, Markus Armbruster, 2020/07/06
[PATCH v3 40/44] qapi: Purge error_propagate() from QAPI core, Markus Armbruster, 2020/07/06
[PATCH v3 38/44] qapi: Smooth another visitor error checking pattern, Markus Armbruster, 2020/07/06
[PATCH v3 31/44] qdev: Use returned bool to check for failure, Coccinelle part, Markus Armbruster, 2020/07/06
[PATCH v3 29/44] qom: Make functions taking Error ** return bool, not 0/-1, Markus Armbruster, 2020/07/06