On 24 July 2017 at 22:20, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > On 07/24/2017 06:09 PM, Peter Maydell wrote: >> >> On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > ... > >>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c >>> index d68e3dcdbd..ad0cc49b19 100644 >>> --- a/hw/arm/sysbus-fdt.c >>> +++ b/hw/arm/sysbus-fdt.c >>> @@ -315,15 +315,14 @@ static int add_amd_xgbe_fdt_node(SysBusDevice >>> *sbdev, void *opaque) >>> node_path = qemu_fdt_node_path(host_fdt, dt_name, vdev->compat, >>> &error_fatal); >>> if (!node_path || !node_path[0]) { >>> - error_setg(&error_fatal, "%s unable to retrieve node path for >>> %s/%s", >>> + error_report("%s unable to retrieve node path for %s/%s", >>> __func__, dt_name, vdev->compat); >>> - } >>> - >>> - if (node_path[1]) { >>> - error_setg(&error_fatal, "%s more than one node matching >>> %s/%s!", >>> + exit(1); >>> + } else if (node_path[1]) { >>> + error_report("%s more than one node matching %s/%s!", >>> __func__, dt_name, vdev->compat); >>> + exit(1); >>> } >>> - >>> g_free(dt_name); >> >> >> This doesn't seem like an improvement. Now the >> error handling in the function is an inconsistent >> mix of error_report()+exit() and error_setg(&error_fatal). > > > I got this from "qapi/error.h": > > 156 * Please don't error_setg(&error_fatal, ...), use error_report() and > 157 * exit(), because that's more obvious. > 158 * Likewise, don't error_setg(&error_abort, ...), use assert(). > > Is this comment outdated?
Probably not, I expect this code predates it. However my point about inconsistency still stands. thanks -- PMM