Re: [libmicrohttpd] Clang static analyzer reports...

[Tim Rühsen]

Hi Christian,
On 2/3/20 3:44 PM, Christian Grothoff wrote:
> Hi Tim,
> 
> Thanks for forwarding the report. I've looked through them. The first
> two clang is pissy because we don't annotate with 'nonnull'.  Doing so
> *consistently* would be a lot of work, if someone wants to do so,
> great, but I won't for the near future.

Not exactly, clang is pissy because you explicitly give NULL as argument
to a non-null annotated function argument.

> The "logic error" (NULL dereference) looks very much like a logic error
> in CLANG. It boils down to:
> 
> 
> state = 42;
> ptr = NULL;
> switch (state)
> {
> case 42:
>   perfectly safe;
>   break;
> case 44:
>   deref ptr;
>   break;
> }
> 
> and clang goes for the wrong case (44) even though 42 was just set 5
> statements above.  So yes, a logic error, but in clang ;-).

The 'state = 42' is set somewhere outside the function with the switch
(MHD_connection_handle_idle).

From looking at the function, you (at least I) can't say if 'state' is
always set to 42 before calling it. Even if this is the case right now -
this sounds like a pitfall for any developer who is not 100% firm with
the code.

It might be a matter of favor to clean this up or not. An alternative is
a clang analyzer suppression file. Keeping the status like it is
effectively disables automatic flaw detection by the CI - in this case I
would simply disable/remove the runner.

Regards, Tim

> 
> Happy hacking!
> 
> Christian     
> 
> On Mon, 2020-02-03 at 15:33 +0100, Tim Rühsen wrote:
>> Hi Christian,
>>
>> there is no online version. I regenerated the report and attached it.
>> Unpack and view it with
>>
>> tar xf scan-build.tgz
>> xdg-open scan-build/index.html
>>
>> And sorry that I can't fix it myself (out of time currently).
>>
>> Regards, Tim
>>
>> On 2/3/20 3:18 PM, Christian Grothoff wrote:
>>> Could you please send the link to the report?
>>>
>>> On Sun, 2020-02-02 at 12:23 +0100, Tim Rühsen wrote:
>>>> Hi,
>>>>
>>>> with clang 8.0.1 static analyzer you'll get "scan-build: 8 bugs
>>>> found."
>>>> on latest master (b8a0f69866a82ae186cfaba955bba5e3f8d9df3e).
>>>>
>>>> CC=clang ./configure
>>>> scan-build --use-cc=clang make -j$(nproc)
>>>>
>>>> Regards, Tim
>>>>
> 
>

signature.asc
Description: OpenPGP digital signature