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

Refactorized CheckStl::pushback(): 

- insert(), reserve() and clear() also invalidate iterators
- Properly support return and throw statements: Bailout after semicolon
- "." is also bad usage of invalid iterator.
This commit is contained in:
PKEuS 2012-11-11 13:03:58 +01:00
parent e8d3a4300d
commit 704f285c90
2 changed files with 30 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
lib/checkstl.cpp
View File

@ -537,7 +537,7 @@ void CheckStl::pushback()
} }
// push_back on vector.. // push_back on vector..
if (Token::Match(tok2, "%varid% . push_front|push_back|resize|reserve", containerId)) { if (Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear", containerId)) {
invalidPointer = true; invalidPointer = true;
function = tok2->strAt(2); function = tok2->strAt(2);
} }
@ -588,6 +588,8 @@ void CheckStl::pushback()
// count { , } and parentheses for tok2 // count { , } and parentheses for tok2
int indent = 0; int indent = 0;
const Token* validatingToken = 0;
std::string invalidIterator; std::string invalidIterator;
for (const Token *tok2 = tok; indent >= 0 && tok2; tok2 = tok2->next()) { for (const Token *tok2 = tok; indent >= 0 && tok2; tok2 = tok2->next()) {
if (tok2->str() == "{" || tok2->str() == "(") if (tok2->str() == "{" || tok2->str() == "(")
@ -599,6 +601,11 @@ void CheckStl::pushback()
--indent; --indent;
} }
if (validatingToken == tok2) {
invalidIterator.clear();
validatingToken = 0;
}
// Using push_back or push_front inside a loop.. // Using push_back or push_front inside a loop..
if (Token::simpleMatch(tok2, "for (")) { if (Token::simpleMatch(tok2, "for (")) {
tok2 = tok2->tokAt(2); tok2 = tok2->tokAt(2);
@ -619,7 +626,7 @@ void CheckStl::pushback()
if (tok3->str() == "break" || tok3->str() == "return") { if (tok3->str() == "break" || tok3->str() == "return") {
pushbackTok = 0; pushbackTok = 0;
break; break;
} else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize (", varId)) { } else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize|clear (", varId)) {
pushbackTok = tok3->tokAt(2); pushbackTok = tok3->tokAt(2);
} }
} }
@ -640,7 +647,7 @@ void CheckStl::pushback()
} }
// push_back on vector.. // push_back on vector..
if (vectorid > 0 && Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize (", vectorid)) { if (vectorid > 0 && Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear (", vectorid)) {
if (!invalidIterator.empty() && Token::Match(tok2->tokAt(2), "insert ( %varid% ,", iteratorid)) { if (!invalidIterator.empty() && Token::Match(tok2->tokAt(2), "insert ( %varid% ,", iteratorid)) {
invalidIteratorError(tok2, invalidIterator, tok2->strAt(4)); invalidIteratorError(tok2, invalidIterator, tok2->strAt(4));
break; break;
@ -650,16 +657,18 @@ void CheckStl::pushback()
tok2 = tok2->linkAt(3); tok2 = tok2->linkAt(3);
} }
else if (tok2->str() == "return" || tok2->str() == "throw")
validatingToken = Token::findsimplematch(tok2->next(), ";");
// TODO: instead of bail out for 'else' try to check all execution paths. // TODO: instead of bail out for 'else' try to check all execution paths.
else if (tok2->str() == "return" || tok2->str() == "break" || tok2->str() == "else") { else if (tok2->str() == "break" || tok2->str() == "else")
invalidIterator.clear(); invalidIterator.clear();
}
// Using invalid iterator.. // Using invalid iterator..
if (!invalidIterator.empty()) { if (!invalidIterator.empty()) {
if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorid)) if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorid))
invalidIteratorError(tok2, invalidIterator, tok2->strAt(1)); invalidIteratorError(tok2, invalidIterator, tok2->strAt(1));
if (Token::Match(tok2, "%varid% ++|--|+|-", iteratorid)) if (Token::Match(tok2, "%varid% ++|--|+|-|.", iteratorid))
invalidIteratorError(tok2, invalidIterator, tok2->str()); invalidIteratorError(tok2, invalidIterator, tok2->str());
} }
} }

15
test/teststl.cpp
View File

@ -1142,6 +1142,21 @@ private:
" ints.insert(iter, 2);\n" " ints.insert(iter, 2);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:6]: (error) After insert(), the iterator 'iter' may be invalid.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (error) After insert(), the iterator 'iter' may be invalid.\n", errout.str());
check("void* f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
" bars.insert(Bar());\n"
" void* v = &i->foo;\n"
" return v;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
check("Foo f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
" bars.insert(Bar());\n"
" return i->foo;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
} }
void insert2() { void insert2() {