Re: [PATCH-for-5.0 10/12] hw/misc/macio/macio: Add missing error-propagation code

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5621 bytes --]

On Wed, Mar 25, 2020 at 08:18:28PM +0100, Philippe Mathieu-Daudé wrote:
> Running the coccinelle script produced:
> 
>   $ spatch \
>     --macro-file scripts/cocci-macro-file.h --include-headers \
>     --sp-file scripts/coccinelle/object_property_missing_error_propagate.cocci \
>     --keep-comments --smpl-spacing --dir hw
> 
>   [[manual check required: error_propagate() might be missing in object_property_set_bool() hw/misc/macio/macio.c:350:8]]
> 
> Add the missing error_propagate() after manual review.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/misc/macio/macio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 79222192e8..fffb64a7d5 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -292,98 +292,102 @@ static const MemoryRegionOps timer_ops = {
>  static void macio_newworld_realize(PCIDevice *d, Error **errp)
>  {
>      MacIOState *s = MACIO(d);
>      NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
>      DeviceState *pic_dev = DEVICE(ns->pic);
>      Error *err = NULL;
>      SysBusDevice *sysbus_dev;
>      MemoryRegion *timer_memory = NULL;
>  
>      macio_common_realize(d, &err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
>  
>      sysbus_dev = SYS_BUS_DEVICE(&s->escc);
>      sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
>                                                         NEWWORLD_ESCCB_IRQ));
>      sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
>                                                         NEWWORLD_ESCCA_IRQ));
>  
>      /* OpenPIC */
>      sysbus_dev = SYS_BUS_DEVICE(ns->pic);
>      memory_region_add_subregion(&s->bar, 0x40000,
>                                  sysbus_mmio_get_region(sysbus_dev, 0));
>  
>      /* IDE buses */
>      macio_realize_ide(s, &ns->ide[0],
>                        qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
>                        qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
>                        0x16, &err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
>  
>      macio_realize_ide(s, &ns->ide[1],
>                        qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
>                        qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
>                        0x1a, &err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
>  
>      /* Timer */
>      timer_memory = g_new(MemoryRegion, 1);
>      memory_region_init_io(timer_memory, OBJECT(s), &timer_ops, NULL, "timer",
>                            0x1000);
>      memory_region_add_subregion(&s->bar, 0x15000, timer_memory);
>  
>      if (ns->has_pmu) {
>          /* GPIOs */
>          sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
>          object_property_set_link(OBJECT(&ns->gpio), OBJECT(pic_dev), "pic",
>                                   &error_abort);
>          memory_region_add_subregion(&s->bar, 0x50,
>                                      sysbus_mmio_get_region(sysbus_dev, 0));
>          object_property_set_bool(OBJECT(&ns->gpio), true, "realized", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>  
>          /* PMU */
>          object_initialize_child(OBJECT(s), "pmu", &s->pmu, sizeof(s->pmu),
>                                  TYPE_VIA_PMU, &error_abort, NULL);
>          object_property_set_link(OBJECT(&s->pmu), OBJECT(sysbus_dev), "gpio",
>                                   &error_abort);
>          qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
>          qdev_set_parent_bus(DEVICE(&s->pmu), BUS(&s->macio_bus));
>  
>          object_property_set_bool(OBJECT(&s->pmu), true, "realized", &err);
>          if (err) {
>              error_propagate(errp, err);
>              return;
>          }
>          sysbus_dev = SYS_BUS_DEVICE(&s->pmu);
>          sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
>                                                             NEWWORLD_PMU_IRQ));
>          memory_region_add_subregion(&s->bar, 0x16000,
>                                      sysbus_mmio_get_region(sysbus_dev, 0));
>      } else {
>          /* CUDA */
>          object_initialize_child(OBJECT(s), "cuda", &s->cuda, sizeof(s->cuda),
>                                  TYPE_CUDA, &error_abort, NULL);
>          qdev_set_parent_bus(DEVICE(&s->cuda), BUS(&s->macio_bus));
>          qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
>                               s->frequency);
>  
>          object_property_set_bool(OBJECT(&s->cuda), true, "realized", &err);
>          if (err) {
>              error_propagate(errp, err);
>              return;
>          }
>          sysbus_dev = SYS_BUS_DEVICE(&s->cuda);
>          sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
>                                                             NEWWORLD_CUDA_IRQ));
>          memory_region_add_subregion(&s->bar, 0x16000,
>                                      sysbus_mmio_get_region(sysbus_dev, 0));
>      }
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]