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

Revert "Improved handling of 0 initializations (#4604)" 

This reverts commit 1201e417ec.
This commit is contained in:
PKEuS 2013-05-09 15:32:02 +02:00
parent 0a104c40b7
commit 881b47e79d
2 changed files with 42 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
lib/checkother.cpp
View File

@ -612,6 +612,7 @@ void CheckOther::checkRedundantAssignment()
std::map<unsigned int, const Token*> varAssignments; std::map<unsigned int, const Token*> varAssignments;
std::map<unsigned int, const Token*> memAssignments; std::map<unsigned int, const Token*> memAssignments;
std::set<unsigned int> initialized;
const Token* writtenArgumentsEnd = 0; const Token* writtenArgumentsEnd = 0;
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
@ -628,6 +629,17 @@ void CheckOther::checkRedundantAssignment()
varAssignments.clear(); varAssignments.clear();
memAssignments.clear(); memAssignments.clear();
} else if (tok->type() == Token::eVariable) { } else if (tok->type() == Token::eVariable) {
// Set initialization flag
if (!Token::Match(tok, "%var% ["))
initialized.insert(tok->varId());
else {
const Token *tok2 = tok->next();
while (tok2 && tok2->str() == "[")
tok2 = tok2->link()->next();
if (tok2 && tok2->str() != ";")
initialized.insert(tok->varId());
}
std::map<unsigned int, const Token*>::iterator it = varAssignments.find(tok->varId()); std::map<unsigned int, const Token*>::iterator it = varAssignments.find(tok->varId());
if (tok->next()->isAssignmentOp() && Token::Match(tok->previous(), "[;{}]")) { // Assignment if (tok->next()->isAssignmentOp() && Token::Match(tok->previous(), "[;{}]")) { // Assignment
if (it != varAssignments.end()) { if (it != varAssignments.end()) {
@ -652,7 +664,9 @@ void CheckOther::checkRedundantAssignment()
} }
it->second = tok; it->second = tok;
} }
if (!Token::simpleMatch(tok->tokAt(2), "0 ;") || (tok->variable() && tok->variable()->nameToken() != tok->tokAt(-2))) if (Token::simpleMatch(tok->tokAt(2), "0 ;"))
varAssignments.erase(tok->varId());
else
varAssignments[tok->varId()] = tok; varAssignments[tok->varId()] = tok;
memAssignments.erase(tok->varId()); memAssignments.erase(tok->varId());
} else if (tok->next()->type() == Token::eIncDecOp || (tok->previous()->type() == Token::eIncDecOp && !Token::Match(tok->next(), ".|[|("))) { // Variable incremented/decremented } else if (tok->next()->type() == Token::eIncDecOp || (tok->previous()->type() == Token::eIncDecOp && !Token::Match(tok->next(), ".|[|("))) { // Variable incremented/decremented
@ -670,10 +684,12 @@ void CheckOther::checkRedundantAssignment()
const Token* param1 = tok->tokAt(2); const Token* param1 = tok->tokAt(2);
writtenArgumentsEnd = param1->next(); writtenArgumentsEnd = param1->next();
if (param1->varId() && param1->strAt(1) == "," && !Token::Match(tok, "strcat|strncat|wcscat|wcsncat")) { if (param1->varId() && param1->strAt(1) == "," && !Token::Match(tok, "strcat|strncat|wcscat|wcsncat")) {
std::map<unsigned int, const Token*>::iterator it = memAssignments.find(param1->varId()); if (tok->str() == "memset" && initialized.find(param1->varId()) == initialized.end() && param1->variable() && param1->variable()->isLocal() && param1->variable()->isArray())
if (it == memAssignments.end()) initialized.insert(param1->varId());
else if (memAssignments.find(param1->varId()) == memAssignments.end())
memAssignments[param1->varId()] = tok; memAssignments[param1->varId()] = tok;
else { else {
const std::map<unsigned int, const Token*>::iterator it = memAssignments.find(param1->varId());
if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok) && warning) if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok) && warning)
redundantCopyInSwitchError(it->second, tok, param1->str()); redundantCopyInSwitchError(it->second, tok, param1->str());
else if (performance) else if (performance)
@ -1154,46 +1170,6 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
"selfAssignment", "Redundant assignment of '" + varname + "' to itself."); "selfAssignment", "Redundant assignment of '" + varname + "' to itself.");
} }
//---------------------------------------------------------------------------
// int a = 1;
// assert(a = 2); // <- assert should not have a side-effect
//---------------------------------------------------------------------------
static inline const Token *findAssertPattern(const Token *start)
{
return Token::findmatch(start, "assert ( %any%");
}
void CheckOther::checkAssignmentInAssert()
{
if (!_settings->isEnabled("warning"))
return;
const Token *tok = findAssertPattern(_tokenizer->tokens());
const Token *endTok = tok ? tok->next()->link() : NULL;
while (tok && endTok) {
for (tok = tok->tokAt(2); tok != endTok; tok = tok->next()) {
if (tok->isName() && (tok->next()->isAssignmentOp() || tok->next()->type() == Token::eIncDecOp))
assignmentInAssertError(tok, tok->str());
else if (Token::Match(tok, "--|++ %var%"))
assignmentInAssertError(tok, tok->strAt(1));
}
tok = findAssertPattern(endTok->next());
endTok = tok ? tok->next()->link() : NULL;
}
}
void CheckOther::assignmentInAssertError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,
"assignmentInAssert", "Assert statement modifies '" + varname + "'.\n"
"Variable '" + varname + "' is modified insert assert statement. "
"Assert statements are removed from release builds so the code inside "
"assert statement is not executed. If the code is needed also in release "
"builds, this is a bug.");
}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// if ((x != 1) || (x != 3)) // expression always true // if ((x != 1) || (x != 3)) // expression always true
// if ((x == 1) && (x == 3)) // expression always false // if ((x == 1) && (x == 3)) // expression always false

