On Feb 23, 2011, at 5:52 AM, Steffen DETTMER wrote:
> (OT)
>
> * Jeff Morriss wrote on Thu, Feb 17, 2011 at 09:27 -0500:
>
>> I imagine there are going to be a LOT of "if (a)" checks in 2 million
>> LOC. Do we really want to change them all?
>
> shouldn't the number of such problems reported by the static code
> analyzer match the number of places to be changed?
Yes, and the problem, it turns out, isn't with "if (a)" checks, it's with "if (a && {something else})" checks. Gerald changed the nmake files to turn off the "dereferencing a null pointer" checks to avoid that particular MSVC++ bug. (I don't think the Clang static analyzer has this problem.)
> (personally, I think in general it can be quite doubtful to add
> some if (p != NULL) around *p, maybe p was checked before or was
> initilized with some p = &o or whatever.
A non-buggy static analyzer would detect that and not warn about it. Even the MSVC++ analyzer can handle at least some instances of that.
> I would be afraid that
> such warning make some people to blindly add if (p != NULL) with
> the possible result to make the app misbehave in a way hard to
> find instead of crashing quick, straight-forward, direct and
> clean, which is easy to debug. We even have a developer rule
> usually NOT to use things like `if (p)' or `if (p != NULL)' [for
> mandatory pointers, but assert(p) should be used] because
> pointers cannot be checked; *p also crashes when already freed or
> uninitialized or whatever - comments appreciated :)).
If by "mandatory pointer" you mean "pointer that must not be null" - e.g., if you have a routine that takes a pointer to an XXX and does something with that XXX, so the pointer is expected not to be null - either
1) the static analyzer will deduce that a pointer is mandatory and will warn, for example, about a call to the routine in question that passes it a null pointer, so the fix should be not to pass a null pointer to it (so that you have an even-easier-to-debug-than-a-crash *build*-time error; run-time errors aren't always that easy to debug...)
or
2) if it can't or won't do that, the developer should, if the analyzer supports it, decorate the routine with a "this pointer must not be null" argument, to let the static analyzer do the check in question.
