New check: Detect redundant assignment to a variable and redundant copying to a buffer
This check partially replaces the check for redundant assignments in switch
This commit is contained in:
PKEuS
parent
7d02a68e5e
commit
2d64b69cf4
|
|
@ -610,6 +610,155 @@ void CheckOther::sizeofForPointerError(const Token *tok, const std::string &varn
|
|||
"write sizeof(*" + varname + ")", true);
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Detect redundant assignments: x = 0; x = 4;
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckOther::checkRedundantAssignment()
|
||||
{
|
||||
if (!_settings->isEnabled("performance"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
|
||||
if (!i->isExecutable())
|
||||
continue;
|
||||
|
||||
///std::cout << std::endl << "scope: " << i->className << std::endl;
|
||||
|
||||
std::map<unsigned int, const Token*> varAssignments;
|
||||
std::map<unsigned int, const Token*> memAssignments;
|
||||
const Token* writtenArgumentsEnd = 0;
|
||||
|
||||
for (const Token* tok = i->classStart->next(); tok != i->classEnd; tok = tok->next()) {
|
||||
if (tok == writtenArgumentsEnd)
|
||||
writtenArgumentsEnd = 0;
|
||||
|
||||
if (tok->str() == "{" && tok->strAt(-1) != "{" && tok->strAt(-1) != "=" && tok->strAt(-4) != "case" && tok->strAt(-3) != "default") { // conditional or non-executable inner scope: Skip it and reset status
|
||||
tok = tok->link();
|
||||
varAssignments.clear();
|
||||
memAssignments.clear();
|
||||
} else if (Token::Match(tok, "for|if|while (")) {
|
||||
tok = tok->linkAt(1);
|
||||
} else if (Token::Match(tok, "break|return|continue|throw|goto")) {
|
||||
varAssignments.clear();
|
||||
memAssignments.clear();
|
||||
} else if (tok->type() == Token::eVariable) {
|
||||
std::map<unsigned int, const Token*>::iterator it = varAssignments.find(tok->varId());
|
||||
if (tok->next()->isAssignmentOp() && Token::Match(tok->previous(), "[;{}]")) { // Assignment
|
||||
///std::cout << "assign: " << tok->varId() << std::endl;
|
||||
if (it != varAssignments.end()) {
|
||||
bool error = true; // Ensure that variable is not used on right side
|
||||
for (const Token* tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
|
||||
if (tok2->str() == ";")
|
||||
break;
|
||||
else if (tok2->varId() == tok->varId())
|
||||
error = false;
|
||||
}
|
||||
if (error) {
|
||||
if (i->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok))
|
||||
redundantAssignmentInSwitchError(it->second, tok, tok->str());
|
||||
else
|
||||
redundantAssignmentError(it->second, tok, tok->str());
|
||||
}
|
||||
it->second = tok;
|
||||
}
|
||||
varAssignments[tok->varId()] = tok;
|
||||
memAssignments.erase(tok->varId());
|
||||
} else if (tok->next()->type() == Token::eIncDecOp || (tok->previous()->type() == Token::eIncDecOp && !Token::Match(tok->next(), ".|[|("))) { // Variable incremented/decremented
|
||||
varAssignments[tok->varId()] = tok;
|
||||
memAssignments.erase(tok->varId());
|
||||
} else if (!Token::Match(tok->tokAt(-2), "sizeof (")) { // Other usage of variable
|
||||
///std::cout << "use: " << tok->varId() << std::endl;
|
||||
if (it != varAssignments.end())
|
||||
varAssignments.erase(it);
|
||||
if (!writtenArgumentsEnd) // Indicates that we are in the first argument of strcpy/memcpy/... function
|
||||
memAssignments.erase(tok->varId());
|
||||
}
|
||||
} else if (Token::Match(tok, "%var% (")) { // Function call. Global variables might be used. Reset their status
|
||||
bool memfunc = Token::Match(tok, "memcpy|memmove|memset|strcpy|strncpy|sprintf|snprintf|strcat|strncat");
|
||||
if (memfunc) {
|
||||
const Token* param1 = tok->tokAt(2);
|
||||
writtenArgumentsEnd = param1->next();
|
||||
if (param1->varId() && param1->strAt(1) == "," && tok->str() != "strcat" && tok->str() != "strncat") {
|
||||
std::map<unsigned int, const Token*>::iterator it = memAssignments.find(param1->varId());
|
||||
if (it == memAssignments.end())
|
||||
memAssignments[param1->varId()] = tok;
|
||||
else {
|
||||
if (i->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok))
|
||||
redundantCopyInSwitchError(it->second, tok, param1->str());
|
||||
else
|
||||
redundantCopyError(it->second, tok, param1->str());
|
||||
}
|
||||
}
|
||||
} else {
|
||||
const Function* func = symbolDatabase->findFunctionByToken(_tokenizer->getFunctionTokenByName(tok->str().c_str()));
|
||||
if (!func || !func->hasBody) {
|
||||
varAssignments.clear();
|
||||
memAssignments.clear();
|
||||
continue;
|
||||
}
|
||||
const Token* funcEnd = func->functionScope->classEnd;
|
||||
bool noreturn;
|
||||
if (!_tokenizer->IsScopeNoReturn(funcEnd, &noreturn) && !noreturn) {
|
||||
for (std::map<unsigned int, const Token*>::iterator i = varAssignments.begin(); i != varAssignments.end(); ++i) {
|
||||
const Variable* var = symbolDatabase->getVariableFromVarId(i->first);
|
||||
if (!var || (!var->isLocal() && !var->isArgument()))
|
||||
i = varAssignments.erase(i);
|
||||
}
|
||||
for (std::map<unsigned int, const Token*>::iterator i = memAssignments.begin(); i != memAssignments.end(); ++i) {
|
||||
const Variable* var = symbolDatabase->getVariableFromVarId(i->first);
|
||||
if (!var || (!var->isLocal() && !var->isArgument()))
|
||||
i = memAssignments.erase(i);
|
||||
}
|
||||
} else {
|
||||
varAssignments.clear();
|
||||
memAssignments.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::performance, "redundantCopy",
|
||||
"Buffer '" + var + "' is being written before its old content has been used.");
|
||||
}
|
||||
|
||||
void CheckOther::redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::warning, "redundantCopyInSwitch",
|
||||
"Buffer '" + var + "' is being written before its old content has been used. This might indicate a missing 'break;'.");
|
||||
}
|
||||
|
||||
void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::performance, "redundantAssignment",
|
||||
"Variable '" + var + "' is reassigned a value before the old one has been used.");
|
||||
}
|
||||
|
||||
void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token* tok2, const std::string &var)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::warning, "redundantAssignInSwitch",
|
||||
"Variable '" + var + "' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.");
|
||||
}
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// switch (x)
|
||||
// {
|
||||
|
|
@ -636,18 +785,8 @@ void CheckOther::checkRedundantAssignmentInSwitch()
|
|||
continue;
|
||||
|
||||
// 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;
|
||||
/**
|
||||
* 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() == "{") {
|
||||
|
|
@ -658,19 +797,11 @@ void CheckOther::checkRedundantAssignmentInSwitch()
|
|||
const Token* endOfConditional = tok2->link();
|
||||
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());
|
||||
} else if (Token::Match(tok3, functionPattern) || Token::Match(tok3, breakPattern)) {
|
||||
varsAssigned.clear();
|
||||
varsWithBitsSet.clear();
|
||||
bitOperations.clear();
|
||||
varsOperatedByPostORPreFix.erase(tok3->varId());
|
||||
|
||||
if (tok3->str() != "strcpy" && tok3->str() != "strncpy")
|
||||
stringsCopied.clear();
|
||||
}
|
||||
}
|
||||
tok2 = endOfConditional;
|
||||
|
|
@ -682,31 +813,10 @@ void CheckOther::checkRedundantAssignmentInSwitch()
|
|||
// 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());
|
||||
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;
|
||||
|
|
@ -730,26 +840,12 @@ void CheckOther::checkRedundantAssignmentInSwitch()
|
|||
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");
|
||||
else if (Token::Match(tok2->previous(), ";|{|}|: strcpy|strncpy ( %var% ,") && tok2->tokAt(2)->varId() != 0) {
|
||||
std::map<unsigned int, const Token*>::iterator i2 = stringsCopied.find(tok2->tokAt(2)->varId());
|
||||
if (i2 == stringsCopied.end())
|
||||
stringsCopied[tok2->tokAt(2)->varId()] = tok2->tokAt(2);
|
||||
else
|
||||
redundantStrcpyInSwitchError(i2->second, i2->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 && tok2->strAt(1) != "|" && tok2->strAt(1) != "&") {
|
||||
varsAssigned.erase(tok2->varId());
|
||||
varsWithBitsSet.erase(tok2->varId());
|
||||
bitOperations.erase(tok2->varId());
|
||||
}
|
||||
|
|
@ -757,44 +853,19 @@ void CheckOther::checkRedundantAssignmentInSwitch()
|
|||
// 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();
|
||||
varsOperatedByPostORPreFix.clear();
|
||||
|
||||
if (tok2->str() != "strcpy" && tok2->str() != "strncpy")
|
||||
stringsCopied.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::string &varname)
|
||||
{
|
||||
reportError(tok, Severity::warning,
|
||||
"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,
|
||||
"redundantBitwiseOperationInSwitch", "Redundant bitwise operation on \"" + varname + "\" in switch");
|
||||
}
|
||||
|
||||
void CheckOther::redundantStrcpyInSwitchError(const Token *tok, const std::string &varname)
|
||||
{
|
||||
reportError(tok, Severity::warning,
|
||||
"redundantStrcpyInSwitch",
|
||||
"Switch case fall-through. Redundant strcpy of \"" + varname + "\".\n"
|
||||
"Switch case fall-through. Redundant strcpy of \"" + varname + "\". The string is overwritten in a later case block.");
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
//---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -59,6 +59,7 @@ public:
|
|||
checkOther.strPlusChar();
|
||||
checkOther.sizeofsizeof();
|
||||
checkOther.sizeofCalculation();
|
||||
checkOther.checkRedundantAssignment();
|
||||
checkOther.checkRedundantAssignmentInSwitch();
|
||||
checkOther.checkAssignmentInAssert();
|
||||
checkOther.checkSizeofForArrayParameter();
|
||||
|
|
@ -168,6 +169,9 @@ public:
|
|||
/** @brief %Check for calculations inside sizeof */
|
||||
void sizeofCalculation();
|
||||
|
||||
/** @brief copying to memory or assigning to a variablen twice */
|
||||
void checkRedundantAssignment();
|
||||
|
||||
/** @brief %Check for assigning to the same variable twice in a switch statement*/
|
||||
void checkRedundantAssignmentInSwitch();
|
||||
|
||||
|
|
@ -277,10 +281,11 @@ private:
|
|||
void zerodivError(const Token *tok);
|
||||
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 redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var);
|
||||
void redundantAssignmentInSwitchError(const Token *tok1, const Token *tok2, const std::string &var);
|
||||
void redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var);
|
||||
void redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var);
|
||||
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);
|
||||
void assignmentInAssertError(const Token *tok, const std::string &varname);
|
||||
|
|
@ -337,6 +342,8 @@ private:
|
|||
|
||||
//performance
|
||||
c.redundantCopyError(0, "varname");
|
||||
c.redundantCopyError(0, 0, "var");
|
||||
c.redundantAssignmentError(0, 0, "var");
|
||||
|
||||
// style/warning
|
||||
c.cstyleCastError(0);
|
||||
|
|
@ -349,8 +356,8 @@ private:
|
|||
c.strPlusCharError(0);
|
||||
c.sizeofsizeofError(0);
|
||||
c.sizeofCalculationError(0, false);
|
||||
c.redundantAssignmentInSwitchError(0, "varname");
|
||||
c.redundantOperationInSwitchError(0, "varname");
|
||||
c.redundantAssignmentInSwitchError(0, 0, "var");
|
||||
c.redundantCopyInSwitchError(0, 0, "var");
|
||||
c.switchCaseFallThrough(0);
|
||||
c.selfAssignmentError(0, "varname");
|
||||
c.assignmentInAssertError(0, "varname");
|
||||
|
|
@ -405,6 +412,7 @@ private:
|
|||
|
||||
//performance
|
||||
"* redundant data copying for const variable\n"
|
||||
"* subsequent assignment or copying to a variable or buffer\n"
|
||||
|
||||
// style
|
||||
"* C-style pointer cast in cpp file\n"
|
||||
|
|
|
|||
|
|
@ -168,6 +168,9 @@ private:
|
|||
TEST_CASE(checkNegativeShift);
|
||||
|
||||
TEST_CASE(incompleteArrayFill);
|
||||
|
||||
TEST_CASE(redundantVarAssignment);
|
||||
TEST_CASE(redundantMemWrite);
|
||||
}
|
||||
|
||||
void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) {
|
||||
|
|
@ -177,6 +180,7 @@ private:
|
|||
Settings settings;
|
||||
settings.addEnabled("style");
|
||||
settings.addEnabled("portability");
|
||||
settings.addEnabled("performance");
|
||||
settings.inconclusive = inconclusive;
|
||||
settings.experimental = experimental;
|
||||
|
||||
|
|
@ -1384,7 +1388,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant assignment of \"y\" in switch\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
|
|
@ -1400,7 +1404,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant assignment of \"y\" in switch\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:11]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
|
|
@ -1557,7 +1561,7 @@ private:
|
|||
" strcpy(str, \"b'\");\n"
|
||||
" }\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:6]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:8]: (warning) Buffer 'str' is being written before its old content has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
|
||||
check("void foo(char *str, int a)\n"
|
||||
"{\n"
|
||||
|
|
@ -1569,7 +1573,7 @@ private:
|
|||
" strncpy(str, \"b'\");\n"
|
||||
" }\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:6]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:8]: (warning) Buffer 'str' is being written before its old content has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
|
||||
check("void foo(char *str, int a)\n"
|
||||
"{\n"
|
||||
|
|
@ -1584,7 +1588,7 @@ private:
|
|||
" z++;\n"
|
||||
" }\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Switch case fall-through. Redundant strcpy of \"str\".\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:10]: (warning) Buffer 'str' is being written before its old content has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
|
||||
check("void foo(char *str, int a)\n"
|
||||
"{\n"
|
||||
|
|
@ -1615,7 +1619,6 @@ private:
|
|||
}
|
||||
|
||||
void switchRedundantOperationTest() {
|
||||
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1628,7 +1631,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1643,7 +1646,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:11]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1682,7 +1685,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1697,7 +1700,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:11]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1736,7 +1739,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1751,7 +1754,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:11]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1790,7 +1793,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1805,7 +1808,7 @@ private:
|
|||
" }\n"
|
||||
" bar(y);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:8]: (warning) Redundant operation on 'y' in switch.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:11]: (warning) Variable 'y' is reassigned a value before the old one has been used. This might indicate a missing 'break;'.\n", errout.str());
|
||||
check("void foo()\n"
|
||||
"{\n"
|
||||
" int y = 1;\n"
|
||||
|
|
@ -1879,21 +1882,6 @@ private:
|
|||
" 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"
|
||||
|
|
@ -3368,9 +3356,9 @@ private:
|
|||
void incorrectLogicOperator3() {
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 5 && x == 1;\n"
|
||||
" b = x < 1 && x == 3;\n"
|
||||
" b = x >= 5 && x == 1;\n"
|
||||
" b = x <= 1 && x == 3;\n"
|
||||
" c = x < 1 && x == 3;\n"
|
||||
" d = x >= 5 && x == 1;\n"
|
||||
" e = x <= 1 && x == 3;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 5 && x == 1.\n"
|
||||
"[test.cpp:3]: (warning) Logical conjunction always evaluates to false: x < 1 && x == 3.\n"
|
||||
|
|
@ -3409,9 +3397,9 @@ private:
|
|||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 3 || x == 4;\n"
|
||||
" b = x < 5 || x == 4;\n"
|
||||
" b = x >= 3 || x == 4;\n"
|
||||
" b = x <= 5 || x == 4;\n"
|
||||
" c = x < 5 || x == 4;\n"
|
||||
" d = x >= 3 || x == 4;\n"
|
||||
" e = x <= 5 || x == 4;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 4, the comparison x > 3 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x == 4, the comparison x < 5 is always true.\n"
|
||||
|
|
@ -3420,9 +3408,9 @@ private:
|
|||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 5 || x != 1;\n"
|
||||
" b = x < 1 || x != 3;\n"
|
||||
" b = x >= 5 || x != 1;\n"
|
||||
" b = x <= 1 || x != 3;\n"
|
||||
" c = x < 1 || x != 3;\n"
|
||||
" d = x >= 5 || x != 1;\n"
|
||||
" e = x <= 1 || x != 3;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x < 1, the comparison x != 3 is always true.\n"
|
||||
|
|
@ -3431,9 +3419,9 @@ private:
|
|||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 6 && x > 5;\n"
|
||||
" b = x > 5 || x > 6;\n"
|
||||
" b = x < 6 && x < 5;\n"
|
||||
" b = x < 5 || x < 6;\n"
|
||||
" c = x > 5 || x > 6;\n"
|
||||
" d = x < 6 && x < 5;\n"
|
||||
" e = x < 5 || x < 6;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 6, the comparison x > 5 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x > 5, the comparison x > 6 is always true.\n"
|
||||
|
|
@ -5963,7 +5951,9 @@ private:
|
|||
" memcpy(a, b, 5);\n"
|
||||
" memmove(a, b, 5);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n"
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n"
|
||||
"[test.cpp:3] -> [test.cpp:5]: (performance) Buffer 'a' is being written before its old content has been used.\n"
|
||||
"[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n"
|
||||
"[test.cpp:4]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memcpy()' with 'sizeof(*a)'?\n"
|
||||
"[test.cpp:5]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memmove()' with 'sizeof(*a)'?\n", errout.str());
|
||||
|
||||
|
|
@ -6016,6 +6006,196 @@ private:
|
|||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (portability, inconclusive) Array 'a' might be filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", errout.str());
|
||||
}
|
||||
|
||||
void redundantVarAssignment() {
|
||||
// Simple tests
|
||||
check("void f(int i) {\n"
|
||||
" i = 1;\n"
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
check("int i;\n"
|
||||
"void f() {\n"
|
||||
" i = 1;\n"
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int i;\n"
|
||||
" i = 1;\n"
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
// Testing different types
|
||||
check("void f() {\n"
|
||||
" Foo& bar = foo();\n"
|
||||
" bar = x;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
check("void f() {\n"
|
||||
" Foo& bar = foo();\n"
|
||||
" bar = x;\n"
|
||||
" bar = y();\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'bar' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
// Tests with function call between assignment
|
||||
check("void bar() {}\n"
|
||||
"void f(int i) {\n"
|
||||
" i = 1;\n"
|
||||
" bar();\n"
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
check("void bar() {}\n"
|
||||
"int i;\n"
|
||||
"void f() {\n"
|
||||
" i = 1;\n"
|
||||
" bar();\n" // Global variable might be accessed in bar()
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void bar() {}\n"
|
||||
"void f() {\n"
|
||||
" int i;\n"
|
||||
" i = 1;\n"
|
||||
" bar();\n"
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
|
||||
check("void f(int i) {\n"
|
||||
" i = 1;\n"
|
||||
" bar();\n" // Unknown function - can be noreturn
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void bar(int i) {}\n"
|
||||
"void f(int i) {\n"
|
||||
" i = 1;\n"
|
||||
" bar(i);\n" // Passed as argument
|
||||
" i = 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// Branch tests
|
||||
check("void f(int i) {\n"
|
||||
" i = 1;\n"
|
||||
" if(x)\n"
|
||||
" i = 0;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int i) {\n"
|
||||
" if(x)\n"
|
||||
" i = 0;\n"
|
||||
" i = 1;\n"
|
||||
" i = 2;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
|
||||
}
|
||||
|
||||
void redundantMemWrite() {
|
||||
// Simple tests
|
||||
check("void f(void* a) {\n"
|
||||
" memcpy(a, foo, bar);\n"
|
||||
" memset(a, 0, bar);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
check("void* a;\n"
|
||||
"void f() {\n"
|
||||
" strcpy(a, foo);\n"
|
||||
" strncpy(a, 0, bar);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" void* a = foo();\n"
|
||||
" sprintf(a, foo);\n"
|
||||
" memmove(a, 0, bar);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
// Writing to different parts of a buffer
|
||||
check("void f(void* a) {\n"
|
||||
" memcpy(a, foo, bar);\n"
|
||||
" memset(a+5, 0, bar);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// Use variable as second argument
|
||||
check("void f(void* a, void* b) {\n"
|
||||
" memset(a, 0, 5);\n"
|
||||
" memcpy(b, a, 5);\n"
|
||||
" memset(a, 1, 5);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// strcat is special
|
||||
check("void f(void* a) {\n"
|
||||
" strcpy(a, foo);\n"
|
||||
" strcat(a, bar);\n" // Not redundant
|
||||
" strcpy(a, x);\n" // Redundant
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
// Tests with function call between copy
|
||||
check("void bar() {}\n"
|
||||
"void f(void* a) {\n"
|
||||
" snprintf(a, foo, bar);\n"
|
||||
" bar();\n"
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
check("void bar() {}\n"
|
||||
"void* a;\n"
|
||||
"void f() {\n"
|
||||
" memset(a, 0, size);\n"
|
||||
" bar();\n" // Global variable might be accessed in bar()
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void bar() {}\n"
|
||||
"void f() {\n"
|
||||
" void* a = foo();\n"
|
||||
" memset(a, 0, size);\n"
|
||||
" bar();\n"
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
|
||||
|
||||
check("void f(void* a) {\n"
|
||||
" memset(a, 0, size);\n"
|
||||
" bar();\n" // Unknown function - can be noreturn
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void bar(void* a) {}\n"
|
||||
"void f(void* a) {\n"
|
||||
" memset(a, 0, size);\n"
|
||||
" bar(a);\n" // Passed as argument
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// Branch tests
|
||||
check("void f(void* a) {\n"
|
||||
" memset(a, 0, size);\n"
|
||||
" if(x)\n"
|
||||
" memset(a, 0, size);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
};
|
||||
|
||||
REGISTER_TEST(TestOther)
|
||||
|
|
|
|||
Loading…
Reference in New Issue