101
test/testother.cpp
View File

@ -106,9 +106,6 @@ private:
TEST_CASE(trac2071); TEST_CASE(trac2071);
TEST_CASE(trac2084); TEST_CASE(trac2084);
TEST_CASE(trac3693); TEST_CASE(trac3693);
TEST_CASE(assignmentInAssert);
TEST_CASE(modulo); TEST_CASE(modulo);
TEST_CASE(incorrectLogicOperator1); TEST_CASE(incorrectLogicOperator1);
@ -1759,6 +1756,12 @@ private:
" }\n" " }\n"
"}", 0, false, false, false, false); "}", 0, false, false, false, false);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f() {\n" // Ticket #4356
" int x = 0;\n" // <- ignore assignment with 0
" x = 3;\n"
"}", 0, false, false, false, false);
ASSERT_EQUALS("", errout.str());
} }
void switchRedundantOperationTest() { void switchRedundantOperationTest() {
@ -3229,62 +3232,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void assignmentInAssert() {
check("void f() {\n"
" int a; a = 0;\n"
" assert(a = 2);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
check("void f(int a) {\n"
" assert(a == 2);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int a, int b) {\n"
" assert(a == 2 && b = 1);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Assert statement modifies 'b'.\n", errout.str());
check("void f() {\n"
" int a; a = 0;\n"
" assert(a += 2);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
check("void f() {\n"
" int a; a = 0;\n"
" assert(a *= 2);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
check("void f() {\n"
" int a; a = 0;\n"
" assert(a -= 2);\n"
" return a;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
check("void f() {\n"
" int a = 0;\n"
" assert(a--);\n" // don't simplify and verify
" return a;\n"
"}\n", 0, false, false, false, false
);
ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
}
void modulo() { void modulo() {
check("bool f(bool& b1, bool& b2, bool& b3) {\n" check("bool f(bool& b1, bool& b2, bool& b3) {\n"
" b1 = a % 5 == 4;\n" " b1 = a % 5 == 4;\n"
@ -5637,8 +5584,7 @@ private:
" memcpy(a, b, 5);\n" " memcpy(a, b, 5);\n"
" memmove(a, b, 5);\n" " memmove(a, b, 5);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n" ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (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: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: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()); "[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());
@ -5857,24 +5803,6 @@ private:
" return x + 1;\n" " return x + 1;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f() {\n" // Ticket #4356
" int x = 0;\n" // <- ignore assignment with 0
" x = 3;\n"
"}", 0, false, false, false, false);
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int i = 54;\n"
" i = 0;\n"
"}", 0, false, false, false, false);
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("void f() {\n"
" int i = 54;\n"
" i = 1;\n"
"}", 0, false, false, false, false);
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());
} }
void redundantMemWrite() { void redundantMemWrite() {
@ -5961,6 +5889,21 @@ private:
" memset(a, 0, size);\n" " memset(a, 0, size);\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// #4455 - initialization of local buffer
check("void f(void) {"
" char buf[10];\n"
" memset(buf, 0, 10);\n"
" strcpy(buf, string);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(void) {\n"
" char buf[10] = {0};\n"
" memset(buf, 0, 10);\n"
" strcpy(buf, string);\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'buf' is being written before its old content has been used.\n", errout.str());
} }
void varFuncNullUB() { // #4482 void varFuncNullUB() { // #4482