All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
@ 2015-12-10 10:19 Markus Armbruster
  2015-12-10 10:49 ` Paolo Bonzini
  2015-12-10 11:32 ` Marcel Apfelbaum
  0 siblings, 2 replies; 9+ messages in thread
From: Markus Armbruster @ 2015-12-10 10:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, qemu-arm, Eduardo Habkost, Michael S. Tsirkin

Done with this admittedly crude Coccinelle semantic patch with manual
burial of dead Error * variables squashed in:

    @@
    identifier FUN;
    expression ERR, EC;
    @@
    -    FUN(&ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    FUN(&error_fatal);
    @@
    identifier FUN;
    expression ARG1, ERR, EC;
    @@
    -    FUN(ARG1, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    FUN(ARG1, &error_fatal);
    @@
    identifier FUN;
    expression ARG1, ARG2, ERR, EC;
    @@
    -    FUN(ARG1, ARG2, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    FUN(ARG1, ARG2, &error_fatal);
    @@
    identifier FUN;
    expression ARG1, ARG2, ARG3, ERR, EC;
    @@
    -    FUN(ARG1, ARG2, ARG3, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    FUN(ARG1, ARG2, ARG3, &error_fatal);
    @@
    identifier FUN;
    expression RET, ERR, EC;
    @@
    -    RET = FUN(&ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    RET = FUN(&error_fatal);
    @@
    identifier FUN;
    expression RET, ARG1, ERR, EC;
    @@
    -    RET = FUN(ARG1, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    RET = FUN(ARG1, &error_fatal);
    @@
    identifier FUN;
    expression RET, ARG1, ARG2, ERR, EC;
    @@
    -    RET = FUN(ARG1, ARG2, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    RET = FUN(ARG1, ARG2, &error_fatal);
    @@
    identifier FUN;
    expression RET, ARG1, ARG2, ARG3, ERR, EC;
    @@
    -    RET = FUN(ARG1, ARG2, ARG3, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
    @@
    type T;
    identifier FUN, RET;
    expression ERR, EC;
    @@
    -    T RET = FUN(&ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    T RET = FUN(&error_fatal);
    @@
    type T;
    identifier FUN, RET;
    expression ARG1, ERR, EC;
    @@
    -    T RET = FUN(ARG1, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    T RET = FUN(ARG1, &error_fatal);
    @@
    type T;
    identifier FUN, RET;
    expression ARG1, ARG2, ERR, EC;
    @@
    -    T RET = FUN(ARG1, ARG2, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    T RET = FUN(ARG1, ARG2, &error_fatal);
    @@
    type T;
    identifier FUN, RET;
    expression ARG1, ARG2, ARG3, ERR, EC;
    @@
    -    T RET = FUN(ARG1, ARG2, ARG3, &ERR);
    -    if (ERR != NULL) {
    -        error_report_err(ERR);
    -        exit(EC);
    -    }
    +    T RET = FUN(ARG1, ARG2, ARG3, &error_fatal);

Cc: qemu-arm@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/exynos4210.c              | 13 ++----------
 hw/arm/highbank.c                |  7 +------
 hw/arm/integratorcp.c            | 13 ++----------
 hw/arm/realview.c                | 20 ++++---------------
 hw/arm/versatilepb.c             | 13 ++----------
 hw/arm/vexpress.c                |  7 +------
 hw/arm/xilinx_zynq.c             | 31 +++++++++--------------------
 hw/char/serial.c                 | 14 ++-----------
 hw/core/qdev-properties-system.c |  8 +-------
 hw/i386/pc.c                     | 14 ++-----------
 hw/smbios/smbios.c               | 43 +++++++---------------------------------
 numa.c                           |  8 ++------
 vl.c                             | 21 +++-----------------
 13 files changed, 38 insertions(+), 174 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index d934980..79b7c5a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -150,27 +150,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
 
     for (n = 0; n < EXYNOS4210_NCPUS; n++) {
         Object *cpuobj = object_new(object_class_get_name(cpu_oc));
-        Error *err = NULL;
 
         /* By default A9 CPUs have EL3 enabled.  This board does not currently
          * support EL3 so the CPU EL3 property is disabled before realization.
          */
         if (object_property_find(cpuobj, "has_el3", NULL)) {
-            object_property_set_bool(cpuobj, false, "has_el3", &err);
-            if (err) {
-                error_report_err(err);
-                exit(1);
-            }
+            object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
         }
 
         s->cpu[n] = ARM_CPU(cpuobj);
         object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
                                 "reset-cbar", &error_abort);
-        object_property_set_bool(cpuobj, true, "realized", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
     }
 
     /*** IRQs ***/
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 85ae69e..a0a5a06 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -279,7 +279,6 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
         Object *cpuobj;
         ARMCPU *cpu;
-        Error *err = NULL;
 
         cpuobj = object_new(object_class_get_name(oc));
         cpu = ARM_CPU(cpuobj);
@@ -297,11 +296,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
             object_property_set_int(cpuobj, MPCORE_PERIPHBASE,
                                     "reset-cbar", &error_abort);
         }
-        object_property_set_bool(cpuobj, true, "realized", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
         cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
         cpu_fiq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ);
     }
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 421bde9..96dedce 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -533,7 +533,6 @@ static void integratorcp_init(MachineState *machine)
     qemu_irq pic[32];
     DeviceState *dev, *sic, *icp;
     int i;
-    Error *err = NULL;
 
     if (!cpu_model) {
         cpu_model = "arm926";
@@ -552,18 +551,10 @@ static void integratorcp_init(MachineState *machine)
      * realization.
      */
     if (object_property_find(cpuobj, "has_el3", NULL)) {
-        object_property_set_bool(cpuobj, false, "has_el3", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
     }
 
-    object_property_set_bool(cpuobj, true, "realized", &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
+    object_property_set_bool(cpuobj, true, "realized", &error_fatal);
 
     cpu = ARM_CPU(cpuobj);
 
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index e14828d..2d6952c 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -99,33 +99,21 @@ static void realview_init(MachineState *machine,
 
     for (n = 0; n < smp_cpus; n++) {
         Object *cpuobj = object_new(object_class_get_name(cpu_oc));
-        Error *err = NULL;
 
         /* By default A9,A15 and ARM1176 CPUs have EL3 enabled.  This board
          * does not currently support EL3 so the CPU EL3 property is disabled
          * before realization.
          */
         if (object_property_find(cpuobj, "has_el3", NULL)) {
-            object_property_set_bool(cpuobj, false, "has_el3", &err);
-            if (err) {
-                error_report_err(err);
-                exit(1);
-            }
+            object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
         }
 
         if (is_pb && is_mpcore) {
-            object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
-            if (err) {
-                error_report_err(err);
-                exit(1);
-            }
+            object_property_set_int(cpuobj, periphbase, "reset-cbar",
+                                    &error_fatal);
         }
 
-        object_property_set_bool(cpuobj, true, "realized", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
 
         cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpuobj), ARM_CPU_IRQ);
     }
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 912c290..70eefe9 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -192,7 +192,6 @@ static void versatile_init(MachineState *machine, int board_id)
     int n;
     int done_smc = 0;
     DriveInfo *dinfo;
-    Error *err = NULL;
 
     if (!machine->cpu_model) {
         machine->cpu_model = "arm926";
@@ -211,18 +210,10 @@ static void versatile_init(MachineState *machine, int board_id)
      * realization.
      */
     if (object_property_find(cpuobj, "has_el3", NULL)) {
-        object_property_set_bool(cpuobj, false, "has_el3", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
     }
 
-    object_property_set_bool(cpuobj, true, "realized", &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
+    object_property_set_bool(cpuobj, true, "realized", &error_fatal);
 
     cpu = ARM_CPU(cpuobj);
 
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 058abbd..ea9a984 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -211,7 +211,6 @@ static void init_cpus(const char *cpu_model, const char *privdev,
     /* Create the actual CPUs */
     for (n = 0; n < smp_cpus; n++) {
         Object *cpuobj = object_new(object_class_get_name(cpu_oc));
-        Error *err = NULL;
 
         if (!secure) {
             object_property_set_bool(cpuobj, false, "has_el3", NULL);
@@ -221,11 +220,7 @@ static void init_cpus(const char *cpu_model, const char *privdev,
             object_property_set_int(cpuobj, periphbase,
                                     "reset-cbar", &error_abort);
         }
-        object_property_set_bool(cpuobj, true, "realized", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
     }
 
     /* Create the private peripheral devices (including the GIC);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1c1a445..409f5f0 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -156,7 +156,6 @@ static void zynq_init(MachineState *machine)
     DeviceState *dev;
     SysBusDevice *busdev;
     qemu_irq pic[64];
-    Error *err = NULL;
     int n;
 
     if (!cpu_model) {
@@ -171,29 +170,17 @@ static void zynq_init(MachineState *machine)
      * realization.
      */
     if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
-        object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
-        if (err) {
-            error_report_err(err);
-            exit(1);
-        }
+        object_property_set_bool(OBJECT(cpu), false, "has_el3", &error_fatal);
     }
 
-    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
-
-    object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
-    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
+    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
+                            &error_fatal);object_property_set_int(OBJECT(cpu),
+                                                                  MPCORE_PERIPHBASE,
+                                                                  "reset-cbar",
+                                                                  &error_fatal);object_property_set_bool(OBJECT(cpu),
+                                                                                                         true,
+                                                                                                         "realized",
+                                                                                                         &error_fatal);
 
     /* max 2GB ram */
     if (ram_size > 0x80000000) {
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 513d73c..566e9ef 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -888,18 +888,13 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          CharDriverState *chr, MemoryRegion *system_io)
 {
     SerialState *s;
-    Error *err = NULL;
 
     s = g_malloc0(sizeof(SerialState));
 
     s->irq = irq;
     s->baudbase = baudbase;
     s->chr = chr;
-    serial_realize_core(s, &err);
-    if (err != NULL) {
-        error_report_err(err);
-        exit(1);
-    }
+    serial_realize_core(s, &error_fatal);
 
     vmstate_register(NULL, base, &vmstate_serial, s);
 
@@ -949,7 +944,6 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
                             CharDriverState *chr, enum device_endian end)
 {
     SerialState *s;
-    Error *err = NULL;
 
     s = g_malloc0(sizeof(SerialState));
 
@@ -958,11 +952,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
     s->baudbase = baudbase;
     s->chr = chr;
 
-    serial_realize_core(s, &err);
-    if (err != NULL) {
-        error_report_err(err);
-        exit(1);
-    }
+    serial_realize_core(s, &error_fatal);
     vmstate_register(NULL, base, &vmstate_serial, s);
 
     memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 921e799..d515e99 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -367,13 +367,7 @@ void qdev_prop_set_drive(DeviceState *dev, const char *name,
 void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
                                 BlockBackend *value)
 {
-    Error *err = NULL;
-
-    qdev_prop_set_drive(dev, name, value, &err);
-    if (err) {
-        error_report_err(err);
-        exit(1);
-    }
+    qdev_prop_set_drive(dev, name, value, &error_fatal);
 }
 
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..3025ca5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -433,7 +433,6 @@ void pc_cmos_init(PCMachineState *pcms,
 {
     int val;
     static pc_cmos_init_late_arg arg;
-    Error *local_err = NULL;
 
     /* various important CMOS locations needed by PC/Bochs bios */
 
@@ -481,11 +480,7 @@ void pc_cmos_init(PCMachineState *pcms,
     object_property_set_link(OBJECT(pcms), OBJECT(s),
                              "rtc_state", &error_abort);
 
-    set_boot_dev(s, MACHINE(pcms)->boot_order, &local_err);
-    if (local_err) {
-        error_report_err(local_err);
-        exit(1);
-    }
+    set_boot_dev(s, MACHINE(pcms)->boot_order, &error_fatal);
 
     val = 0;
     val |= 0x02; /* FPU is there */
@@ -1122,7 +1117,6 @@ void pc_cpus_init(PCMachineState *pcms)
     int i;
     X86CPU *cpu = NULL;
     MachineState *machine = MACHINE(pcms);
-    Error *error = NULL;
     unsigned long apic_id_limit;
 
     /* init CPUs */
@@ -1143,11 +1137,7 @@ void pc_cpus_init(PCMachineState *pcms)
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
-                         &error);
-        if (error) {
-            error_report_err(error);
-            exit(1);
-        }
+                         &error_fatal);
         object_unref(OBJECT(cpu));
     }
 
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index b81a1d3..a3e575a 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -937,7 +937,6 @@ static void save_opt(const char **dest, QemuOpts *opts, const char *name)
 
 void smbios_entry_add(QemuOpts *opts)
 {
-    Error *local_err = NULL;
     const char *val;
 
     assert(!smbios_immutable);
@@ -948,11 +947,7 @@ void smbios_entry_add(QemuOpts *opts)
         int size;
         struct smbios_table *table; /* legacy mode only */
 
-        qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-            exit(1);
-        }
+        qemu_opts_validate(opts, qemu_smbios_file_opts, &error_fatal);
 
         size = get_image_size(val);
         if (size == -1 || size < sizeof(struct smbios_structure_header)) {
@@ -1034,11 +1029,7 @@ void smbios_entry_add(QemuOpts *opts)
 
         switch (type) {
         case 0:
-            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type0_opts, &error_fatal);
             save_opt(&type0.vendor, opts, "vendor");
             save_opt(&type0.version, opts, "version");
             save_opt(&type0.date, opts, "date");
@@ -1054,11 +1045,7 @@ void smbios_entry_add(QemuOpts *opts)
             }
             return;
         case 1:
-            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type1_opts, &error_fatal);
             save_opt(&type1.manufacturer, opts, "manufacturer");
             save_opt(&type1.product, opts, "product");
             save_opt(&type1.version, opts, "version");
@@ -1076,11 +1063,7 @@ void smbios_entry_add(QemuOpts *opts)
             }
             return;
         case 2:
-            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type2_opts, &error_fatal);
             save_opt(&type2.manufacturer, opts, "manufacturer");
             save_opt(&type2.product, opts, "product");
             save_opt(&type2.version, opts, "version");
@@ -1089,11 +1072,7 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type2.location, opts, "location");
             return;
         case 3:
-            qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type3_opts, &error_fatal);
             save_opt(&type3.manufacturer, opts, "manufacturer");
             save_opt(&type3.version, opts, "version");
             save_opt(&type3.serial, opts, "serial");
@@ -1101,11 +1080,7 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type3.sku, opts, "sku");
             return;
         case 4:
-            qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type4_opts, &error_fatal);
             save_opt(&type4.sock_pfx, opts, "sock_pfx");
             save_opt(&type4.manufacturer, opts, "manufacturer");
             save_opt(&type4.version, opts, "version");
@@ -1114,11 +1089,7 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type4.part, opts, "part");
             return;
         case 17:
-            qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            qemu_opts_validate(opts, qemu_smbios_type17_opts, &error_fatal);
             save_opt(&type17.loc_pfx, opts, "loc_pfx");
             save_opt(&type17.bank, opts, "bank");
             save_opt(&type17.manufacturer, opts, "manufacturer");
diff --git a/numa.c b/numa.c
index fdfe294..bbdf5b8 100644
--- a/numa.c
+++ b/numa.c
@@ -450,17 +450,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
 
     memory_region_init(mr, owner, name, ram_size);
     for (i = 0; i < MAX_NODES; i++) {
-        Error *local_err = NULL;
         uint64_t size = numa_info[i].node_mem;
         HostMemoryBackend *backend = numa_info[i].node_memdev;
         if (!backend) {
             continue;
         }
-        MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-            exit(1);
-        }
+        MemoryRegion *seg = host_memory_backend_get_memory(backend,
+                                                           &error_fatal);
 
         if (memory_region_is_mapped(seg)) {
             char *path = object_get_canonical_path_component(OBJECT(backend));
diff --git a/vl.c b/vl.c
index 4211ff1..5cc6bfd 100644
--- a/vl.c
+++ b/vl.c
@@ -4329,12 +4329,7 @@ int main(int argc, char **argv, char **envp)
     configure_accelerator(current_machine);
 
     if (qtest_chrdev) {
-        Error *local_err = NULL;
-        qtest_init(qtest_chrdev, qtest_log, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-            exit(1);
-        }
+        qtest_init(qtest_chrdev, qtest_log, &error_fatal);
     }
 
     machine_opts = qemu_get_machine_opts();
@@ -4345,24 +4340,14 @@ int main(int argc, char **argv, char **envp)
 
     opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
     if (opts) {
-        Error *local_err = NULL;
-
         boot_order = qemu_opt_get(opts, "order");
         if (boot_order) {
-            validate_bootdevices(boot_order, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            validate_bootdevices(boot_order, &error_fatal);
         }
 
         boot_once = qemu_opt_get(opts, "once");
         if (boot_once) {
-            validate_bootdevices(boot_once, &local_err);
-            if (local_err) {
-                error_report_err(local_err);
-                exit(1);
-            }
+            validate_bootdevices(boot_once, &error_fatal);
         }
 
         boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 10:19 [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors Markus Armbruster
@ 2015-12-10 10:49 ` Paolo Bonzini
  2015-12-10 12:31   ` Markus Armbruster
  2015-12-10 11:32 ` Marcel Apfelbaum
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2015-12-10 10:49 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel
  Cc: qemu-arm, Eduardo Habkost, Michael S. Tsirkin



On 10/12/2015 11:19, Markus Armbruster wrote:
> +    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
> +                            &error_fatal);object_property_set_int(OBJECT(cpu),
> +                                                                  MPCORE_PERIPHBASE,
> +                                                                  "reset-cbar",
> +                                                                  &error_fatal);object_property_set_bool(OBJECT(cpu),
> +                                                                                                         true,
> +                                                                                                         "realized",
> +                                                                                                         &error_fatal);

Something went wrong here. :)

>  void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
>                                  BlockBackend *value)
>  {
> -    Error *err = NULL;
> -
> -    qdev_prop_set_drive(dev, name, value, &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    qdev_prop_set_drive(dev, name, value, &error_fatal);
>  }


This should be inlined entirely into the callers (possibly as a follow up).

Otherwise looks great, thanks!

Paolo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 10:19 [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors Markus Armbruster
  2015-12-10 10:49 ` Paolo Bonzini
@ 2015-12-10 11:32 ` Marcel Apfelbaum
  2015-12-10 12:34   ` Markus Armbruster
  1 sibling, 1 reply; 9+ messages in thread
From: Marcel Apfelbaum @ 2015-12-10 11:32 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel
  Cc: Paolo Bonzini, qemu-arm, Eduardo Habkost, Michael S. Tsirkin

On 12/10/2015 12:19 PM, Markus Armbruster wrote:
> Done with this admittedly crude Coccinelle semantic patch with manual
> burial of dead Error * variables squashed in:
>
>      @@
>      identifier FUN;
>      expression ERR, EC;
>      @@
>      -    FUN(&ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    FUN(&error_fatal);
>      @@
>      identifier FUN;
>      expression ARG1, ERR, EC;
>      @@
>      -    FUN(ARG1, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    FUN(ARG1, &error_fatal);
>      @@
>      identifier FUN;
>      expression ARG1, ARG2, ERR, EC;
>      @@
>      -    FUN(ARG1, ARG2, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    FUN(ARG1, ARG2, &error_fatal);
>      @@
>      identifier FUN;
>      expression ARG1, ARG2, ARG3, ERR, EC;
>      @@
>      -    FUN(ARG1, ARG2, ARG3, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    FUN(ARG1, ARG2, ARG3, &error_fatal);
>      @@
>      identifier FUN;
>      expression RET, ERR, EC;
>      @@
>      -    RET = FUN(&ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    RET = FUN(&error_fatal);
>      @@
>      identifier FUN;
>      expression RET, ARG1, ERR, EC;
>      @@
>      -    RET = FUN(ARG1, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    RET = FUN(ARG1, &error_fatal);
>      @@
>      identifier FUN;
>      expression RET, ARG1, ARG2, ERR, EC;
>      @@
>      -    RET = FUN(ARG1, ARG2, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    RET = FUN(ARG1, ARG2, &error_fatal);
>      @@
>      identifier FUN;
>      expression RET, ARG1, ARG2, ARG3, ERR, EC;
>      @@
>      -    RET = FUN(ARG1, ARG2, ARG3, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>      @@
>      type T;
>      identifier FUN, RET;
>      expression ERR, EC;
>      @@
>      -    T RET = FUN(&ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    T RET = FUN(&error_fatal);
>      @@
>      type T;
>      identifier FUN, RET;
>      expression ARG1, ERR, EC;
>      @@
>      -    T RET = FUN(ARG1, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    T RET = FUN(ARG1, &error_fatal);
>      @@
>      type T;
>      identifier FUN, RET;
>      expression ARG1, ARG2, ERR, EC;
>      @@
>      -    T RET = FUN(ARG1, ARG2, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    T RET = FUN(ARG1, ARG2, &error_fatal);
>      @@
>      type T;
>      identifier FUN, RET;
>      expression ARG1, ARG2, ARG3, ERR, EC;
>      @@
>      -    T RET = FUN(ARG1, ARG2, ARG3, &ERR);
>      -    if (ERR != NULL) {
>      -        error_report_err(ERR);
>      -        exit(EC);
>      -    }
>      +    T RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>

That's so cool!
Isn't it the time to have our own Coccinelle directory
with scripts like this? And to make them part of make check?

Is a pity to have them lost into a git comment...

Thanks,
Marcel

> Cc: qemu-arm@nongnu.org
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/arm/exynos4210.c              | 13 ++----------
>   hw/arm/highbank.c                |  7 +------
>   hw/arm/integratorcp.c            | 13 ++----------
>   hw/arm/realview.c                | 20 ++++---------------
>   hw/arm/versatilepb.c             | 13 ++----------
>   hw/arm/vexpress.c                |  7 +------
>   hw/arm/xilinx_zynq.c             | 31 +++++++++--------------------
>   hw/char/serial.c                 | 14 ++-----------
>   hw/core/qdev-properties-system.c |  8 +-------
>   hw/i386/pc.c                     | 14 ++-----------
>   hw/smbios/smbios.c               | 43 +++++++---------------------------------
>   numa.c                           |  8 ++------
>   vl.c                             | 21 +++-----------------
>   13 files changed, 38 insertions(+), 174 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index d934980..79b7c5a 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -150,27 +150,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
>
>       for (n = 0; n < EXYNOS4210_NCPUS; n++) {
>           Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> -        Error *err = NULL;
>
>           /* By default A9 CPUs have EL3 enabled.  This board does not currently
>            * support EL3 so the CPU EL3 property is disabled before realization.
>            */
>           if (object_property_find(cpuobj, "has_el3", NULL)) {
> -            object_property_set_bool(cpuobj, false, "has_el3", &err);
> -            if (err) {
> -                error_report_err(err);
> -                exit(1);
> -            }
> +            object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
>           }
>
>           s->cpu[n] = ARM_CPU(cpuobj);
>           object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
>                                   "reset-cbar", &error_abort);
> -        object_property_set_bool(cpuobj, true, "realized", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>       }
>
>       /*** IRQs ***/
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 85ae69e..a0a5a06 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -279,7 +279,6 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>           ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>           Object *cpuobj;
>           ARMCPU *cpu;
> -        Error *err = NULL;
>
>           cpuobj = object_new(object_class_get_name(oc));
>           cpu = ARM_CPU(cpuobj);
> @@ -297,11 +296,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>               object_property_set_int(cpuobj, MPCORE_PERIPHBASE,
>                                       "reset-cbar", &error_abort);
>           }
> -        object_property_set_bool(cpuobj, true, "realized", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>           cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
>           cpu_fiq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ);
>       }
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 421bde9..96dedce 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -533,7 +533,6 @@ static void integratorcp_init(MachineState *machine)
>       qemu_irq pic[32];
>       DeviceState *dev, *sic, *icp;
>       int i;
> -    Error *err = NULL;
>
>       if (!cpu_model) {
>           cpu_model = "arm926";
> @@ -552,18 +551,10 @@ static void integratorcp_init(MachineState *machine)
>        * realization.
>        */
>       if (object_property_find(cpuobj, "has_el3", NULL)) {
> -        object_property_set_bool(cpuobj, false, "has_el3", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
>       }
>
> -    object_property_set_bool(cpuobj, true, "realized", &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>
>       cpu = ARM_CPU(cpuobj);
>
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index e14828d..2d6952c 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -99,33 +99,21 @@ static void realview_init(MachineState *machine,
>
>       for (n = 0; n < smp_cpus; n++) {
>           Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> -        Error *err = NULL;
>
>           /* By default A9,A15 and ARM1176 CPUs have EL3 enabled.  This board
>            * does not currently support EL3 so the CPU EL3 property is disabled
>            * before realization.
>            */
>           if (object_property_find(cpuobj, "has_el3", NULL)) {
> -            object_property_set_bool(cpuobj, false, "has_el3", &err);
> -            if (err) {
> -                error_report_err(err);
> -                exit(1);
> -            }
> +            object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
>           }
>
>           if (is_pb && is_mpcore) {
> -            object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
> -            if (err) {
> -                error_report_err(err);
> -                exit(1);
> -            }
> +            object_property_set_int(cpuobj, periphbase, "reset-cbar",
> +                                    &error_fatal);
>           }
>
> -        object_property_set_bool(cpuobj, true, "realized", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>
>           cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpuobj), ARM_CPU_IRQ);
>       }
> diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
> index 912c290..70eefe9 100644
> --- a/hw/arm/versatilepb.c
> +++ b/hw/arm/versatilepb.c
> @@ -192,7 +192,6 @@ static void versatile_init(MachineState *machine, int board_id)
>       int n;
>       int done_smc = 0;
>       DriveInfo *dinfo;
> -    Error *err = NULL;
>
>       if (!machine->cpu_model) {
>           machine->cpu_model = "arm926";
> @@ -211,18 +210,10 @@ static void versatile_init(MachineState *machine, int board_id)
>        * realization.
>        */
>       if (object_property_find(cpuobj, "has_el3", NULL)) {
> -        object_property_set_bool(cpuobj, false, "has_el3", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
>       }
>
> -    object_property_set_bool(cpuobj, true, "realized", &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>
>       cpu = ARM_CPU(cpuobj);
>
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index 058abbd..ea9a984 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -211,7 +211,6 @@ static void init_cpus(const char *cpu_model, const char *privdev,
>       /* Create the actual CPUs */
>       for (n = 0; n < smp_cpus; n++) {
>           Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> -        Error *err = NULL;
>
>           if (!secure) {
>               object_property_set_bool(cpuobj, false, "has_el3", NULL);
> @@ -221,11 +220,7 @@ static void init_cpus(const char *cpu_model, const char *privdev,
>               object_property_set_int(cpuobj, periphbase,
>                                       "reset-cbar", &error_abort);
>           }
> -        object_property_set_bool(cpuobj, true, "realized", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
>       }
>
>       /* Create the private peripheral devices (including the GIC);
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 1c1a445..409f5f0 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -156,7 +156,6 @@ static void zynq_init(MachineState *machine)
>       DeviceState *dev;
>       SysBusDevice *busdev;
>       qemu_irq pic[64];
> -    Error *err = NULL;
>       int n;
>
>       if (!cpu_model) {
> @@ -171,29 +170,17 @@ static void zynq_init(MachineState *machine)
>        * realization.
>        */
>       if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
> -        object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
> -        if (err) {
> -            error_report_err(err);
> -            exit(1);
> -        }
> +        object_property_set_bool(OBJECT(cpu), false, "has_el3", &error_fatal);
>       }
>
> -    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -
> -    object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
> +                            &error_fatal);object_property_set_int(OBJECT(cpu),
> +                                                                  MPCORE_PERIPHBASE,
> +                                                                  "reset-cbar",
> +                                                                  &error_fatal);object_property_set_bool(OBJECT(cpu),
> +                                                                                                         true,
> +                                                                                                         "realized",
> +                                                                                                         &error_fatal);
>
>       /* max 2GB ram */
>       if (ram_size > 0x80000000) {
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 513d73c..566e9ef 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -888,18 +888,13 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>                            CharDriverState *chr, MemoryRegion *system_io)
>   {
>       SerialState *s;
> -    Error *err = NULL;
>
>       s = g_malloc0(sizeof(SerialState));
>
>       s->irq = irq;
>       s->baudbase = baudbase;
>       s->chr = chr;
> -    serial_realize_core(s, &err);
> -    if (err != NULL) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    serial_realize_core(s, &error_fatal);
>
>       vmstate_register(NULL, base, &vmstate_serial, s);
>
> @@ -949,7 +944,6 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>                               CharDriverState *chr, enum device_endian end)
>   {
>       SerialState *s;
> -    Error *err = NULL;
>
>       s = g_malloc0(sizeof(SerialState));
>
> @@ -958,11 +952,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>       s->baudbase = baudbase;
>       s->chr = chr;
>
> -    serial_realize_core(s, &err);
> -    if (err != NULL) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    serial_realize_core(s, &error_fatal);
>       vmstate_register(NULL, base, &vmstate_serial, s);
>
>       memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
> index 921e799..d515e99 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -367,13 +367,7 @@ void qdev_prop_set_drive(DeviceState *dev, const char *name,
>   void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
>                                   BlockBackend *value)
>   {
> -    Error *err = NULL;
> -
> -    qdev_prop_set_drive(dev, name, value, &err);
> -    if (err) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    qdev_prop_set_drive(dev, name, value, &error_fatal);
>   }
>
>   void qdev_prop_set_chr(DeviceState *dev, const char *name,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e20e07..3025ca5 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -433,7 +433,6 @@ void pc_cmos_init(PCMachineState *pcms,
>   {
>       int val;
>       static pc_cmos_init_late_arg arg;
> -    Error *local_err = NULL;
>
>       /* various important CMOS locations needed by PC/Bochs bios */
>
> @@ -481,11 +480,7 @@ void pc_cmos_init(PCMachineState *pcms,
>       object_property_set_link(OBJECT(pcms), OBJECT(s),
>                                "rtc_state", &error_abort);
>
> -    set_boot_dev(s, MACHINE(pcms)->boot_order, &local_err);
> -    if (local_err) {
> -        error_report_err(local_err);
> -        exit(1);
> -    }
> +    set_boot_dev(s, MACHINE(pcms)->boot_order, &error_fatal);
>
>       val = 0;
>       val |= 0x02; /* FPU is there */
> @@ -1122,7 +1117,6 @@ void pc_cpus_init(PCMachineState *pcms)
>       int i;
>       X86CPU *cpu = NULL;
>       MachineState *machine = MACHINE(pcms);
> -    Error *error = NULL;
>       unsigned long apic_id_limit;
>
>       /* init CPUs */
> @@ -1143,11 +1137,7 @@ void pc_cpus_init(PCMachineState *pcms)
>
>       for (i = 0; i < smp_cpus; i++) {
>           cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
> -                         &error);
> -        if (error) {
> -            error_report_err(error);
> -            exit(1);
> -        }
> +                         &error_fatal);
>           object_unref(OBJECT(cpu));
>       }
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index b81a1d3..a3e575a 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -937,7 +937,6 @@ static void save_opt(const char **dest, QemuOpts *opts, const char *name)
>
>   void smbios_entry_add(QemuOpts *opts)
>   {
> -    Error *local_err = NULL;
>       const char *val;
>
>       assert(!smbios_immutable);
> @@ -948,11 +947,7 @@ void smbios_entry_add(QemuOpts *opts)
>           int size;
>           struct smbios_table *table; /* legacy mode only */
>
> -        qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> -            exit(1);
> -        }
> +        qemu_opts_validate(opts, qemu_smbios_file_opts, &error_fatal);
>
>           size = get_image_size(val);
>           if (size == -1 || size < sizeof(struct smbios_structure_header)) {
> @@ -1034,11 +1029,7 @@ void smbios_entry_add(QemuOpts *opts)
>
>           switch (type) {
>           case 0:
> -            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type0_opts, &error_fatal);
>               save_opt(&type0.vendor, opts, "vendor");
>               save_opt(&type0.version, opts, "version");
>               save_opt(&type0.date, opts, "date");
> @@ -1054,11 +1045,7 @@ void smbios_entry_add(QemuOpts *opts)
>               }
>               return;
>           case 1:
> -            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type1_opts, &error_fatal);
>               save_opt(&type1.manufacturer, opts, "manufacturer");
>               save_opt(&type1.product, opts, "product");
>               save_opt(&type1.version, opts, "version");
> @@ -1076,11 +1063,7 @@ void smbios_entry_add(QemuOpts *opts)
>               }
>               return;
>           case 2:
> -            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type2_opts, &error_fatal);
>               save_opt(&type2.manufacturer, opts, "manufacturer");
>               save_opt(&type2.product, opts, "product");
>               save_opt(&type2.version, opts, "version");
> @@ -1089,11 +1072,7 @@ void smbios_entry_add(QemuOpts *opts)
>               save_opt(&type2.location, opts, "location");
>               return;
>           case 3:
> -            qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type3_opts, &error_fatal);
>               save_opt(&type3.manufacturer, opts, "manufacturer");
>               save_opt(&type3.version, opts, "version");
>               save_opt(&type3.serial, opts, "serial");
> @@ -1101,11 +1080,7 @@ void smbios_entry_add(QemuOpts *opts)
>               save_opt(&type3.sku, opts, "sku");
>               return;
>           case 4:
> -            qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type4_opts, &error_fatal);
>               save_opt(&type4.sock_pfx, opts, "sock_pfx");
>               save_opt(&type4.manufacturer, opts, "manufacturer");
>               save_opt(&type4.version, opts, "version");
> @@ -1114,11 +1089,7 @@ void smbios_entry_add(QemuOpts *opts)
>               save_opt(&type4.part, opts, "part");
>               return;
>           case 17:
> -            qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            qemu_opts_validate(opts, qemu_smbios_type17_opts, &error_fatal);
>               save_opt(&type17.loc_pfx, opts, "loc_pfx");
>               save_opt(&type17.bank, opts, "bank");
>               save_opt(&type17.manufacturer, opts, "manufacturer");
> diff --git a/numa.c b/numa.c
> index fdfe294..bbdf5b8 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -450,17 +450,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>
>       memory_region_init(mr, owner, name, ram_size);
>       for (i = 0; i < MAX_NODES; i++) {
> -        Error *local_err = NULL;
>           uint64_t size = numa_info[i].node_mem;
>           HostMemoryBackend *backend = numa_info[i].node_memdev;
>           if (!backend) {
>               continue;
>           }
> -        MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> -            exit(1);
> -        }
> +        MemoryRegion *seg = host_memory_backend_get_memory(backend,
> +                                                           &error_fatal);
>
>           if (memory_region_is_mapped(seg)) {
>               char *path = object_get_canonical_path_component(OBJECT(backend));
> diff --git a/vl.c b/vl.c
> index 4211ff1..5cc6bfd 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4329,12 +4329,7 @@ int main(int argc, char **argv, char **envp)
>       configure_accelerator(current_machine);
>
>       if (qtest_chrdev) {
> -        Error *local_err = NULL;
> -        qtest_init(qtest_chrdev, qtest_log, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> -            exit(1);
> -        }
> +        qtest_init(qtest_chrdev, qtest_log, &error_fatal);
>       }
>
>       machine_opts = qemu_get_machine_opts();
> @@ -4345,24 +4340,14 @@ int main(int argc, char **argv, char **envp)
>
>       opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
>       if (opts) {
> -        Error *local_err = NULL;
> -
>           boot_order = qemu_opt_get(opts, "order");
>           if (boot_order) {
> -            validate_bootdevices(boot_order, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            validate_bootdevices(boot_order, &error_fatal);
>           }
>
>           boot_once = qemu_opt_get(opts, "once");
>           if (boot_once) {
> -            validate_bootdevices(boot_once, &local_err);
> -            if (local_err) {
> -                error_report_err(local_err);
> -                exit(1);
> -            }
> +            validate_bootdevices(boot_once, &error_fatal);
>           }
>
>           boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 10:49 ` Paolo Bonzini
@ 2015-12-10 12:31   ` Markus Armbruster
  2015-12-10 13:41     ` Markus Armbruster
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2015-12-10 12:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-arm, Michael S. Tsirkin, qemu-devel, Eduardo Habkost

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/12/2015 11:19, Markus Armbruster wrote:
>> +    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
>> + &error_fatal);object_property_set_int(OBJECT(cpu),
>> + MPCORE_PERIPHBASE,
>> + "reset-cbar",
>> + &error_fatal);object_property_set_bool(OBJECT(cpu),
>> + true,
>> + "realized",
>> + &error_fatal);
>
> Something went wrong here. :)

Once again I demonstrate incompetence at proofreading my own patches %-}

No idea what happened.  I'll fix it.

>>  void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
>>                                  BlockBackend *value)
>>  {
>> -    Error *err = NULL;
>> -
>> -    qdev_prop_set_drive(dev, name, value, &err);
>> -    if (err) {
>> -        error_report_err(err);
>> -        exit(1);
>> -    }
>> +    qdev_prop_set_drive(dev, name, value, &error_fatal);
>>  }
>
>
> This should be inlined entirely into the callers (possibly as a follow up).

Yes, a FOO_nofail() wrapper around a FOO() taking Error ** is pointless.
I'll prep a follow-up patch.

> Otherwise looks great, thanks!

Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 11:32 ` Marcel Apfelbaum
@ 2015-12-10 12:34   ` Markus Armbruster
  2015-12-10 13:58     ` Marcel Apfelbaum
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2015-12-10 12:34 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, qemu-arm,
	marcel, Paolo Bonzini

Marcel Apfelbaum <marcel.apfelbaum@gmail.com> writes:

> On 12/10/2015 12:19 PM, Markus Armbruster wrote:
>> Done with this admittedly crude Coccinelle semantic patch with manual
>> burial of dead Error * variables squashed in:
>>
>>      @@
>>      identifier FUN;
>>      expression ERR, EC;
>>      @@
>>      -    FUN(&ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    FUN(&error_fatal);
>>      @@
>>      identifier FUN;
>>      expression ARG1, ERR, EC;
>>      @@
>>      -    FUN(ARG1, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    FUN(ARG1, &error_fatal);
>>      @@
>>      identifier FUN;
>>      expression ARG1, ARG2, ERR, EC;
>>      @@
>>      -    FUN(ARG1, ARG2, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    FUN(ARG1, ARG2, &error_fatal);
>>      @@
>>      identifier FUN;
>>      expression ARG1, ARG2, ARG3, ERR, EC;
>>      @@
>>      -    FUN(ARG1, ARG2, ARG3, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    FUN(ARG1, ARG2, ARG3, &error_fatal);
>>      @@
>>      identifier FUN;
>>      expression RET, ERR, EC;
>>      @@
>>      -    RET = FUN(&ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    RET = FUN(&error_fatal);
>>      @@
>>      identifier FUN;
>>      expression RET, ARG1, ERR, EC;
>>      @@
>>      -    RET = FUN(ARG1, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    RET = FUN(ARG1, &error_fatal);
>>      @@
>>      identifier FUN;
>>      expression RET, ARG1, ARG2, ERR, EC;
>>      @@
>>      -    RET = FUN(ARG1, ARG2, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    RET = FUN(ARG1, ARG2, &error_fatal);
>>      @@
>>      identifier FUN;
>>      expression RET, ARG1, ARG2, ARG3, ERR, EC;
>>      @@
>>      -    RET = FUN(ARG1, ARG2, ARG3, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>>      @@
>>      type T;
>>      identifier FUN, RET;
>>      expression ERR, EC;
>>      @@
>>      -    T RET = FUN(&ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    T RET = FUN(&error_fatal);
>>      @@
>>      type T;
>>      identifier FUN, RET;
>>      expression ARG1, ERR, EC;
>>      @@
>>      -    T RET = FUN(ARG1, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    T RET = FUN(ARG1, &error_fatal);
>>      @@
>>      type T;
>>      identifier FUN, RET;
>>      expression ARG1, ARG2, ERR, EC;
>>      @@
>>      -    T RET = FUN(ARG1, ARG2, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    T RET = FUN(ARG1, ARG2, &error_fatal);
>>      @@
>>      type T;
>>      identifier FUN, RET;
>>      expression ARG1, ARG2, ARG3, ERR, EC;
>>      @@
>>      -    T RET = FUN(ARG1, ARG2, ARG3, &ERR);
>>      -    if (ERR != NULL) {
>>      -        error_report_err(ERR);
>>      -        exit(EC);
>>      -    }
>>      +    T RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>>
>
> That's so cool!

I'm afraid my sledgehammer approach to Coccinelle would make its
inventors wince...

> Isn't it the time to have our own Coccinelle directory
> with scripts like this?

Could do that if there's interest.

>                         And to make them part of make check?

I'm afraid that's not practical.  spatch solves a difficult problem, and
takes its own sweet time to do it.

> Is a pity to have them lost into a git comment...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 12:31   ` Markus Armbruster
@ 2015-12-10 13:41     ` Markus Armbruster
  0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2015-12-10 13:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-arm, Michael S. Tsirkin, qemu-devel, Eduardo Habkost

Markus Armbruster <armbru@redhat.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> On 10/12/2015 11:19, Markus Armbruster wrote:
>>> +    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
>>> + &error_fatal);object_property_set_int(OBJECT(cpu),
>>> + MPCORE_PERIPHBASE,
>>> + "reset-cbar",
>>> + &error_fatal);object_property_set_bool(OBJECT(cpu),
>>> + true,
>>> + "realized",
>>> + &error_fatal);
>>
>> Something went wrong here. :)
>
> Once again I demonstrate incompetence at proofreading my own patches %-}
>
> No idea what happened.  I'll fix it.

Coccinelle bug.  If I append '//' to the relevant line in the semantic
patch, I get

+                            &error_fatal);//object_property_set_int(OBJECT(cpu),

Oopsie :)

[...]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 12:34   ` Markus Armbruster
@ 2015-12-10 13:58     ` Marcel Apfelbaum
  2015-12-10 14:16       ` Paolo Bonzini
  2015-12-10 14:32       ` Markus Armbruster
  0 siblings, 2 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2015-12-10 13:58 UTC (permalink / raw)
  To: Markus Armbruster, Marcel Apfelbaum
  Cc: Paolo Bonzini, qemu-arm, Michael S. Tsirkin, qemu-devel, Eduardo Habkost

On 12/10/2015 02:34 PM, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> writes:
>
>> On 12/10/2015 12:19 PM, Markus Armbruster wrote:
>>> Done with this admittedly crude Coccinelle semantic patch with manual
>>> burial of dead Error * variables squashed in:
>>>
>>>       @@
>>>       identifier FUN;
>>>       expression ERR, EC;
>>>       @@
>>>       -    FUN(&ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    FUN(&error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression ARG1, ERR, EC;
>>>       @@
>>>       -    FUN(ARG1, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    FUN(ARG1, &error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression ARG1, ARG2, ERR, EC;
>>>       @@
>>>       -    FUN(ARG1, ARG2, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    FUN(ARG1, ARG2, &error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression ARG1, ARG2, ARG3, ERR, EC;
>>>       @@
>>>       -    FUN(ARG1, ARG2, ARG3, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    FUN(ARG1, ARG2, ARG3, &error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression RET, ERR, EC;
>>>       @@
>>>       -    RET = FUN(&ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    RET = FUN(&error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression RET, ARG1, ERR, EC;
>>>       @@
>>>       -    RET = FUN(ARG1, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    RET = FUN(ARG1, &error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression RET, ARG1, ARG2, ERR, EC;
>>>       @@
>>>       -    RET = FUN(ARG1, ARG2, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    RET = FUN(ARG1, ARG2, &error_fatal);
>>>       @@
>>>       identifier FUN;
>>>       expression RET, ARG1, ARG2, ARG3, ERR, EC;
>>>       @@
>>>       -    RET = FUN(ARG1, ARG2, ARG3, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>>>       @@
>>>       type T;
>>>       identifier FUN, RET;
>>>       expression ERR, EC;
>>>       @@
>>>       -    T RET = FUN(&ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    T RET = FUN(&error_fatal);
>>>       @@
>>>       type T;
>>>       identifier FUN, RET;
>>>       expression ARG1, ERR, EC;
>>>       @@
>>>       -    T RET = FUN(ARG1, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    T RET = FUN(ARG1, &error_fatal);
>>>       @@
>>>       type T;
>>>       identifier FUN, RET;
>>>       expression ARG1, ARG2, ERR, EC;
>>>       @@
>>>       -    T RET = FUN(ARG1, ARG2, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    T RET = FUN(ARG1, ARG2, &error_fatal);
>>>       @@
>>>       type T;
>>>       identifier FUN, RET;
>>>       expression ARG1, ARG2, ARG3, ERR, EC;
>>>       @@
>>>       -    T RET = FUN(ARG1, ARG2, ARG3, &ERR);
>>>       -    if (ERR != NULL) {
>>>       -        error_report_err(ERR);
>>>       -        exit(EC);
>>>       -    }
>>>       +    T RET = FUN(ARG1, ARG2, ARG3, &error_fatal);
>>>
>>
>> That's so cool!
>
> I'm afraid my sledgehammer approach to Coccinelle would make its
> inventors wince...
>
>> Isn't it the time to have our own Coccinelle directory
>> with scripts like this?
>
> Could do that if there's interest.
>
>>                          And to make them part of make check?
>
> I'm afraid that's not practical.  spatch solves a difficult problem, and
> takes its own sweet time to do it.

So it takes a long time to run. We could make it depend on an environment variable,
so at least the maintainers will run it :)

My point is, now we *could* have a guarantee that if anyone uses the old
way, we can catch it in time. It can be easily lost in the review process.

Anyway, it was only a thought.
Thanks,
Marcel


>
>> Is a pity to have them lost into a git comment...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 13:58     ` Marcel Apfelbaum
@ 2015-12-10 14:16       ` Paolo Bonzini
  2015-12-10 14:32       ` Markus Armbruster
  1 sibling, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2015-12-10 14:16 UTC (permalink / raw)
  To: Marcel Apfelbaum, Markus Armbruster, Marcel Apfelbaum
  Cc: qemu-arm, Michael S. Tsirkin, qemu-devel, Eduardo Habkost



On 10/12/2015 14:58, Marcel Apfelbaum wrote:
> My point is, now we *could* have a guarantee that if anyone uses the old
> way, we can catch it in time. It can be easily lost in the review process.

The right way to do that would be in scripts/checkpatch.pl, by catching
error_report_err followed by exit.

Paolo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors
  2015-12-10 13:58     ` Marcel Apfelbaum
  2015-12-10 14:16       ` Paolo Bonzini
@ 2015-12-10 14:32       ` Markus Armbruster
  1 sibling, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2015-12-10 14:32 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, qemu-arm,
	Marcel Apfelbaum, Paolo Bonzini

Marcel Apfelbaum <marcel@redhat.com> writes:

> On 12/10/2015 02:34 PM, Markus Armbruster wrote:
>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> writes:
>>
>>> On 12/10/2015 12:19 PM, Markus Armbruster wrote:
>>>> Done with this admittedly crude Coccinelle semantic patch with manual
>>>> burial of dead Error * variables squashed in:
[...]
>>>
>>> That's so cool!
>>
>> I'm afraid my sledgehammer approach to Coccinelle would make its
>> inventors wince...
>>
>>> Isn't it the time to have our own Coccinelle directory
>>> with scripts like this?
>>
>> Could do that if there's interest.
>>
>>>                          And to make them part of make check?
>>
>> I'm afraid that's not practical.  spatch solves a difficult problem, and
>> takes its own sweet time to do it.
>
> So it takes a long time to run. We could make it depend on an
> environment variable,
> so at least the maintainers will run it :)
>
> My point is, now we *could* have a guarantee that if anyone uses the old
> way, we can catch it in time. It can be easily lost in the review process.

In my experience, to reduce recurrence of an unwanted code pattern, you
first need to get rid of the bad examples in the tree.  Without that,
it's basically futile.

For the ones that keep coming back, further steps may be in order, such
as making checkpatch complain.

> Anyway, it was only a thought.

It's not without merit.

>>> Is a pity to have them lost into a git comment...

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-12-10 14:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-10 10:19 [Qemu-devel] [PATCH] Use error_fatal to simplify obvious fatal errors Markus Armbruster
2015-12-10 10:49 ` Paolo Bonzini
2015-12-10 12:31   ` Markus Armbruster
2015-12-10 13:41     ` Markus Armbruster
2015-12-10 11:32 ` Marcel Apfelbaum
2015-12-10 12:34   ` Markus Armbruster
2015-12-10 13:58     ` Marcel Apfelbaum
2015-12-10 14:16       ` Paolo Bonzini
2015-12-10 14:32       ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.