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

Fixed #157 (Forgetting to put a break in a switch statement)

This commit is contained in:
Zachary Blair 2010-06-30 00:10:30 -07:00
parent c6888845a0
commit 5ea28ccbba
3 changed files with 234 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
lib/checkother.cpp
View File

@ -267,6 +267,89 @@ void CheckOther::checkFflushOnInputStream()
} }
} }
//---------------------------------------------------------------------------
// switch (x)
// {
// case 2:
// y = a; // <- this assignment is redundant
// case 3:
// y = b; // <- case 2 falls through and sets y twice
// }
//---------------------------------------------------------------------------
void CheckOther::checkRedundantAssignmentInSwitch()
{
const char switchPattern[] = "switch ( %any% ) { case";
const char breakPattern[] = "break|return|exit|goto";
const char functionPattern[] = "%var% (";
// Find the beginning of a switch. E.g.:
// switch (var) { ...
const Token *tok = Token::findmatch(_tokenizer->tokens(), switchPattern);
while (tok)
{
// Check the contents of the switch statement
std::map<unsigned int, const Token*> varsAssigned;
int indentLevel = 0;
for (const Token *tok2 = tok->tokAt(5); tok2; tok2 = tok2->next())
{
if (tok2->str() == "{")
{
// Inside a conditional or loop. Don't mark variable accesses as being redundant. E.g.:
// case 3: b = 1;
// case 4: if (a) { b = 2; } // Doesn't make the b=1 redundant because it's conditional
if (Token::Match(tok2->previous(), ")|else {") && tok2->link())
{
const Token* endOfConditional = tok2->link();
for (const Token* tok3 = tok2; tok3 != endOfConditional; tok3 = tok3->next())
{
if (tok3->varId() != 0)
varsAssigned.erase(tok3->varId());
else if (Token::Match(tok3, functionPattern) || Token::Match(tok3, breakPattern))
varsAssigned.clear();
}
tok2 = endOfConditional;
}
else
++ indentLevel;
}
else if (tok2->str() == "}")
{
-- indentLevel;
// End of the switch block
if (indentLevel < 0)
break;
}
// 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 i = varsAssigned.find(tok2->varId());
if (i == varsAssigned.end())
varsAssigned[tok2->varId()] = tok2;
else
redundantAssignmentInSwitchError(i->second, i->second->str());
}
// 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)
varsAssigned.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();
}
tok = Token::findmatch(tok->next(), switchPattern);
}
}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// strtol(str, 0, radix) <- radix must be 0 or 2-36 // strtol(str, 0, radix) <- radix must be 0 or 2-36
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
@ -3773,3 +3856,9 @@ void CheckOther::sizeofsizeofError(const Token *tok)
reportError(tok, Severity::style, reportError(tok, Severity::style,
"sizeofsizeof", "Suspicious code 'sizeof sizeof ..', most likely there should only be one sizeof. The current code is equivalent to 'sizeof(size_t)'."); "sizeofsizeof", "Suspicious code 'sizeof sizeof ..', most likely there should only be one sizeof. The current code is equivalent to 'sizeof(size_t)'.");
} }
void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::style,
"redundantAssignInSwitch", "Redundant assignment of \"" + varname + "\" in switch");
}

7
lib/checkother.h
View File

@ -62,6 +62,7 @@ public:
checkOther.strPlusChar(); checkOther.strPlusChar();
checkOther.sizeofsizeof(); checkOther.sizeofsizeof();
checkOther.checkEmptyCatchBlock(); checkOther.checkEmptyCatchBlock();
checkOther.checkRedundantAssignmentInSwitch();
} }
/** @brief Run checks against the simplified token list */ /** @brief Run checks against the simplified token list */
@ -178,6 +179,9 @@ public:
void sizeofsizeof(); void sizeofsizeof();
void sizeofsizeofError(const Token *tok); void sizeofsizeofError(const Token *tok);
/** @brief %Check for assigning to the same variable twice in a switch statement*/
void checkRedundantAssignmentInSwitch();
// Error messages.. // Error messages..
void cstyleCastError(const Token *tok); void cstyleCastError(const Token *tok);
void redundantIfDelete0Error(const Token *tok); void redundantIfDelete0Error(const Token *tok);
@ -205,6 +209,7 @@ public:
void emptyStringTestError(const Token *tok, const std::string &var_name, const bool isTestForEmpty); void emptyStringTestError(const Token *tok, const std::string &var_name, const bool isTestForEmpty);
void fflushOnInputStreamError(const Token *tok, const std::string &varname); void fflushOnInputStreamError(const Token *tok, const std::string &varname);
void emptyCatchBlockError(const Token *tok); void emptyCatchBlockError(const Token *tok);
void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
void getErrorMessages() void getErrorMessages()
{ {
@ -234,6 +239,7 @@ public:
strPlusChar(0); strPlusChar(0);
sizeofsizeofError(0); sizeofsizeofError(0);
emptyCatchBlockError(0); emptyCatchBlockError(0);
redundantAssignmentInSwitchError(0, "varname");
// optimisations // optimisations
postIncrementError(0, "varname", true); postIncrementError(0, "varname", true);
@ -269,6 +275,7 @@ public:
"* condition that is always true/false\n" "* condition that is always true/false\n"
"* unusal pointer arithmetic. For example: \"abc\" + 'd'\n" "* unusal pointer arithmetic. For example: \"abc\" + 'd'\n"
"* empty catch() block\n" "* empty catch() block\n"
"* redundant assignment in a switch statement\n"
// optimisations // optimisations
"* optimisation: detect post increment/decrement\n" "* optimisation: detect post increment/decrement\n"

138
test/testother.cpp
View File

@ -100,6 +100,8 @@ private:
TEST_CASE(sizeofsizeof); TEST_CASE(sizeofsizeof);
TEST_CASE(emptyCatchBlock); TEST_CASE(emptyCatchBlock);
TEST_CASE(switchRedundantAssignmentTest);
} }
void check(const char code[]) void check(const char code[])
@ -119,6 +121,7 @@ private:
checkOther.sizeofsizeof(); checkOther.sizeofsizeof();
checkOther.checkEmptyCatchBlock(); checkOther.checkEmptyCatchBlock();
checkOther.checkRedundantAssignmentInSwitch();
// Simplify token list.. // Simplify token list..
tokenizer.simplifyTokenList(); tokenizer.simplifyTokenList();
@ -2715,6 +2718,141 @@ private:
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
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"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (style) 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"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (style) 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"
" y = 2;\n"
" case 3:\n"
" if (x)\n"
" {\n"
" y = 3;\n"
" }\n"
" }\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"
"}\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"
"}\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"
"}\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"
"}\n");
ASSERT_EQUALS("", errout.str());
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"
"}\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"
"}\n");
ASSERT_EQUALS("", errout.str());
}
}; };
REGISTER_TEST(TestOther) REGISTER_TEST(TestOther)