[webkit-dev] Eliminate potential null pointer dereference?

[Kentaro Hara]

+1 to the idea that we should try to add a test for each change. That
being said, as rniwa mentioned, it is sometimes difficult to make a
test because

(A) we do not come up with a test case
(B) we know that the code is unreachable (e.g.
https://bugs.webkit.org/show_bug.cgi?id=84377)

What is the consensus for such cases? As long as the change improves
codebase and the benefit is explicitly described in ChangeLog, is it
OK to commit the patch? Or shouldn't we try to fix a "potential" bug
observed in static analysis?



On Fri, Apr 20, 2012 at 11:40 AM, Benjamin Poulain <benjamin at webkit.org> wrote:
> On Fri, Apr 20, 2012 at 10:53 AM, Luke Macpherson
> <macpherson at chromium.org> wrote:
>> What do we hope to achieve by adding a test when fixing a bug? To
>> prevent a bug from being reintroduced into the codebase at a later
>> date. This is a reasonable goal, so let’s remember that the goal is to
>> prevent the bug from recurring, not to add a test for its own sake. In
>> this case, the potential null pointer dereference was found using
>> coverity, a static analysis tool that we run nightly. If the bug were
>> to be reintroduced it is reasonable to expect that static analysis
>> would be able to find it again.
>
> I remember seeing similar patch based on Coverty, and when asked for a
> test, the author discovered the null check was useless in that place
> and the code was changed differently.
>
> I wholeheartedly agree we should try to make tests for code changed
> based on static code analyzer. This will help making sure the change
> is meaningful, and the test would prevent regression.
>
> Benjamin



-- 
Kentaro Hara, Tokyo, Japan (http://haraken.info)