Fixed #3794 (New check: Missing break in switch (duplicate bitwise operation))

This commit is contained in:
Zachary Blair 2012-05-28 21:19:22 -07:00
parent e584544915
commit 2bd171dded
4 changed files with 365 additions and 137 deletions

View File

@ -600,6 +600,9 @@ void CheckOther::checkRedundantAssignmentInSwitch()
// Check the contents of the switch statement
std::map<unsigned int, const Token*> varsAssigned;
std::map<unsigned int, const Token*> stringsCopied;
std::map<unsigned int, const Token*> varsWithBitsSet;
std::map<unsigned int, std::string> bitOperations;
for (const Token *tok2 = i->classStart->next(); tok2 != i->classEnd; tok2 = tok2->next()) {
if (tok2->str() == "{") {
// Inside a conditional or loop. Don't mark variable accesses as being redundant. E.g.:
@ -611,8 +614,12 @@ void CheckOther::checkRedundantAssignmentInSwitch()
if (tok3->varId() != 0) {
varsAssigned.erase(tok3->varId());
stringsCopied.erase(tok3->varId());
varsWithBitsSet.erase(tok3->varId());
bitOperations.erase(tok3->varId());
} else if (Token::Match(tok3, functionPattern) || Token::Match(tok3, breakPattern)) {
varsAssigned.clear();
varsWithBitsSet.clear();
bitOperations.clear();
if (tok3->str() != "strcpy" && tok3->str() != "strncpy")
stringsCopied.clear();
@ -633,7 +640,38 @@ void CheckOther::checkRedundantAssignmentInSwitch()
redundantAssignmentInSwitchError(i2->second, i2->second->str());
stringsCopied.erase(tok2->varId());
varsWithBitsSet.erase(tok2->varId());
bitOperations.erase(tok2->varId());
}
// Bitwise operation. Report an error if it's performed twice before a break. E.g.:
// case 3: b |= 1; // <== redundant
// case 4: b |= 1;
else if (Token::Match(tok2->previous(), ";|{|}|: %var% = %var% %or%|& %num% ;") &&
tok2->varId() != 0 && tok2->varId() == tok2->tokAt(2)->varId()) {
std::string bitOp = tok2->strAt(3) + tok2->strAt(4);
std::map<unsigned int, const Token*>::iterator i2 = varsWithBitsSet.find(tok2->varId());
// This variable has not had a bit operation performed on it yet, so just make a note of it
if (i2 == varsWithBitsSet.end()) {
varsWithBitsSet[tok2->varId()] = tok2;
bitOperations[tok2->varId()] = bitOp;
}
// The same bit operation has been performed on the same variable twice, so report an error
else if (bitOperations[tok2->varId()] == bitOp)
redundantBitwiseOperationInSwitchError(i2->second, i2->second->str());
// A different bit operation was performed on the variable, so clear it
else {
varsWithBitsSet.erase(tok2->varId());
bitOperations.erase(tok2->varId());
}
stringsCopied.erase(tok2->varId());
varsAssigned.erase(tok2->varId());
}
// String copy. Report an error if it's copied to twice before a break. E.g.:
// case 3: strcpy(str, "a"); // <== redundant
// case 4: strcpy(str, "b");
@ -647,13 +685,18 @@ void CheckOther::checkRedundantAssignmentInSwitch()
// Not a simple assignment so there may be good reason if this variable is assigned to twice. E.g.:
// case 3: b = 1;
// case 4: b++;
else if (tok2->varId() != 0)
else if (tok2->varId() != 0 && tok2->strAt(1) != "|" && tok2->strAt(1) != "&") {
varsAssigned.erase(tok2->varId());
varsWithBitsSet.erase(tok2->varId());
bitOperations.erase(tok2->varId());
}
// Reset our record of assignments if there is a break or function call. E.g.:
// case 3: b = 1; break;
if (Token::Match(tok2, functionPattern) || Token::Match(tok2, breakPattern)) {
varsAssigned.clear();
varsWithBitsSet.clear();
bitOperations.clear();
if (tok2->str() != "strcpy" && tok2->str() != "strncpy")
stringsCopied.clear();
@ -668,6 +711,12 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::s
"redundantAssignInSwitch", "Redundant assignment of \"" + varname + "\" in switch");
}
void CheckOther::redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,
"redundantBitwiseOperationInSwitch", "Redundant bitwise operation on \"" + varname + "\" in switch");
}
void CheckOther::redundantStrcpyInSwitchError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,

