[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/4] hw/arm/sysbus-fdt: Replace error_setg(&e
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/4] hw/arm/sysbus-fdt: Replace error_setg(&error_fatal) by error_report() + exit() |
Date: |
Thu, 21 Jun 2018 08:14:03 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
Hi Markus,
On 06/08/2018 03:27 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
>
>> Use error_report() + exit() instead of error_setg(&error_fatal),
>> as suggested by the "qapi/error.h" documentation:
>>
>> Please don't error_setg(&error_fatal, ...), use error_report() and
>> exit(), because that's more obvious.
>>
>> This fixes CID 1352173:
>> "Passing null pointer dt_name to qemu_fdt_node_path, which dereferences
>> it."
>>
>> And this also fixes:
>>
>> hw/arm/sysbus-fdt.c:322:9: warning: Array access (from variable
>> 'node_path') results in a null pointer dereference
>> if (node_path[1]) {
>> ^~~~~~~~~~~~
>>
>> Fixes: Coverity CID 1352173 (Dereference after null check)
>
> You lost
>
> Suggested-by: Eric Blake <address@hidden>
>
> Intentional?
Not at all :/
>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>> hw/arm/sysbus-fdt.c | 42 +++++++++++++++++++++++-------------------
>> 1 file changed, 23 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
>> index e4c492ea44..8e2784fa11 100644
>> --- a/hw/arm/sysbus-fdt.c
>> +++ b/hw/arm/sysbus-fdt.c
>> @@ -91,7 +91,7 @@ static void copy_properties_from_host(HostProperty *props,
>> int nb_props,
>> r = qemu_fdt_getprop(host_fdt, node_path,
>> props[i].name,
>> &prop_len,
>> - props[i].optional ? &err : &error_fatal);
>> + &err);
>> if (r) {
>> qemu_fdt_setprop(guest_fdt, nodename,
>> props[i].name, r, prop_len);
>> @@ -102,6 +102,7 @@ static void copy_properties_from_host(HostProperty
>> *props, int nb_props,
>> } else {
>> error_free(err);
>> }
>> + assert(props[i].optional); /* mandatory property not found */
>> }
>> }
>> }
>
> This is not the conversion the commit message promised: it replaces
> exit(1) by abort(). Why?
I can't understand it either, I suppose I cherry-picked from an
incorrect branch, because I also added your R-b locally.
I'll resend.
Thanks,
Phil.
- Re: [Qemu-devel] [PATCH v2 1/4] hw/block/fdc: Replace error_setg(&error_abort) by assert(), (continued)
[Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr_drc: Replace error_setg(&error_abort) by abort(), Philippe Mathieu-Daudé, 2018/06/07
[Qemu-devel] [PATCH v2 3/4] hw/arm/sysbus-fdt: Replace error_setg(&error_fatal) by error_report() + exit(), Philippe Mathieu-Daudé, 2018/06/07
[Qemu-devel] [PATCH v2 4/4] device_tree: Replace error_setg(&error_fatal) by error_report() + exit(), Philippe Mathieu-Daudé, 2018/06/07