Cppcheck: Reduction of False Positives with a MSVC Project File

classic Classic list List threaded Threaded
26 messages Options
12 « Prev
Jan-Marek Glogowski Jan-Marek Glogowski
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

Hi Luke,

Am October 25, 2018 12:40:33 AM UTC schrieb Luke Benes <[hidden email]>:
>In my first attempt to improve the quality of the cppcheck reports,
>Tamás Zolnai pointed out that including every possible header resulted
>in some valid warnings not being reported.

[snip]

>It seems many valid variableScope warnings are still being omitted. I'm
>still looking into that. Are there any other categories of valid errors
>that are missing? Specific examples would be helpful.
>
>Overall, does this report have a higher signal-to-noise ratio than our
>current weekly report?

No idea, you're the expert here - probably that's easier to evaluate in comparison to your original, even shorter report.

Why not have two reports? If your final report has much less false positives and probably even generally with a higher error severity (variableScope normally doesn't indicate an error, can it?), then people can concentrate on these first.

Now I don't know how long it takes to generate them, but one can still toggle between them.

Jan-Marek
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Michael Stahl-3 Michael Stahl-3
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
On 25.10.18 02:40, Luke Benes wrote:
> It seems many valid variableScope warnings are still being omitted.

those warnings are quite dangerous anyway if naively believed, tdf#96089
was quite a pain to debug...
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Tamás Zolnai Tamás Zolnai
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by Jan-Marek Glogowski
Hi Luke, all

It does not matter that variableScope is a low priority issue or a can be dangerous . If someone want to disable a check it can be disabled explictiely (I guess).
The main point that this change seems to simply reduce the scope of cppcheck. If this is the purpose then we can just run cppcheck on an empty file and so we won't see any issue (all false positives will disappear).

>Overall, does this report have a higher signal-to-noise ratio than our
>current weekly report?

@Luke
You do ask it? Was it not you how came up with the idea to reduce the false positives with specifying the includes? Now you are asking others what the result of your change. I'm sure it used to happen in the opposite direction.

It does not make any sense to have two reports if the only difference is that first report contains less bugs than the other, if the filtering has no logical meaning.

Best Regards,
Tamás

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

> Was it not you how came up with the idea to reduce the false positives with specifying the includes?

 

No, it was not my idea. On #cppcheck, I was told by danmar, the primary developer of cppcheck, that our script is using cppcheck incorrectly. Without being passed the same include locations as we pass the compiler, we should expect a large amount of garbage.

 

In fact, according to the developer, we should not get any False Postives if we call cppcheck correctly. He encouraged me to file bug reports for any FP that remain, once cppcheck is being run properly.  

 

> The main point that this change seems to simply reduce the scope of cppcheck. If this is the purpose then we can just run cppcheck on an empty file and so we won't see any issue (all false positives will disappear).

 

Again, No my goal is to improve the Signal-to-noise. FPs can be dangerous as in tdf#96089 and make it much harder to spot real issues.

 

Currently, I am in the process of comparing old cppcheck fixes with and without the '-Iinclude' option.  So far, the three that I have checked would not be filtered out. In other words, had we been calling cppcheck the way I propose, these issues would have been much easier for developers to spot(4000 vs 500).


-Luke

 

 

 



_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Kaganski Mike Kaganski Mike
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

Hi Luke,

I believe that continuing the "manual approach" is moot. It was
mentioned that the correct way is to generate the proper project
structure for cppcheck using our existing tools that generate IDE
integrations (and they have correct include correct includes, deps and all).

--
Best regards,
Mike Kaganski
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by Tamás Zolnai
> It was mentioned that the correct way is to generate the proper project
> structure for cppcheck using our existing tools that generate IDE
> integrations (and they have correct include correct includes, deps and all).

Mike,
Yes, agreed. But AFAIK, no one other than me is looking into this. Quite a few issues with the GbuildToIDE approach need to be solved.

In the meantime, our current cppcheck script generates tens of thousands of config errors (if run with check-config or verbose),reports hundred or possibly thousands of false positives, and is being called with depreciated parameters. These issues can be fixed now.

I am in the process of going through past cppcheck related commits to verify how many, if any at all, would be missed had we been using the '-I include/' parameter.  So far, I have not identified any. If the consensus is that you do want to improve the script with the manual approach, I will not continue testing. Please advise.

However, keep in mind that from the GbuildToJson output, any framework that is built in the future will also call cppcheck with the '-I include/' parameter. Therefore, if there is a bug with cppcheck not reporting valid errors when called this way, then that approach too will suffer too.

 



_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
12 « Prev