diff mbox series

[PATCH-for-5.1,v2,53/54] hw/mips/mips_malta: Add missing error-propagation code

Message ID 20200406174743.16956-54-f4bug@amsat.org
State New
Headers show
Series various: Fix error-propagation with Coccinelle scripts | expand

Commit Message

Philippe Mathieu-Daudé April 6, 2020, 5:47 p.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé April 6, 2020, 7:27 p.m. UTC | #1 
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);
           |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Richard Henderson April 7, 2020, 6:36 p.m. UTC | #2 
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~
Philippe Mathieu-Daudé April 12, 2020, 10:28 p.m. UTC | #3 
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 mbox series

Patch

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);
     }