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

Fixed #2628: Detect redudant usage of operator++/-- in switch.

This commit is contained in:
Kumar Ashwani 2012-08-22 14:40:57 +02:00 committed by PKEuS
parent e05a597066
commit afe030ce9b
3 changed files with 396 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
lib/checkother.cpp
View File

@ -605,6 +605,14 @@ void CheckOther::checkRedundantAssignmentInSwitch()
std::map<unsigned int, const Token*> stringsCopied;
std::map<unsigned int, const Token*> varsWithBitsSet;
std::map<unsigned int, std::string> bitOperations;
/**
* A separate map for variables with post/pre increment/decrement has been kept since
* CASE 1(Reduntant): CASE 2(NOT Reduntant): CASE 3(NOT Reduntant):
* ret++; ret = 3; ret++;
* ret = 2; ret++; ret++;
*
*/
std::map<unsigned int, const Token*> varsOperatedByPostORPreFix;
for (const Token *tok2 = i->classStart->next(); tok2 != i->classEnd; tok2 = tok2->next()) {
if (tok2->str() == "{") {
@ -616,6 +624,7 @@ void CheckOther::checkRedundantAssignmentInSwitch()
for (const Token* tok3 = tok2; tok3 != endOfConditional; tok3 = tok3->next()) {
if (tok3->varId() != 0) {
varsAssigned.erase(tok3->varId());
varsOperatedByPostORPreFix.erase(tok3->varId());
stringsCopied.erase(tok3->varId());
varsWithBitsSet.erase(tok3->varId());
bitOperations.erase(tok3->varId());
@ -623,6 +632,7 @@ void CheckOther::checkRedundantAssignmentInSwitch()
varsAssigned.clear();
varsWithBitsSet.clear();
bitOperations.clear();
varsOperatedByPostORPreFix.erase(tok3->varId());
if (tok3->str() != "strcpy" && tok3->str() != "strncpy")
stringsCopied.clear();
@ -635,18 +645,33 @@ void CheckOther::checkRedundantAssignmentInSwitch()
// Variable assignment. Report an error if it's assigned to twice before a break. E.g.:
// case 3: b = 1; // <== redundant
// case 4: b = 2;
if (Token::Match(tok2->previous(), ";|{|}|: %var% = %any% ;") && tok2->varId() != 0) {
std::map<unsigned int, const Token*>::iterator i2 = varsAssigned.find(tok2->varId());
if (i2 == varsAssigned.end())
varsAssigned[tok2->varId()] = tok2;
else
redundantAssignmentInSwitchError(i2->second, i2->second->str());
std::map<unsigned int, const Token*>::iterator i3 = varsOperatedByPostORPreFix.find(tok2->varId());
if (i2 == varsAssigned.end() && i3 == varsOperatedByPostORPreFix.end())
varsAssigned[tok2->varId()] = tok2;
else {
if (i3 == varsOperatedByPostORPreFix.end())
redundantAssignmentInSwitchError(i2->second, i2->second->str());
else
redundantOperationInSwitchError(i3->second, i3->second->str());
}
stringsCopied.erase(tok2->varId());
varsWithBitsSet.erase(tok2->varId());
bitOperations.erase(tok2->varId());
}
else if ((Token::Match(tok2->previous(), ";|{|}|: %var% ++|-- ;") ||
Token::Match(tok2->tokAt(-2), ";|{|}|: ++|-- %var% ;")) && tok2->varId() != 0) {
std::map<unsigned int, const Token*>::iterator i2 = varsOperatedByPostORPreFix.find(tok2->varId());
if (i2 == varsOperatedByPostORPreFix.end())
varsOperatedByPostORPreFix[tok2->varId()] = tok2;
}
// Bitwise operation. Report an error if it's performed twice before a break. E.g.:
// case 3: b |= 1; // <== redundant
// case 4: b |= 1;
@ -700,6 +725,7 @@ void CheckOther::checkRedundantAssignmentInSwitch()
varsAssigned.clear();
varsWithBitsSet.clear();
bitOperations.clear();
varsOperatedByPostORPreFix.clear();
if (tok2->str() != "strcpy" && tok2->str() != "strncpy")
stringsCopied.clear();
@ -714,6 +740,13 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::s
"redundantAssignInSwitch", "Redundant assignment of \"" + varname + "\" in switch");
}
void CheckOther::redundantOperationInSwitchError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,
"redundantOperationInSwitch", "Redundant operation on '" + varname + "' in switch.");
}
void CheckOther::redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,

3
lib/checkother.h
View File

@ -261,6 +261,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 redundantOperationInSwitchError(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);
@ -329,6 +330,7 @@ private:
c.sizeofsizeofError(0);
c.sizeofCalculationError(0, false);
c.redundantAssignmentInSwitchError(0, "varname");
c.redundantOperationInSwitchError(0, "varname");
c.switchCaseFallThrough(0);
c.selfAssignmentError(0, "varname");
c.assignmentInAssertError(0, "varname");
@ -393,6 +395,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 pre/post operation 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"

356
test/testother.cpp
View File

@ -80,6 +80,7 @@ private:
TEST_CASE(sizeofCalculation);
TEST_CASE(switchRedundantAssignmentTest);
TEST_CASE(switchRedundantOperationTest);
TEST_CASE(switchRedundantBitwiseOperationTest);
TEST_CASE(switchFallThroughCase);
TEST_CASE(unreachableCode);
@ -1606,6 +1607,361 @@ private:
ASSERT_EQUALS("", errout.str());
}
void switchRedundantOperationTest() {
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" ++y;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" ++y;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y;\n"
" case 3:\n"
" ++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"
" ++y;\n"
" case 3:\n"
" ++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"
" --y;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" --y;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y;\n"
" case 3:\n"
" --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"
" --y;\n"
" case 3:\n"
" --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"
" y++;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y++;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" 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"
" y++;\n"
" case 3:\n"
" 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"
" y--;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" y--;\n"
" }\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" y = 2;\n"
" case 3:\n"
" 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"
" y--;\n"
" case 3:\n"
" 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"
" y++;\n"
" case 3:\n"
" if (x)\n"
" {\n"
" y = 3;\n"
" }\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++;\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++;\n"
" case 3:\n"
" y++;\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++;\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++;\n"
" break;\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"
" while(xyz()) {\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y++;\n"
" continue;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" while(xyz()) {\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y++;\n"
" throw e;\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo()\n"
"{\n"
" int y = 1;\n"
" switch (x)\n"
" {\n"
" case 2:\n"
" y++;\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++;\n"
" bar();\n"
" case 3:\n"
" y = 3;\n"
" }\n"
" bar(y);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void switchRedundantBitwiseOperationTest() {
check("void foo(int a)\n"