diff mbox

[RFC,v1,2/5] hw: Remove assert_no_error usages

Message ID 6f22de9cc11bca34cce114adbeaf6c3486f5a822.1386049636.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite Dec. 3, 2013, 5:50 a.m. UTC 
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 <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev-properties-system.c |  8 ++------
 hw/core/qdev-properties.c        | 40 ++++++++++------------------------------
 hw/core/qdev.c                   | 28 +++++++---------------------
 hw/dma/xilinx_axidma.c           | 13 ++++---------
 hw/net/xilinx_axienet.c          | 13 ++++---------
 include/hw/xilinx.h              | 14 ++++----------
 6 files changed, 31 insertions(+), 85 deletions(-)

Comments

Markus Armbruster Dec. 3, 2013, 9:35 a.m. UTC | #1 
Peter Crosthwaite <peter.crosthwaite@xilinx.com> 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 <peter.crosthwaite@xilinx.com>
[...]
> 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.

[...]
Peter Crosthwaite Dec. 3, 2013, 10:04 a.m. UTC | #2 
On Tue, Dec 3, 2013 at 7:35 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> 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 <peter.crosthwaite@xilinx.com>
> [...]
>> 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

> [...]
>
diff mbox

Patch

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 729efa8..3f29b49 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -352,21 +352,17 @@  void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
                        CharDriverState *value)
 {
-    Error *errp = NULL;
     assert(!value || value->label);
     object_property_set_str(OBJECT(dev),
-                            value ? value->label : "", name, &errp);
-    assert_no_error(errp);
+                            value ? value->label : "", name, &error_abort);
 }
 
 void qdev_prop_set_netdev(DeviceState *dev, const char *name,
                           NetClientState *value)
 {
-    Error *errp = NULL;
     assert(!value || value->name);
     object_property_set_str(OBJECT(dev),
-                            value ? value->name : "", name, &errp);
-    assert_no_error(errp);
+                            value ? value->name : "", name, &error_abort);
 }
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..b949f0e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1003,73 +1003,55 @@  void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
 
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
-    Error *errp = NULL;
-    object_property_set_bool(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_bool(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
 {
-    Error *errp = NULL;
-    object_property_set_str(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_str(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
-    Error *errp = NULL;
     char str[2 * 6 + 5 + 1];
     snprintf(str, sizeof(str), "%02x:%02x:%02x:%02x:%02x:%02x",
              value[0], value[1], value[2], value[3], value[4], value[5]);
 
-    object_property_set_str(OBJECT(dev), str, name, &errp);
-    assert_no_error(errp);
+    object_property_set_str(OBJECT(dev), str, name, &error_abort);
 }
 
 void qdev_prop_set_enum(DeviceState *dev, const char *name, int value)
 {
     Property *prop;
-    Error *errp = NULL;
 
     prop = qdev_prop_find(dev, name);
     object_property_set_str(OBJECT(dev), prop->info->enum_table[value],
-                            name, &errp);
-    assert_no_error(errp);
+                            name, &error_abort);
 }
 
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
@@ -1161,12 +1143,10 @@  static void set_size(Object *obj, Visitor *v, void *opaque,
 static int parse_size(DeviceState *dev, Property *prop, const char *str)
 {
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    Error *errp = NULL;
 
     if (str != NULL) {
-        parse_option_size(prop->name, str, ptr, &errp);
+        parse_option_size(prop->name, str, ptr, &error_abort);
     }
-    assert_no_error(errp);
     return 0;
 }
 
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
@@ -656,14 +656,13 @@  void qdev_property_add_static(DeviceState *dev, Property *prop,
     }
 
     if (prop->qtype == QTYPE_QBOOL) {
-        object_property_set_bool(obj, prop->defval, prop->name, &local_err);
+        object_property_set_bool(obj, prop->defval, prop->name, &error_abort);
     } else if (prop->info->enum_table) {
         object_property_set_str(obj, prop->info->enum_table[prop->defval],
-                                prop->name, &local_err);
+                                prop->name, &error_abort);
     } else if (prop->qtype == QTYPE_QINT) {
-        object_property_set_int(obj, prop->defval, prop->name, &local_err);
+        object_property_set_int(obj, prop->defval, prop->name, &error_abort);
     }
-    assert_no_error(local_err);
 }
 
 static bool device_get_realized(Object *obj, Error **err)
@@ -723,7 +722,6 @@  static void device_initfn(Object *obj)
     DeviceState *dev = DEVICE(obj);
     ObjectClass *class;
     Property *prop;
-    Error *err = NULL;
 
     if (qdev_hotplug) {
         dev->hotplugged = 1;
@@ -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);
-    }
 
     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);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_control_dev, &errp);
-    assert_no_error(errp);
+                             (Object **) &s->tx_control_dev, &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_DMA_DATA_STREAM);
     object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
                       TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
     object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_data_dev, &error_abort);
     object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_control_dev, &error_abort);
 
     sysbus_init_irq(sbd, &s->streams[0].irq);
     sysbus_init_irq(sbd, &s->streams[1].irq);
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 3eb7715..0bd5eda 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -980,26 +980,21 @@  static void xilinx_enet_init(Object *obj)
 {
     XilinxAXIEnet *s = XILINX_AXI_ENET(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);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_control_dev, &errp);
-    assert_no_error(errp);
+                             (Object **) &s->tx_control_dev, &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_ENET_DATA_STREAM);
     object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
                       TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
     object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_data_dev, &error_abort);
     object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_control_dev, &error_abort);
 
     sysbus_init_irq(sbd, &s->irq);
 
diff --git a/include/hw/xilinx.h b/include/hw/xilinx.h
index 0c0251a..9d6debe 100644
--- a/include/hw/xilinx.h
+++ b/include/hw/xilinx.h
@@ -59,16 +59,13 @@  xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *ds,
                         StreamSlave *cs, hwaddr base, qemu_irq irq, int txmem,
                         int rxmem)
 {
-    Error *errp = NULL;
-
     qdev_set_nic_properties(dev, nd);
     qdev_prop_set_uint32(dev, "rxmem", rxmem);
     qdev_prop_set_uint32(dev, "txmem", txmem);
     object_property_set_link(OBJECT(dev), OBJECT(ds),
-                             "axistream-connected", &errp);
+                             "axistream-connected", &error_abort);
     object_property_set_link(OBJECT(dev), OBJECT(cs),
-                             "axistream-control-connected", &errp);
-    assert_no_error(errp);
+                             "axistream-control-connected", &error_abort);
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
@@ -78,14 +75,11 @@  static inline void
 xilinx_axidma_init(DeviceState *dev, StreamSlave *ds, StreamSlave *cs,
                    hwaddr base, qemu_irq irq, qemu_irq irq2, int freqhz)
 {
-    Error *errp = NULL;
-
     qdev_prop_set_uint32(dev, "freqhz", freqhz);
     object_property_set_link(OBJECT(dev), OBJECT(ds),
-                             "axistream-connected", &errp);
+                             "axistream-connected", &error_abort);
     object_property_set_link(OBJECT(dev), OBJECT(cs),
-                             "axistream-control-connected", &errp);
-    assert_no_error(errp);
+                             "axistream-control-connected", &error_abort);
     qdev_init_nofail(dev);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);