Message ID | 20200406174743.16956-54-f4bug@amsat.org |
---|---|
State | New |
Headers | show |
Series | various: Fix error-propagation with Coccinelle scripts | expand |
On 4/6/20 7:47 PM, Philippe Mathieu-Daudé wrote: > Running the coccinelle script produced: > > $ spatch \ > --macro-file scripts/cocci-macro-file.h --include-headers \ > --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \ > --keep-comments --smpl-spacing --dir . > HANDLING: ./hw/mips/mips_malta.c > [[manual check required: error_propagate() might be missing in object_property_set_int() ./hw/mips/mips_malta.c:1193:4]] > [[manual check required: error_propagate() might be missing in object_property_set_str() ./hw/mips/mips_malta.c:1192:4]] > > Add the missing error_propagate() after review by adding > a Error* parameter to create_cps(). > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/mips/mips_malta.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index e4c4de1b4e..8d43cfd41b 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -1183,18 +1183,27 @@ static void create_cpu_without_cps(MachineState *ms, > } > > static void create_cps(MachineState *ms, MaltaState *s, > - qemu_irq *cbus_irq, qemu_irq *i8259_irq) > + qemu_irq *cbus_irq, qemu_irq *i8259_irq, > + Error **errp) > { > Error *err = NULL; > > sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps), > TYPE_MIPS_CPS); > object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_bool(OBJECT(&s->cps), true, "realized", &err); > - if (err != NULL) { > - error_report("%s", error_get_pretty(err)); > - exit(1); > + if (err) { > + error_propagate(errp, err); > + return; > } > > sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); > @@ -1207,7 +1216,7 @@ static void mips_create_cpu(MachineState *ms, MaltaState *s, > qemu_irq *cbus_irq, qemu_irq *i8259_irq) > { > if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(ms->cpu_type)) { > - create_cps(ms, s, cbus_irq, i8259_irq); > + create_cps(ms, s, cbus_irq, i8259_irq, &error_fatal); > } else { > create_cpu_without_cps(ms, cbus_irq, i8259_irq); > } > This patch also requires: -- >8 -- @@ -1241,7 +1241,7 @@ void mips_malta_init(MachineState *machine) int64_t kernel_entry, bootloader_run_addr; PCIBus *pci_bus; ISABus *isa_bus; - qemu_irq cbus_irq, i8259_irq; + qemu_irq cbus_irq, i8259_irq = NULL; I2CBus *smbus; DriveInfo *dinfo; int fl_idx = 0; --- else it fails building with gcc -O3: hw/mips/mips_malta.c: In function ‘mips_malta_init’: hw/mips/mips_malta.c:1419:5: warning: ‘i8259_irq’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1419 | qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
On 4/6/20 10:47 AM, Philippe Mathieu-Daudé wrote: > Running the coccinelle script produced: > > $ spatch \ > --macro-file scripts/cocci-macro-file.h --include-headers \ > --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \ > --keep-comments --smpl-spacing --dir . > HANDLING: ./hw/mips/mips_malta.c > [[manual check required: error_propagate() might be missing in object_property_set_int() ./hw/mips/mips_malta.c:1193:4]] > [[manual check required: error_propagate() might be missing in object_property_set_str() ./hw/mips/mips_malta.c:1192:4]] > > Add the missing error_propagate() after review by adding > a Error* parameter to create_cps(). > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/mips/mips_malta.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index e4c4de1b4e..8d43cfd41b 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -1183,18 +1183,27 @@ static void create_cpu_without_cps(MachineState *ms, > } > > static void create_cps(MachineState *ms, MaltaState *s, > - qemu_irq *cbus_irq, qemu_irq *i8259_irq) > + qemu_irq *cbus_irq, qemu_irq *i8259_irq, > + Error **errp) > { > Error *err = NULL; > > sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps), > TYPE_MIPS_CPS); > object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_bool(OBJECT(&s->cps), true, "realized", &err); > - if (err != NULL) { > - error_report("%s", error_get_pretty(err)); > - exit(1); > + if (err) { > + error_propagate(errp, err); > + return; > } > > sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); > @@ -1207,7 +1216,7 @@ static void mips_create_cpu(MachineState *ms, MaltaState *s, > qemu_irq *cbus_irq, qemu_irq *i8259_irq) > { > if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(ms->cpu_type)) { > - create_cps(ms, s, cbus_irq, i8259_irq); > + create_cps(ms, s, cbus_irq, i8259_irq, &error_fatal); Are you going to replace this with something else later? Otherwise it doesn't seem useful to add the argument here, as opposed to using error_fatal in create_cpus() r~
On 4/7/20 8:36 PM, Richard Henderson wrote: > On 4/6/20 10:47 AM, Philippe Mathieu-Daudé wrote: >> Running the coccinelle script produced: >> >> $ spatch \ >> --macro-file scripts/cocci-macro-file.h --include-headers \ >> --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \ >> --keep-comments --smpl-spacing --dir . >> HANDLING: ./hw/mips/mips_malta.c >> [[manual check required: error_propagate() might be missing in object_property_set_int() ./hw/mips/mips_malta.c:1193:4]] >> [[manual check required: error_propagate() might be missing in object_property_set_str() ./hw/mips/mips_malta.c:1192:4]] >> >> Add the missing error_propagate() after review by adding >> a Error* parameter to create_cps(). >> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/mips/mips_malta.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c >> index e4c4de1b4e..8d43cfd41b 100644 >> --- a/hw/mips/mips_malta.c >> +++ b/hw/mips/mips_malta.c >> @@ -1183,18 +1183,27 @@ static void create_cpu_without_cps(MachineState *ms, >> } >> >> static void create_cps(MachineState *ms, MaltaState *s, >> - qemu_irq *cbus_irq, qemu_irq *i8259_irq) >> + qemu_irq *cbus_irq, qemu_irq *i8259_irq, >> + Error **errp) >> { >> Error *err = NULL; >> >> sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps), >> TYPE_MIPS_CPS); >> object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> object_property_set_bool(OBJECT(&s->cps), true, "realized", &err); >> - if (err != NULL) { >> - error_report("%s", error_get_pretty(err)); >> - exit(1); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> } >> >> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); >> @@ -1207,7 +1216,7 @@ static void mips_create_cpu(MachineState *ms, MaltaState *s, >> qemu_irq *cbus_irq, qemu_irq *i8259_irq) >> { >> if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(ms->cpu_type)) { >> - create_cps(ms, s, cbus_irq, i8259_irq); >> + create_cps(ms, s, cbus_irq, i8259_irq, &error_fatal); > > Are you going to replace this with something else later? No, indeed. > Otherwise it doesn't seem useful to add the argument here, as opposed to using > error_fatal in create_cpus() Good suggestion, thanks!
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index e4c4de1b4e..8d43cfd41b 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1183,18 +1183,27 @@ static void create_cpu_without_cps(MachineState *ms, } static void create_cps(MachineState *ms, MaltaState *s, - qemu_irq *cbus_irq, qemu_irq *i8259_irq) + qemu_irq *cbus_irq, qemu_irq *i8259_irq, + Error **errp) { Error *err = NULL; sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps), TYPE_MIPS_CPS); object_property_set_str(OBJECT(&s->cps), ms->cpu_type, "cpu-type", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_int(OBJECT(&s->cps), ms->smp.cpus, "num-vp", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->cps), true, "realized", &err); - if (err != NULL) { - error_report("%s", error_get_pretty(err)); - exit(1); + if (err) { + error_propagate(errp, err); + return; } sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); @@ -1207,7 +1216,7 @@ static void mips_create_cpu(MachineState *ms, MaltaState *s, qemu_irq *cbus_irq, qemu_irq *i8259_irq) { if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(ms->cpu_type)) { - create_cps(ms, s, cbus_irq, i8259_irq); + create_cps(ms, s, cbus_irq, i8259_irq, &error_fatal); } else { create_cpu_without_cps(ms, cbus_irq, i8259_irq); }