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

[Peter Crosthwaite]

On Tue, Dec 3, 2013 at 7:35 PM, Markus Armbruster <address@hidden> wrote:
> 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?
>

Correct, it's dead code.

>>
>>      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.
>

Will fix on respin.

Regards,
Peter

> [...]
>