[Qemu-devel] [PATCH 4/4] arm: Clean up fragile use of error_is_set() in realize() methods

[Markus Armbruster]

Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque.  It's wrong when ERRP
may be null, because errors go undetected when it is.  It's fragile
when proving ERRP non-null involves a non-local argument.  Else, it's
unnecessarily opaque (see commit 84d18f0).

I guess the error_is_set(errp) in the DeviceClass realize() methods
are merely fragile right now, because I can't find a call chain that
passes a null errp argument.

Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.

Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
 hw/intc/arm_gic.c     | 6 ++++--
 hw/intc/arm_gic_kvm.c | 6 ++++--
 hw/intc/armv7m_nvic.c | 6 ++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 955b8d4..f6c7144 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -793,13 +793,15 @@ void gic_init_irqs_and_distributor(GICState *s, int 
num_irq)
 static void arm_gic_realize(DeviceState *dev, Error **errp)
 {
     /* Device instance realize function for the GIC sysbus device */
+    Error *local_err = NULL;
     int i;
     GICState *s = ARM_GIC(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
 
-    agc->parent_realize(dev, errp);
-    if (error_is_set(errp)) {
+    agc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 719d227..35d79a6 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -513,14 +513,16 @@ static void kvm_arm_gic_reset(DeviceState *dev)
 
 static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
 {
+    Error *local_err = NULL;
     int i;
     GICState *s = KVM_ARM_GIC(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s);
     int ret;
 
-    kgc->parent_realize(dev, errp);
-    if (error_is_set(errp)) {
+    kgc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 6066fa6..0d79def 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -472,6 +472,7 @@ static void armv7m_nvic_reset(DeviceState *dev)
 
 static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
 {
+    Error *local_err = NULL;
     nvic_state *s = NVIC(dev);
     NVICClass *nc = NVIC_GET_CLASS(s);
 
@@ -480,8 +481,9 @@ static void armv7m_nvic_realize(DeviceState *dev, Error 
**errp)
     /* Tell the common code we're an NVIC */
     s->gic.revision = 0xffffffff;
     s->num_irq = s->gic.num_irq;
-    nc->parent_realize(dev, errp);
-    if (error_is_set(errp)) {
+    nc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
     gic_init_irqs_and_distributor(&s->gic, s->num_irq);
-- 
1.8.1.4