View File

@ -255,6 +255,7 @@ private:
void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1);
void cctypefunctionCallError(const Token *tok, const std::string &functionName, const std::string &value);
void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
void redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname);
void redundantStrcpyInSwitchError(const Token *tok, const std::string &varname);
void switchCaseFallThrough(const Token *tok);
void selfAssignmentError(const Token *tok, const std::string &varname);
@ -375,6 +376,7 @@ private:
"* condition that is always true/false\n"
"* unusal pointer arithmetic. For example: \"abc\" + 'd'\n"
"* redundant assignment in a switch statement\n"
"* redundant bitwise operation in a switch statement\n"
"* redundant strcpy in a switch statement\n"
"* look for 'sizeof sizeof ..'\n"
"* look for calculations inside sizeof()\n"

View File

@ -621,7 +621,7 @@ bool Token::Match(const Token *tok, const char pattern[], unsigned int varid)
}
break;
case 'n':
// Number (%num)
// Number (%num%)
{
if (!tok->isNumber())
return false;

View File

@ -79,6 +79,7 @@ private:
TEST_CASE(sizeofCalculation);
TEST_CASE(switchRedundantAssignmentTest);
TEST_CASE(switchRedundantBitwiseOperationTest);
TEST_CASE(switchFallThroughCase);
TEST_CASE(unreachableCode);
@ -1343,112 +1344,112 @@ private:
void switchRedundantAssignmentTest() {
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant assignment of \"y\" in switch\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y = 2;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y = 2;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant assignment of \"y\" in switch\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" if (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" if (x)\n"
" {\n"
" y = 3;\n"
" }\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y = 2;\n"
" if (y)\n"
" printf(\"%d\", y);\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y = 2;\n"
" if (y)\n"
" printf(\"%d\", y);\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int x = a;\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" x = 2;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int x = a;\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" x = 2;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" {\n"
" int y = 2;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" {\n"
" int y = 2;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" break;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" break;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
@ -1488,97 +1489,273 @@ private:
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" printf(\"%d\", y);\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" printf(\"%d\", y);\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" bar();\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" bar();\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(char *str, int a)\n"
"{\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" }\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
check("void foo(char *str, int a)\n"
"{\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strncpy(str, \"a'\");\n"
" case 3:\n"
" strncpy(str, \"b'\");\n"
" }\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strncpy(str, \"a'\");\n"
" case 3:\n"
" strncpy(str, \"b'\");\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
check("void foo(char *str, int a)\n"
"{\n"
" int z = 0;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" z++;\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" z++;\n"
" }\n"
" int z = 0;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" z++;\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" z++;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
check("void foo(char *str, int a)\n"
"{\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" break;\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" break;\n"
" }\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" break;\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(char *str, int a)\n"
"{\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" printf(str);\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" }\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" strcpy(str, \"a'\");\n"
" printf(str);\n"
" case 3:\n"
" strcpy(str, \"b'\");\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void switchRedundantBitwiseOperationTest() {
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" case 3:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant bitwise operation on \"y\" in switch\n", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" default:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant bitwise operation on \"y\" in switch\n", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" default:\n"
" if (z)\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= z;\n"
" z++\n"
" default:\n"
" y |= z;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" bar(y);\n"
" case 3:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" y = 4;\n"
" case 3:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y &= 3;\n"
" case 3:\n"
" y &= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant bitwise operation on \"y\" in switch\n", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 3;\n"
" break;\n"
" case 3:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y ^= 3;\n"
" case 3:\n"
" y ^= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 2;\n"
" case 3:\n"
" y |= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y &= 2;\n"
" case 3:\n"
" y &= 3;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo(int a)\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y |= 2;\n"
" case 3:\n"
" y &= 2;\n"
" break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}