[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH for 2.10 21/35] arm/sysbus-fdt: fix n
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH for 2.10 21/35] arm/sysbus-fdt: fix null pointer dereference |
Date: |
Tue, 29 May 2018 11:33:07 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
Hi Eric,
On 07/24/2017 06:57 PM, Eric Blake wrote:
> On 07/24/2017 04:52 PM, Eric Blake wrote:
>> On 07/24/2017 04:48 PM, Philippe Mathieu-Daudé wrote:
>>> On 07/24/2017 06:09 PM, Peter Maydell wrote:
>>>> On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <address@hidden> wrote:
>>>>> Use error_report() + exit() instead of error_setg(&error_fatal).
>>>>>
>>>>> hw/arm/sysbus-fdt.c:322:9: warning: Array access (from variable
>>>>> 'node_path') results in a null pointer dereference
>>>>> if (node_path[1]) {
>>>>> ^~~~~~~~~~~~
>>>>
>>>> I don't understand what this warning is trying to say.
>>>> We can't get to this point with a NULL node_path,
>>>> because of the previous conditional, which is using
>>>> error_setg(&error_fatal).
>>>
>>> Ok I see, Clang is unaware than error_setg(&error_fatal) is a noreturn.
>>
>> Indeed, and that's because error_setg(&error_fatal) is not in preferred
>> form.
>>
>>>
>>> Patch dropped.
>>
>> That's a shame. Rather, we should patch this file (and others) to avoid
>> all the inconsistent uses of error_setg(&error_*), to comply with the
>> error.h documentation.
I started to port/clean this up.
To avoid future inconsistencies, do you think we should/can enforce this
check in checkpatch (which is stricter than human review)?
Is the "Qemu error function tests" section a good place to put this check?
>
> In other words, switching to the preferred spelling in the following files:
> device_tree.c
> hw/arm/sysbus-fdt.c
> hw/block/fdc.c
> hw/ppc/spapr_drc.c
>
> is desirable, and has the added benefit of also silencing a Coverity
> false positive. But it should be done in terms of switching to the
> preferred spelling, as it touches more instances than just the one that
> shuts up Coverity.
>
- Re: [Qemu-arm] [Qemu-devel] [PATCH for 2.10 21/35] arm/sysbus-fdt: fix null pointer dereference,
Philippe Mathieu-Daudé <=