Re: [PATCH-for-5.0 07/12] hw/mips/cps: Add missing error-propagation code

2020-03-26 Thread Aleksandar Markovic 
21:18 Sre, 25.03.2020. Philippe Mathieu-Daudé  је
написао/ла:
>
> Patch created mechanically by running:
>
>   $ spatch \
> --macro-file scripts/cocci-macro-file.h --include-headers \
> --sp-file
scripts/coccinelle/object_property_missing_error_propagate.cocci \
> --keep-comments --smpl-spacing --in-place --dir hw
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/mips/cps.c | 52 +++
>  1 file changed, 52 insertions(+)
>
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index 92b9b1a5f6..d682633401 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -68,100 +68,152 @@ static bool cpu_mips_itu_supported(CPUMIPSState
*env)
>  static void mips_cps_realize(DeviceState *dev, Error **errp)
>  {
>  MIPSCPSState *s = MIPS_CPS(dev);
>  CPUMIPSState *env;
>  MIPSCPU *cpu;
>  int i;
>  Error *err = NULL;
>  target_ulong gcr_base;
>  bool itu_present = false;
>  bool saar_present = false;
>
>  for (i = 0; i < s->num_vp; i++) {
>  cpu = MIPS_CPU(cpu_create(s->cpu_type));
>
>  /* Init internal devices */
>  cpu_mips_irq_init_cpu(cpu);
>  cpu_mips_clock_init(cpu);
>
>  env = >env;
>  if (cpu_mips_itu_supported(env)) {
>  itu_present = true;
>  /* Attach ITC Tag to the VP */
>  env->itc_tag = mips_itu_get_tag_region(>itu);
>  env->itu = >itu;
>  }
>  qemu_register_reset(main_cpu_reset, cpu);
>  }
>
>  cpu = MIPS_CPU(first_cpu);
>  env = >env;
>  saar_present = (bool)env->saarp;
>
>  /* Inter-Thread Communication Unit */
>  if (itu_present) {
>  sysbus_init_child_obj(OBJECT(dev), "itu", >itu,
sizeof(s->itu),
>TYPE_MIPS_ITU);
>  object_property_set_int(OBJECT(>itu), 16, "num-fifo", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>itu), 16, "num-semaphores",
);
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_bool(OBJECT(>itu), saar_present,
"saar-present",
>   );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  if (saar_present) {
>  s->itu.saar = >CP0_SAAR;
>  }
>  object_property_set_bool(OBJECT(>itu), true, "realized",
);
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  if (err != NULL) {
>  error_propagate(errp, err);
>  return;
>  }
>
>  memory_region_add_subregion(>container, 0,
>
sysbus_mmio_get_region(SYS_BUS_DEVICE(>itu), 0));
>  }
>
>  /* Cluster Power Controller */
>  sysbus_init_child_obj(OBJECT(dev), "cpc", >cpc, sizeof(s->cpc),
>TYPE_MIPS_CPC);
>  object_property_set_int(OBJECT(>cpc), s->num_vp, "num-vp", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>cpc), 1, "vp-start-running",
);
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_bool(OBJECT(>cpc), true, "realized", );
>  if (err != NULL) {
>  error_propagate(errp, err);
>  return;
>  }
>
>  memory_region_add_subregion(>container, 0,
>
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>cpc), 0));
>
>  /* Global Interrupt Controller */
>  sysbus_init_child_obj(OBJECT(dev), "gic", >gic, sizeof(s->gic),
>TYPE_MIPS_GIC);
>  object_property_set_int(OBJECT(>gic), s->num_vp, "num-vp", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>gic), 128, "num-irq", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_bool(OBJECT(>gic), true, "realized", );
>  if (err != NULL) {
>  error_propagate(errp, err);
>  return;
>  }
>
>  memory_region_add_subregion(>container, 0,
>
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>gic), 0));
>
>  /* Global Configuration Registers */
>  gcr_base = env->CP0_CMGCRBase << 4;
>
>  sysbus_init_child_obj(OBJECT(dev), "gcr", >gcr, sizeof(s->gcr),
>TYPE_MIPS_GCR);
>  object_property_set_int(OBJECT(>gcr), s->num_vp, "num-vp", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>gcr), 0x800, "gcr-rev", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>gcr), gcr_base, "gcr-base", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>

Re: [PATCH-for-5.0 07/12] hw/mips/cps: Add missing error-propagation code

2020-03-26 Thread Peter Maydell 
On Wed, 25 Mar 2020 at 19:18, Philippe Mathieu-Daudé  wrote:
>
> Patch created mechanically by running:
>
>   $ spatch \
> --macro-file scripts/cocci-macro-file.h --include-headers \
> --sp-file 
> scripts/coccinelle/object_property_missing_error_propagate.cocci \
> --keep-comments --smpl-spacing --in-place --dir hw
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/mips/cps.c | 52 +++
>  1 file changed, 52 insertions(+)
>

