walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix #7803: false negative: condition is always true 'if (flags & A)' (#938)

This commit is contained in:
Nekto89 2017-09-08 15:30:42 +03:00 committed by Daniel Marjamäki
parent bddc698295
commit 9cbfa0e383
2 changed files with 41 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/checkcondition.cpp
View File

@ -1151,12 +1151,6 @@ void CheckCondition::alwaysTrueFalse()
const Scope * scope = symbolDatabase->functionScopes[i]; const Scope * scope = symbolDatabase->functionScopes[i];
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
const bool constValCond = Token::Match(tok->tokAt(-2), "if|while ( %num%|%char% )") && !Token::Match(tok,"0|1"); // just one number or char inside if|while
const bool constValExpr = Token::Match(tok, "%num%|%char%") && tok->astParent() && Token::Match(tok->astParent(),"&&|%oror%|?"); // just one number or char in boolean expression
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
if (!(constValCond || constValExpr || compExpr))
continue;
if (tok->link()) // don't write false positives when templates are used if (tok->link()) // don't write false positives when templates are used
continue; continue;
if (!tok->hasKnownIntValue()) if (!tok->hasKnownIntValue())
@ -1164,6 +1158,16 @@ void CheckCondition::alwaysTrueFalse()
if (Token::Match(tok, "[01]")) if (Token::Match(tok, "[01]"))
continue; continue;
const bool constIfWhileExpression =
tok->astParent()
&& Token::Match(tok->astParent()->astOperand1(), "if|while")
&& !tok->isBoolean();
const bool constValExpr = Token::Match(tok, "%num%|%char%") && tok->astParent() && Token::Match(tok->astParent(),"&&|%oror%|?"); // just one number or char in boolean expression
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
if (!(constIfWhileExpression || constValExpr || compExpr))
continue;
// Don't warn in assertions. Condition is often 'always true' by intention. // Don't warn in assertions. Condition is often 'always true' by intention.
// If platform,defines,etc cause 'always false' then that is not dangerous neither. // If platform,defines,etc cause 'always false' then that is not dangerous neither.
bool assertFound = false; bool assertFound = false;

32
test/testcondition.cpp
View File

@ -97,6 +97,7 @@ private:
TEST_CASE(alwaysTrue); TEST_CASE(alwaysTrue);
TEST_CASE(checkInvalidTestForOverflow); TEST_CASE(checkInvalidTestForOverflow);
TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
} }
void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
@ -355,7 +356,7 @@ private:
" g(x);\n" " g(x);\n"
" }\n" " }\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'x' is always true\n", errout.str());
check("void g(int & x);\n" check("void g(int & x);\n"
"void f() {\n" "void f() {\n"
@ -1935,6 +1936,7 @@ private:
" if (x & 3 == 2) {}\n" " if (x & 3 == 2) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n" ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n"
"[test.cpp:2]: (style) Condition 'x&3==2' is always false\n"
"[test.cpp:2]: (style) Condition '3==2' is always false\n", errout.str()); "[test.cpp:2]: (style) Condition '3==2' is always false\n", errout.str());
check("void f() {\n" check("void f() {\n"
@ -2153,6 +2155,7 @@ private:
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n" ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n"
"[test.cpp:3]: (style) Condition 'L'b'' is always true\n" "[test.cpp:3]: (style) Condition 'L'b'' is always true\n"
"[test.cpp:4]: (style) Condition '1&&'c'' is always true\n"
"[test.cpp:4]: (style) Condition ''c'' is always true\n" "[test.cpp:4]: (style) Condition ''c'' is always true\n"
"[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str()); "[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str());
} }
@ -2188,6 +2191,33 @@ private:
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void checkConditionIsAlwaysTrueOrFalseInsideIfWhile() {
check("void f() {\n"
" enum states {A,B,C};\n"
" const unsigned g_flags = B|C;\n"
" if(g_flags & A) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'g_flags&A' is always false\n", errout.str());
check("void f() {\n"
" int a = 5;"
" if(a) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a' is always true\n", errout.str());
check("void f() {\n"
" int a = 5;"
" while(a + 1) { a--; }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int a = 5;"
" while(a + 1) { return; }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
}
}; };
REGISTER_TEST(TestCondition) REGISTER_TEST(TestCondition)