diff --git a/src/checkstl.cpp b/src/checkstl.cpp index e21454e6f..a9b14038e 100644 --- a/src/checkstl.cpp +++ b/src/checkstl.cpp @@ -18,6 +18,10 @@ */ #include "checkstl.h" +#include "errorlogger.h" +#include "token.h" +#include "tokenize.h" + CheckStl::CheckStl(const Tokenizer *tokenizer, ErrorLogger *errorLogger) : _tokenizer(tokenizer), _errorLogger(errorLogger) @@ -124,3 +128,78 @@ void CheckStl::stlOutOfBounds() } } + + + +void CheckStl::erase() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "for (")) + { + for (const Token *tok2 = tok->tokAt(2); tok2 && tok2->str() != ";"; tok2 = tok2->next()) + { + if (Token::Match(tok2, "%var% = %var% . begin ( ) ; %var% != %var% . end ( ) ") && + tok2->str() == tok2->tokAt(8)->str() && + tok2->tokAt(2)->str() == tok2->tokAt(10)->str()) + { + eraseCheckLoop(tok2); + break; + } + } + } + + if (Token::Match(tok, "while ( %var% != %var% . end ( )")) + { + eraseCheckLoop(tok->tokAt(2)); + } + } +} + + +void CheckStl::eraseCheckLoop(const Token *it) +{ + const Token *tok = it; + + // Search for the start of the loop body.. + int indentlevel = 1; + while (indentlevel > 0 && 0 != (tok = tok->next())) + { + if (tok->str() == "(") + ++indentlevel; + else if (tok->str() == ")") + --indentlevel; + } + + if (! Token::simpleMatch(tok, ") {")) + return; + + // Parse loop.. + // Error if it contains "erase(it)" but neither "break;" nor "it=" + indentlevel = 0; + const Token *tok2 = 0; + while (0 != (tok = tok->next())) + { + if (tok->str() == "{") + ++indentlevel; + else if (tok->str() == "}") + { + --indentlevel; + if (indentlevel < 0) + break; + } + else if (Token::simpleMatch(tok, "break ;") || Token::simpleMatch(tok, (it->str() + " =").c_str())) + { + tok2 = 0; + break; + } + else if (Token::simpleMatch(tok, ("erase ( " + it->str() + " )").c_str())) + tok2 = tok; + } + + // Write error message.. + if (tok2) + _errorLogger->erase(_tokenizer, tok2); +} + + diff --git a/src/checkstl.h b/src/checkstl.h index 141502f8f..14094b6aa 100644 --- a/src/checkstl.h +++ b/src/checkstl.h @@ -23,8 +23,9 @@ #define checkstlH //--------------------------------------------------------------------------- -#include "tokenize.h" -#include "errorlogger.h" +class ErrorLogger; +class Token; +class Tokenizer; class CheckStl { @@ -39,11 +40,27 @@ public: */ void stlOutOfBounds(); + /** + * Finds errors like this: + * for (it = foo.begin(); it != bar.end(); ++it) + */ void iterators(); + /** + * Dangerous usage of erase + */ + void erase(); + private: const Tokenizer *_tokenizer; ErrorLogger *_errorLogger; + + /** + * Helper function used by the 'erase' function + * This function parses a loop + * @param it iterator token + */ + void eraseCheckLoop(const Token *it); }; //--------------------------------------------------------------------------- diff --git a/src/cppcheck.cpp b/src/cppcheck.cpp index b955321d2..714fc4464 100644 --- a/src/cppcheck.cpp +++ b/src/cppcheck.cpp @@ -415,6 +415,9 @@ void CppCheck::checkFile(const std::string &code, const char FileName[]) if (ErrorLogger::stlOutOfBounds()) checkStl.stlOutOfBounds(); + + if (ErrorLogger::erase()) + checkStl.erase(); } Settings CppCheck::settings() const diff --git a/src/errorlogger.h b/src/errorlogger.h index eacd35030..217a3a154 100644 --- a/src/errorlogger.h +++ b/src/errorlogger.h @@ -444,6 +444,15 @@ public: return true; } + void erase(const Tokenizer *tokenizer, const Token *Location) + { + _writemsg(tokenizer, Location, "error", "Dangerous usage of erase", "erase"); + } + static bool erase() + { + return true; + } + static std::string callStackToString(const std::list &callStack); diff --git a/test/teststl.cpp b/test/teststl.cpp index a46025b61..fa2007aa3 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -40,6 +40,9 @@ private: TEST_CASE(iterator2); TEST_CASE(STLSize); TEST_CASE(STLSizeNoErr); + TEST_CASE(erase); + TEST_CASE(eraseBreak); + TEST_CASE(eraseAssign); } void check(const char code[]) @@ -121,6 +124,68 @@ private: ASSERT_EQUALS(std::string(""), errout.str()); } } + + + + + + + void checkErase(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + // Clear the error buffer.. + errout.str(""); + + // Check char variable usage.. + CheckStl checkStl(&tokenizer, this); + checkStl.erase(); + } + + + void erase() + { + checkErase("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Dangerous usage of erase\n", errout.str()); + } + + void eraseBreak() + { + checkErase("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void eraseAssign() + { + checkErase("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " it = foo.begin();\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + + }; REGISTER_TEST(TestStl) diff --git a/tools/errmsg.cpp b/tools/errmsg.cpp index f94e1bc88..072e2ca33 100644 --- a/tools/errmsg.cpp +++ b/tools/errmsg.cpp @@ -113,6 +113,7 @@ int main() // checkstl.cpp.. err.push_back(Message("iteratorUsage", Message::error, "Same iterator is used with both %1 and %2", "container1", "container2")); + err.push_back(Message("erase", Message::error, "Dangerous usage of erase"));