>  /* Inter-Thread Communication Unit */
>  if (itu_present) {
>  sysbus_init_child_obj(OBJECT(dev), "itu", >itu, sizeof(s->itu),
>TYPE_MIPS_ITU);
>  object_property_set_int(OBJECT(>itu), 16, "num-fifo", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>itu), 16, "num-semaphores", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_bool(OBJECT(>itu), saar_present, 
> "saar-present",
>   );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  if (saar_present) {
>  s->itu.saar = >CP0_SAAR;
>  }
>  object_property_set_bool(OBJECT(>itu), true, "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  if (err != NULL) {
>  error_propagate(errp, err);
>  return;
>  }

I think Coccinelle has been fooled here by the slightly non-idiomatic
use of "err != NULL" in the guard and has inserted a duplicate
check...

>  memory_region_add_subregion(>container, 0,
> sysbus_mmio_get_region(SYS_BUS_DEVICE(>itu), 
> 0));
>  }
>
>  /* Cluster Power Controller */
>  sysbus_init_child_obj(OBJECT(dev), "cpc", >cpc, sizeof(s->cpc),
>TYPE_MIPS_CPC);
>  object_property_set_int(OBJECT(>cpc), s->num_vp, "num-vp", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_int(OBJECT(>cpc), 1, "vp-start-running", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
>  object_property_set_bool(OBJECT(>cpc), true, "realized", );
>  if (err != NULL) {
>  error_propagate(errp, err);
>  return;
>  }

...but oddly it gets it right here and in a couple of other cases
in this patch.

thanks
-- PMM

[PATCH-for-5.0 07/12] hw/mips/cps: Add missing error-propagation code

2020-03-25 Thread Philippe Mathieu-Daudé 
Patch created mechanically by running:

  $ spatch \
--macro-file scripts/cocci-macro-file.h --include-headers \
--sp-file scripts/coccinelle/object_property_missing_error_propagate.cocci \
--keep-comments --smpl-spacing --in-place --dir hw

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/cps.c | 52 +++
 1 file changed, 52 insertions(+)

diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 92b9b1a5f6..d682633401 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -68,100 +68,152 @@ static bool cpu_mips_itu_supported(CPUMIPSState *env)
 static void mips_cps_realize(DeviceState *dev, Error **errp)
 {
 MIPSCPSState *s = MIPS_CPS(dev);
 CPUMIPSState *env;
 MIPSCPU *cpu;
 int i;
 Error *err = NULL;
 target_ulong gcr_base;
 bool itu_present = false;
 bool saar_present = false;
 
 for (i = 0; i < s->num_vp; i++) {
 cpu = MIPS_CPU(cpu_create(s->cpu_type));
 
 /* Init internal devices */
 cpu_mips_irq_init_cpu(cpu);
 cpu_mips_clock_init(cpu);
 
 env = >env;
 if (cpu_mips_itu_supported(env)) {
 itu_present = true;
 /* Attach ITC Tag to the VP */
 env->itc_tag = mips_itu_get_tag_region(>itu);
 env->itu = >itu;
 }
 qemu_register_reset(main_cpu_reset, cpu);
 }
 
 cpu = MIPS_CPU(first_cpu);
 env = >env;
 saar_present = (bool)env->saarp;
 
 /* Inter-Thread Communication Unit */
 if (itu_present) {
 sysbus_init_child_obj(OBJECT(dev), "itu", >itu, sizeof(s->itu),
   TYPE_MIPS_ITU);
 object_property_set_int(OBJECT(>itu), 16, "num-fifo", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_int(OBJECT(>itu), 16, "num-semaphores", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_bool(OBJECT(>itu), saar_present, "saar-present",
  );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 if (saar_present) {
 s->itu.saar = >CP0_SAAR;
 }
 object_property_set_bool(OBJECT(>itu), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
 
 memory_region_add_subregion(>container, 0,
sysbus_mmio_get_region(SYS_BUS_DEVICE(>itu), 0));
 }
 
 /* Cluster Power Controller */
 sysbus_init_child_obj(OBJECT(dev), "cpc", >cpc, sizeof(s->cpc),
   TYPE_MIPS_CPC);
 object_property_set_int(OBJECT(>cpc), s->num_vp, "num-vp", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_int(OBJECT(>cpc), 1, "vp-start-running", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_bool(OBJECT(>cpc), true, "realized", );
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
 
 memory_region_add_subregion(>container, 0,
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>cpc), 
0));
 
 /* Global Interrupt Controller */
 sysbus_init_child_obj(OBJECT(dev), "gic", >gic, sizeof(s->gic),
   TYPE_MIPS_GIC);
 object_property_set_int(OBJECT(>gic), s->num_vp, "num-vp", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_int(OBJECT(>gic), 128, "num-irq", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_bool(OBJECT(>gic), true, "realized", );
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
 
 memory_region_add_subregion(>container, 0,
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>gic), 
0));
 
 /* Global Configuration Registers */
 gcr_base = env->CP0_CMGCRBase << 4;
 
 sysbus_init_child_obj(OBJECT(dev), "gcr", >gcr, sizeof(s->gcr),
   TYPE_MIPS_GCR);
 object_property_set_int(OBJECT(>gcr), s->num_vp, "num-vp", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_int(OBJECT(>gcr), 0x800, "gcr-rev", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_int(OBJECT(>gcr), gcr_base, "gcr-base", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_link(OBJECT(>gcr), OBJECT(>gic.mr), "gic", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
 object_property_set_link(OBJECT(>gcr), OBJECT(>cpc.mr), "cpc", );
+if (err) {
+error_propagate(errp,