[webkit-dev] Eliminate potential null pointer dereference?

Rachel Blum groby at chromium.org
Fri Apr 20 13:50:10 PDT 2012


>
> I completely agree with Maciej here that if this is a reachable code, then
> the patch author should put a reasonable effort into creating a test case. And
> most importantly, these changes are clearly not "code cleanup".
>

I'm disagreeing here. (as far as NULL checks go).

Unless there's a demonstrable reason that you _need_ a value uninitialized,
why is the burden of proof on the person doing cleanup? Yes, at the point
the code was written, it's well possible that the author was aware that the
value would always be initialized for use. However, if code is added to a
class, that invariant is not always checked again.

I think the confusion is over the intent of the person making the cleanup
change. We (I speak as one of the people pushing static analysis) are not
interested in *changing* WebKit behavior. We're not fixing existing bugs.
We're interested in making sure behavior is deterministic. Requiring the
construction of what amounts to an exploit for each fix for uninitialized
variables seems a bit overkill :)

I agree that the CHANGELOG entry should state that we deliberately didn't
add tests. My personal policy is to propose those patches, complete with
"No new tests/ cleanup only". If I get a lot of pushback on the review, I'm
happy to abandon it.

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


More information about the webkit-dev mailing list