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

AssignIf: Better handling of complex conditions with multiple subconditions

This commit is contained in:
Daniel Marjamäki 2012-09-28 17:03:16 +02:00
parent 7362ca9e8f
commit bb62325ddb
3 changed files with 49 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
lib/checkassignif.cpp
View File

@ -54,29 +54,39 @@ void CheckAssignIf::assignIf()
for (const Token *tok2 = tok->tokAt(4); tok2; tok2 = tok2->next()) { for (const Token *tok2 = tok->tokAt(4); tok2; tok2 = tok2->next()) {
if (tok2->str() == "(" || tok2->str() == "}" || tok2->str() == "=") if (tok2->str() == "(" || tok2->str() == "}" || tok2->str() == "=")
break; break;
if (Token::Match(tok2,"if ( (| %varid% %any% %num% &&|%oror%|)", varid)) { if (Token::Match(tok2, "if|while|for (")) {
const Token *vartok = tok2->tokAt(tok2->strAt(2)=="(" ? 3 : 2); // parse condition
const std::string& op(vartok->strAt(1)); const Token * const end = tok2->next()->link();
const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2)); for (; tok2 != end; tok2 = tok2->next()) {
std::string condition; if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid))
if (Token::simpleMatch(tok2, "if ( (")) break;
condition = "'" + vartok->str() + op + vartok->strAt(2) + "'"; if (Token::Match(tok2,"&&|%oror%|( %varid% %any% %num% &&|%oror%|)", varid)) {
if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num)) const Token *vartok = tok2->next();
assignIfError(tok2, condition, false); const std::string& op(vartok->strAt(1));
else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num)) const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2));
assignIfError(tok2, condition, true); const std::string condition(vartok->str() + op + vartok->strAt(2));
break; if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num))
assignIfError(tok, tok2, condition, false);
else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num))
assignIfError(tok, tok2, condition, true);
}
}
} }
} }
} }
} }
} }
void CheckAssignIf::assignIfError(const Token *tok, const std::string &condition, bool result) void CheckAssignIf::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result)
{ {
reportError(tok, Severity::style, std::list<const Token *> locations;
locations.push_back(tok1);
locations.push_back(tok2);
reportError(locations,
Severity::style,
"assignIfError", "assignIfError",
"Mismatching assignment and comparison, comparison " + condition + (condition.empty()?"":" ") + "is always " + std::string(result ? "true" : "false") + "."); "Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + ".");
} }

4
lib/checkassignif.h
View File

@ -63,7 +63,7 @@ public:
private: private:
void assignIfError(const Token *tok, const std::string &condition, bool result); void assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result);
void comparisonError(const Token *tok, void comparisonError(const Token *tok,
const std::string &bitop, const std::string &bitop,
@ -76,7 +76,7 @@ private:
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckAssignIf c(0, settings, errorLogger); CheckAssignIf c(0, settings, errorLogger);
c.assignIfError(0, "", false); c.assignIfError(0, 0, "", false);
c.comparisonError(0, "&", 6, "==", 1, false); c.comparisonError(0, "&", 6, "==", 1, false);
c.multiConditionError(0,1); c.multiConditionError(0,1);
} }

31
test/testassignif.cpp
View File

@ -65,33 +65,46 @@ private:
" int y = x & 4;\n" " int y = x & 4;\n"
" if (y == 3);\n" " if (y == 3);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Mismatching assignment and comparison, comparison is always false.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
check("void foo(int x)\n" check("void foo(int x)\n"
"{\n" "{\n"
" int y = x & 4;\n" " int y = x & 4;\n"
" if (y != 3);\n" " if (y != 3);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Mismatching assignment and comparison, comparison is always true.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y!=3' is always true.\n", errout.str());
check("void foo(int x) {\n"
" int y = x & 4;\n"
" if ((y == 3) && (z == 1));\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
// | // |
check("void foo(int x) {\n" check("void foo(int x) {\n"
" int y = x | 0x14;\n" " int y = x | 0x14;\n"
" if (y == 0x710);\n" " if (y == 0x710);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (style) Mismatching assignment and comparison, comparison is always false.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==1808' is always false.\n", errout.str());
check("void foo(int x) {\n" check("void foo(int x) {\n"
" int y = x | 0x14;\n" " int y = x | 0x14;\n"
" if (y == 0x71f);\n" " if (y == 0x71f);\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// multiple conditions
check("void foo(int x) {\n"
" int y = x & 4;\n"
" if ((y == 3) && (z == 1));\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
check("void foo(int x) {\n"
" int y = x & 4;\n"
" if ((x==123) || ((y == 3) && (z == 1)));\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
check("void f(int x) {\n"
" int y = x & 7;\n"
" if (setvalue(&y) && y != 8);\n"
"}");
ASSERT_EQUALS("", errout.str());
} }
void compare() { void compare() {