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
