Re: [Qemu-devel] [RFC PATCH v1 2/5] hw: Remove assert_no_error usages

[Markus Armbruster]

Peter Crosthwaite <address@hidden> writes:

> Replace assert_no_error() usages with the error_abort system.
> &error_abort is passed into API calls to signal to the Error sub-system
> that any errors are fatal. Removes need for caller assertions.
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
[...]
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index e374a93..7d869fc 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
[...]
> @@ -739,31 +737,19 @@ static void device_initfn(Object *obj)
>      class = object_get_class(OBJECT(dev));
>      do {
>          for (prop = DEVICE_CLASS(class)->props; prop && prop->name; prop++) {
> -            qdev_property_add_legacy(dev, prop, &err);
> -            assert_no_error(err);
> -            qdev_property_add_static(dev, prop, &err);
> -            assert_no_error(err);
> +            qdev_property_add_legacy(dev, prop, &error_abort);
> +            qdev_property_add_static(dev, prop, &error_abort);
>          }
>          class = object_class_get_parent(class);
>      } while (class != object_class_by_name(TYPE_DEVICE));
> -    if (err != NULL) {
> -        qerror_report_err(err);
> -        error_free(err);
> -        exit(1);
> -    }

Removal of these five lines isn't about replacing assert_no_error(),
it's burying dead code: err is initialized to null, and every place
where err can be set is followed by an assert_no_error(), therefore err
must still be null here.  Correct?

>  
>      object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
> -                             (Object **)&dev->parent_bus, &err);
> -    assert_no_error(err);
> +                             (Object **)&dev->parent_bus, &error_abort);
>  }
>  
>  static void device_post_init(Object *obj)
>  {
> -    DeviceState *dev = DEVICE(obj);
> -    Error *err = NULL;
> -
> -    qdev_prop_set_globals(dev, &err);
> -    assert_no_error(err);
> +    qdev_prop_set_globals(DEVICE(obj), &error_abort);
>  }
>  
>  /* Unlink device from bus and free the structure.  */
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index d67c5f1..bb92e41 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -569,26 +569,21 @@ static void xilinx_axidma_init(Object *obj)
>  {
>      XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> -    Error *errp = NULL;
>  
>      object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
> -                             (Object **) &s->tx_data_dev, &errp);
> -    assert_no_error(errp);
> +                             (Object **) &s->tx_data_dev, &error_abort);

You could use the opportunity and drop the space between cast and its
operand, for consistency with the other casts around here.  No need to
respin just for that, of course.

[...]