[webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt

[Maciej Stachowiak]

I like having static analysis results available - that seems useful.

I think static analysis tools are more useful when we can set a blanket policy of fixing all warnings they report. For example, our warning levels combined with -Werror are a very strong tool in this regard, despite the limited coverage. We can tweak the warning flags to only identify warnings we think outweigh the cost.

I am not sure if Coverity can become this type of tool. As others have mentioned, past patches fixing the issues it reports have at times not been net improvements to the code (in that the issue was a false positive, and the resulting code was less readable).

Does Coverity have a way to adjust what issues it reports? If so, then starting with a consensus set that the WebKit community agrees are generally worthwhile would make for a very powerful tool. If it does not, then I suspect we could only use it for informational purposes, and not as a "get failures down to zero" tool, which would significantly diminish the value. We would also have to be clear in that case that "coverity reports a problem" is not, by itself, a sufficient justification for a code change.

With those caveats, I like the idea of making the report available to everyone. At the very least, it would let patch reviewers see Coverity's complaints in context.

Regards,
Maciej

On Sep 17, 2012, at 4:11 PM, James Hawkins <jhawkins at chromium.org> wrote:

> Hey folks,
> 
> TL;DR - If you have opinions one way or another about having a Coverity instance available for WebKit developers, please respond to this message.
> 
> 
> Coverity is a static analysis tool [1] which scans source code and reports defects in the code.  We've been using Coverity to find defects in Chrome for a while now, and though there is sometimes a bit of subjectivity involved in the defect types (e.g. whether a return value should be checked), the signal is generally high.
> 
> Off the top of my head, the following are the defects I spend most of my time fixing:
> * Uninitialized variables (including member variables).
>   - Chrome has had at least 4 crash fixes in the past few months due to this defect (which were caught by Coverity).
> * Passing large parameters by value.
>   - Generally a trivial fix.  I don't have performance data to say what affect fixing these hash, but 'death by a thousand cuts' eh?
> * Forward/Reverse/I - Nulls.
>   - Coverity is very good at understanding when a value is NULL and the tool will tell you which code paths are using a NULL value.
> * Tons of security issue-causing defects.
> 
> 
> I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal.
> 
> A few details:
> * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure.
> * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes).
> 
> The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects.  Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine).
> 
> Let me know if you love the idea or hate it.
> 
> Thanks,
> James
> 
> 
> [1] http://www.coverity.com/products/static-analysis.html
> [2] http://softwareintegrity.coverity.com/coverity-scan-2011-open-source-integrity-report-registration.html - registration required now :(
> 
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120917/5c77894b/attachment.html>