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 <jhawk...@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@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to