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/mips/cps.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'hw/mips') diff --git a/hw/mips/cps.c b/hw/mips/cps.c index 0d7f3cf..22b9328 100644 --- a/hw/mips/cps.c +++ b/hw/mips/cps.c @@ -109,8 +109,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) if (saar_present) { s->itu.saar = &env->CP0_SAAR; } - sysbus_realize(SYS_BUS_DEVICE(&s->itu), &err); - if (err != NULL) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->itu), &err)) { error_propagate(errp, err); return; } @@ -125,8 +124,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running", &error_abort); - sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err); - if (err != NULL) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err)) { error_propagate(errp, err); return; } @@ -140,8 +138,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_int(OBJECT(&s->gic), 128, "num-irq", &error_abort); - sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err); - if (err != NULL) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err)) { error_propagate(errp, err); return; } @@ -163,8 +160,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc", &error_abort); - sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err); - if (err != NULL) { + if (!sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err)) { error_propagate(errp, err); return; } -- 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/mips/boston.c | 4 ++-- hw/mips/cps.c | 24 ++++++++++++------------ hw/mips/jazz.c | 4 ++-- hw/mips/malta.c | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) (limited to 'hw/mips') diff --git a/hw/mips/boston.c b/hw/mips/boston.c index f5d4ac8..766458c 100644 --- a/hw/mips/boston.c +++ b/hw/mips/boston.c @@ -455,9 +455,9 @@ static void boston_mach_init(MachineState *machine) is_64b = cpu_supports_isa(machine->cpu_type, ISA_MIPS64); object_initialize_child(OBJECT(machine), "cps", &s->cps, TYPE_MIPS_CPS); - object_property_set_str(OBJECT(&s->cps), machine->cpu_type, "cpu-type", + object_property_set_str(OBJECT(&s->cps), "cpu-type", machine->cpu_type, &error_fatal); - object_property_set_int(OBJECT(&s->cps), machine->smp.cpus, "num-vp", + object_property_set_int(OBJECT(&s->cps), "num-vp", machine->smp.cpus, &error_fatal); sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal); diff --git a/hw/mips/cps.c b/hw/mips/cps.c index 22b9328..83a073f 100644 --- a/hw/mips/cps.c +++ b/hw/mips/cps.c @@ -100,11 +100,11 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) /* Inter-Thread Communication Unit */ if (itu_present) { object_initialize_child(OBJECT(dev), "itu", &s->itu, TYPE_MIPS_ITU); - object_property_set_int(OBJECT(&s->itu), 16, "num-fifo", + object_property_set_int(OBJECT(&s->itu), "num-fifo", 16, &error_abort); - object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores", + object_property_set_int(OBJECT(&s->itu), "num-semaphores", 16, &error_abort); - object_property_set_bool(OBJECT(&s->itu), saar_present, "saar-present", + object_property_set_bool(OBJECT(&s->itu), "saar-present", saar_present, &error_abort); if (saar_present) { s->itu.saar = &env->CP0_SAAR; @@ -120,9 +120,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) /* Cluster Power Controller */ object_initialize_child(OBJECT(dev), "cpc", &s->cpc, TYPE_MIPS_CPC); - object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp", + object_property_set_int(OBJECT(&s->cpc), "num-vp", s->num_vp, &error_abort); - object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running", + object_property_set_int(OBJECT(&s->cpc), "vp-start-running", 1, &error_abort); if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err)) { error_propagate(errp, err); @@ -134,9 +134,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) /* Global Interrupt Controller */ object_initialize_child(OBJECT(dev), "gic", &s->gic, TYPE_MIPS_GIC); - object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp", + object_property_set_int(OBJECT(&s->gic), "num-vp", s->num_vp, &error_abort); - object_property_set_int(OBJECT(&s->gic), 128, "num-irq", + object_property_set_int(OBJECT(&s->gic), "num-irq", 128, &error_abort); if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err)) { error_propagate(errp, err); @@ -150,15 +150,15 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) gcr_base = env->CP0_CMGCRBase << 4; object_initialize_child(OBJECT(dev), "gcr", &s->gcr, TYPE_MIPS_GCR); - object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", + object_property_set_int(OBJECT(&s->gcr), "num-vp", s->num_vp, &error_abort); - object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", + object_property_set_int(OBJECT(&s->gcr), "gcr-rev", 0x800, &error_abort); - object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", + object_property_set_int(OBJECT(&s->gcr), "gcr-base", gcr_base, &error_abort); - object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic", + object_property_set_link(OBJECT(&s->gcr), "gic", OBJECT(&s->gic.mr), &error_abort); - object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc", + object_property_set_link(OBJECT(&s->gcr), "cpc", OBJECT(&s->cpc.mr), &error_abort); if (!sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err)) { error_propagate(errp, err); diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c index 0002bff..82a6e32 100644 --- a/hw/mips/jazz.c +++ b/hw/mips/jazz.c @@ -290,8 +290,8 @@ static void mips_jazz_init(MachineState *machine, dev = qdev_new("dp8393x"); qdev_set_nic_properties(dev, nd); qdev_prop_set_uint8(dev, "it_shift", 2); - object_property_set_link(OBJECT(dev), OBJECT(rc4030_dma_mr), - "dma_mr", &error_abort); + object_property_set_link(OBJECT(dev), "dma_mr", + OBJECT(rc4030_dma_mr), &error_abort); sysbus = SYS_BUS_DEVICE(dev); sysbus_realize_and_unref(sysbus, &error_fatal); sysbus_mmio_map(sysbus, 0, 0x80001000); diff --git a/hw/mips/malta.c b/hw/mips/malta.c index d95926a..a59e20c 100644 --- a/hw/mips/malta.c +++ b/hw/mips/malta.c @@ -1184,9 +1184,9 @@ static void create_cps(MachineState *ms, MaltaState *s, qemu_irq *cbus_irq, qemu_irq *i8259_irq) { object_initialize_child(OBJECT(s), "cps", &s->cps, TYPE_MIPS_CPS); - object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", + object_property_set_str(OBJECT(&s->cps), "cpu-type", ms->cpu_type, &error_fatal); - object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", + object_property_set_int(OBJECT(&s->cps), "num-vp", ms->smp.cpus, &error_fatal); sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal); -- 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/mips/cps.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'hw/mips') diff --git a/hw/mips/cps.c b/hw/mips/cps.c index 83a073f..615e1a1 100644 --- a/hw/mips/cps.c +++ b/hw/mips/cps.c @@ -71,7 +71,6 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) CPUMIPSState *env; MIPSCPU *cpu; int i; - Error *err = NULL; target_ulong gcr_base; bool itu_present = false; bool saar_present = false; @@ -109,8 +108,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) if (saar_present) { s->itu.saar = &env->CP0_SAAR; } - if (!sysbus_realize(SYS_BUS_DEVICE(&s->itu), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->itu), errp)) { return; } @@ -124,8 +122,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_int(OBJECT(&s->cpc), "vp-start-running", 1, &error_abort); - if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpc), errp)) { return; } @@ -138,8 +135,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_int(OBJECT(&s->gic), "num-irq", 128, &error_abort); - if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) { return; } @@ -160,8 +156,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_link(OBJECT(&s->gcr), "cpc", OBJECT(&s->cpc.mr), &error_abort); - if (!sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err)) { - error_propagate(errp, err); + if (!sysbus_realize(SYS_BUS_DEVICE(&s->gcr), errp)) { return; } -- cgit v1.1