diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 142251967..25cac8b15 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -19,7 +19,7 @@ #include "checkstl.h" #include "token.h" #include "executionpath.h" - +#include // Register this check class (by creating a static instance of it) namespace @@ -833,3 +833,74 @@ void CheckStl::redundantIfRemoveError(const Token *tok) { reportError(tok, Severity::style, "redundantIfRemove", "Redundant condition. The remove function in the STL will not do anything if element doesn't exist"); } + + + +void CheckStl::missingComparison() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::simpleMatch(tok, "for (")) + { + for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) + { + if (tok2->str() == ";") + break; + + if (!Token::Match(tok2, "%var% = %var% . begin ( ) ; %var% != %var% . end ( ) ; ++| %var% ++| ) {")) + continue; + + // same iterator name + if (tok2->str() != tok2->strAt(8)) + continue; + + // same container + if (tok2->strAt(2) != tok2->strAt(10)) + continue; + + // increment iterator + if (!Token::simpleMatch(tok2->tokAt(16), ("++ " + tok2->str() + " )").c_str()) && + !Token::simpleMatch(tok2->tokAt(16), (tok2->str() + " ++ )").c_str())) + { + continue; + } + + const std::string &itName(tok2->str()); + const Token *incrementToken = 0; + unsigned int indentlevel = 0; + // Parse loop.. + for (const Token *tok3 = tok2->tokAt(20); tok3; tok3 = tok3->next()) + { + if (tok3->str() == "{") + ++indentlevel; + else if (tok3->str() == "}") + { + if (indentlevel == 0) + break; + --indentlevel; + } + else if (tok3->str() == itName && Token::simpleMatch(tok3->next(), "++")) + incrementToken = tok3; + else if (tok3->str() == "++" && Token::simpleMatch(tok3->next(), itName.c_str())) + incrementToken = tok3; + else if (tok3->str() == itName && Token::simpleMatch(tok3->next(), "!=")) + incrementToken = 0; + } + if (incrementToken) + missingComparisonError(incrementToken, tok2->tokAt(16)); + } + } + } +} + +void CheckStl::missingComparisonError(const Token *incrementToken1, const Token *incrementToken2) +{ + std::ostringstream errmsg; + errmsg << "The iterator is incremented at line " << incrementToken1->linenr() + << " and then at line " << incrementToken2->linenr() + << ". The loop might unintentionally skip an element in the container. There is no comparison between these increments to prevent that the iterator is incremented beyond the end."; + + reportError(incrementToken1, Severity::style, "StlMissingComparison", errmsg.str()); +} + + diff --git a/lib/checkstl.h b/lib/checkstl.h index 8cbae60c7..534852a1d 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -58,6 +58,7 @@ public: // Style check checkStl.size(); checkStl.redundantCondition(); + checkStl.missingComparison(); } @@ -113,6 +114,15 @@ public: * */ void redundantCondition(); + /** + * @brief Missing inner comparison, when incrementing iterator inside loop + * Dangers: + * - may increment iterator beyond end + * - may unintentionally skip elements in list/set etc + */ + void missingComparison(); + void missingComparisonError(const Token *incrementToken1, const Token *incrementToken2); + private: /** diff --git a/test/teststl.cpp b/test/teststl.cpp index 9234a584d..b5ea268d4 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -93,6 +93,9 @@ private: // if (ints.find(123) != ints.end()) ints.remove(123); TEST_CASE(redundantCondition1); TEST_CASE(redundantCondition2); + + // missing inner comparison when incrementing iterator inside loop + TEST_CASE(missingInnerComparison1); } void check(const std::string &code) @@ -610,7 +613,7 @@ private: { check("void f(list &ints)\n" "{\n" - " for (list::iterator it = ints.begin(); it != ints.end(); ++it) {\n" + " for (list::iterator it = ints.begin(); it != ints.end();) {\n" " if (*it == 123) {\n" " list::iterator copy = it;\n" " ++copy;\n" @@ -1039,6 +1042,17 @@ private: ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. The remove function in the STL will not do anything if element doesn't exist\n", errout.str()); } + void missingInnerComparison1() + { + check("void f(std::set &ints) {\n" + " for (std::set::iterator it = ints.begin(); it != ints.end(); ++it) {\n" + " if (a) {\n" + " it++;\n" + " }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) The iterator is incremented at line 4 and then at line 2. The loop might unintentionally skip an element in the container. There is no comparison between these increments to prevent that the iterator is incremented beyond the end.\n", errout.str()); + } }; REGISTER_TEST(TestStl)