When Cppcheck runs over this code,[1] it complains about an error:
void bool_express(bool aPre, bool aX, bool aPost)
{
bool x;
const bool pre = aPre;
if (pre) {
x = aX;
}
const bool post = aPost;
// error checking passage of the original code:
if ( !pre || !x || !post ) {
if ( !pre ) {
trace("pre failed");
} else if ( !x ) { // <-- HERE Cppcheck complains
trace("x failed");
} else {
trace("post failed");
}
} else {
// success passage of the original code:
trace("ok");
}
}
This is the message, that makes me nervous:
Id: uninitvar
Summary: Uninitialized variable: x
Message: Uninitialized variable: x
I think it's a false positive, but I admit that this may not be obvious. Nevertheless I don't want to touch that code, because it's old and a lot heavier than this extracted version.
Have you ever experienced situations like this? How to deal with it?
[1] This code is reduced to its bones, the original code establishes a precondition (pre
), does something (x
), then forces a postcondition (post
), after that, some error conditions are checked. I transformed the runtime-results of the functions that are called and then stored in pre
, x
, and post
into the 3 arguments of my test case.
It is a false positive in Cppcheck. I solved it by adding an inline suppression:[1]
and I also brought it to the attention of the Cppcheck developers:
#7663 (False positive: uninitialised variable in unreachable code)
[1] I decided not to initialize the variable. This is not for performance reasons, but to be informed about the bug being fixed in a future release, when Cppcheck will say
If your
pre
condition isfalse
than x will be uninitialized. At lineif ( !x )
CppCheck warns about usage of indeterminate value. To fix it initializex
variable.The static analysis appears to be complaining because if
pre
is false, thenx
is never set.Your code is structured such that the value of
x
is never accessed ifpre
is false - I'd argue that the static analyser isn't giving a useful output in this case.Enumerating the various cases we have (so we can be reasonably sure that it's cppcheck and not us!):
The first statement in which
x
is accessed is in the lineif ( !pre || !x || !post )
- due to short-circuiting evaluation:if( A || B || C )
doesn't evaluateB
orC
ifA
is true; hence we never try to read an uninitialisedx
(sincex
is only uninitialised ifpre
is false, in which case we stopped evaluated the expression!)The second usage is in
if ( !pre ) { trace("pre failed"); } else if ( !x ) { // <-- HERE Cppcheck complains
Again, we can only hit the offending line if
pre
was true (in which casex
is properly initialised).From this, we can conclude that either:
The actual code mistakenly tries to read
x
in some condition even ispre
is false, and you've missed it when building the example (sometimes the logical flow of a program can be a bit obtuse)The static analyser is lazy and spots the line
else if( !x )
and can't determine if this line is reachable with an uninitialised value.From the code you've provided, you shouldn't be concerned: the static analysis tool is technically correct that
x
can be uninitialised, but in those cases it's not used (and hence probably shouldn't be warning you).I'd recommend assigning
x
a default value if you're not confident, or if the actual logic is exceedingly obtuse.