[webkit-dev] Eliminate potential null pointer dereference?

[Ryosuke Niwa]

On Fri, Apr 20, 2012 at 10:53 AM, Luke Macpherson
<macpherson at chromium.org>wrote:
>
> Tests are a good thing, but they are not the only thing. Consider the
> state-space of a large piece of software like webkit - it is
> essentially infinite. You can’t test every case and code path to
> ensure correctness.


While I do understand where you're coming from, this is an agreed
policy. We should state why tests are absent in change logs or in bugs when
it's hard to create one putting reasonable efforts into creating one.

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.


Is the code reachable? It's quite possible that the code is unreachable and
therefore there is no way to hit that crash. Without a test, we can't
answer that question.

If the bug were to be reintroduced it is reasonable to expect that static
> analysis would be able to find it again.


WebKit contributors are not required to use such a tool prior to committing
their changes at least for now, but we DO require contributors to run our
layout tests. And I don't think we want to require all contributors to run
coverity on webkit before committing their patches. Given that, there
is definitely a benefit in adding a test case for simple fixes like this.

On the other hand, I don't think it's realistic to force contributors to
come up with a test case if it's really hard since that could become a
significant development overhead as well. And in those cases where we
decide that it's too hard to create a test, we explicitly mention it in the
change log. It is this piece the particular patch ap brought up is missing.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120420/d465a17e/attachment.html>