diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 608516f74..3bc9c61fc 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -741,35 +741,99 @@ void CheckStl::if_find() const Token* tok = i->classDef; - if (Token::Match(tok, "if ( !| %var% . find ( %any% ) )")) { + if (Token::Match(tok, "if ( !| %var% . find (")) { // goto %var% tok = tok->tokAt(2); - if (!tok->isName()) + if (tok->str() == "!") + tok = tok->next(); + if (!Token::simpleMatch(tok->linkAt(3), ") )")) + continue; + + const unsigned int varid = tok->varId(); + const Variable *var = symbolDatabase->getVariableFromVarId(varid); + if (var) { + // Is the variable a std::string or STL container? + const Token * decl = var->typeStartToken(); + + if (decl->str() == "const") + decl = decl->next(); + // stl container + if (Token::Match(decl, "std :: %var% < %type% > &| %varid%", varid)) + if_findError(tok, false); + else if (Token::Match(decl, "std :: string &| %varid%", varid)) + if_findError(tok, true); + } + } + + //check also for vector-like or pointer containers + else if (Token::Match(tok, "if ( !| * %var%") || Token::Match(tok, "if ( !| %var% [")) { + tok = tok->tokAt(2); + + const Token *tok2 = tok; + if (tok2->str() == "!") + tok2 = tok2->next(); + tok2 = tok2->next(); + if (tok2->str() == "[") + tok2 = tok2->link(); + tok2 = tok2->next(); + + if (!Token::simpleMatch(tok2, ". find (")) + continue; + if (!Token::simpleMatch(tok2->linkAt(2), ") )")) + continue; + + // goto %var% + if (tok->str() == "!") + tok = tok->next(); + if (tok->str() == "*") tok = tok->next(); const unsigned int varid = tok->varId(); const Variable *var = symbolDatabase->getVariableFromVarId(varid); if (var) { // Is the variable a std::string or STL container? - const Token * decl = var->nameToken(); - while (decl && !Token::Match(decl, "[;{}(,]")) - decl = decl->previous(); + const Token * decl = var->typeStartToken(); - if (decl) + //jump next to 'const' + if (decl->str() == "const") decl = decl->next(); + //pretty bad limitation.. but it is there in order to avoid + //own implementations of 'find' or any container + if (!Token::simpleMatch(decl, "std ::")) + continue; - // stl container - if (Token::Match(decl, "const| std :: %var% < %type% > &|*| %varid%", varid)) - if_findError(tok, false); - else if (Token::Match(decl, "const| std :: string &|*| %varid%", varid)) - if_findError(tok, true); + decl = decl->tokAt(2); + + if (Token::Match(decl, "%var% <")) { + decl = decl->tokAt(2); + //stl-like + if (Token::Match(decl, "std :: %var% < %type% >| >>|> &| %varid%", varid)) + if_findError(tok, false); + //not stl-like, then let's hope it's a pointer or an array + else if (Token::Match(decl, "%type% >")) { + decl = decl->tokAt(2); + if (Token::Match(decl, "* &| %varid%", varid) || + Token::Match(decl, "&| %varid% [ ]| %any% ]| ", varid)) + if_findError(tok, false); + } + + else if (Token::Match(decl, "std :: string > &| %varid%", varid)) + if_findError(tok, true); + } + + else if (decl && decl->str() == "string") { + decl = decl->next(); + if (Token::Match(decl, "* &| %varid%", varid) || + Token::Match(decl, "&| %varid% [ ]| %any% ]| ", varid)) + if_findError(tok, true); + } } } else if (Token::Match(tok, "if ( !| std :: find|find_if (")) { // goto '(' for the find tok = tok->tokAt(4); - if (tok->isName()) + if (tok->str() != "(") tok = tok->next(); // check that result is checked properly diff --git a/test/teststl.cpp b/test/teststl.cpp index 1a44309d6..6d0e32aa2 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1107,20 +1107,90 @@ private: // set::find // --------------------------- - // error + // error (simple) check("void f(std::set s)\n" "{\n" " if (s.find(12)) { }\n" "}\n"); ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); - // ok + // error (pointer) + check("void f(std::set *s)\n" + "{\n" + " if (*s.find(12)) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); + + // error (array-like pointer) + check("void f(std::set *s)\n" + "{\n" + " if (s[0].find(12)) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); + + // error (array) + check("void f(std::set s [10])\n" + "{\n" + " if (s[0].find(12)) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); + + // error (undefined length array) + check("void f(std::set s [])\n" + "{\n" + " if (s[0].find(12)) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); + + // error (vector) + check("void f(std::vector > s)\n" + "{\n" + " if (s[0].find(12)) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find is an iterator, but it is not properly checked.\n", errout.str()); + + // ok (simple) check("void f(std::set s)\n" "{\n" " if (s.find(123) != s.end()) { }\n" "}\n"); ASSERT_EQUALS("", errout.str()); + // ok (pointer) + check("void f(std::set *s)\n" + "{\n" + " if (*s.find(12) != s.end()) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ok (array-like pointer) + check("void f(std::set *s)\n" + "{\n" + " if (s[0].find(12) != s.end()) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ok (array) + check("void f(std::set s [10])\n" + "{\n" + " if (s[0].find(123) != s.end()) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ok (undefined length array) + check("void f(std::set s [])\n" + "{\n" + " if (s[0].find(123) != s.end()) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ok (vector) + check("void f(std::vector > s)\n" + "{\n" + " if (s[0].find(123) != s.end()) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + // --------------------------- // std::find @@ -1142,12 +1212,26 @@ private: } void if_str_find() { - // error + // error (simple) check("void f(const std::string &s)\n" "{\n" " if (s.find(\"abc\")) { }\n" "}\n"); ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str()); + + // error (pointer) + check("void f(const std::string *s)\n" + "{\n" + " if (*s.find(\"abc\")) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str()); + + // error (vector) + check("void f(const std::vector &s)\n" + "{\n" + " if (s[0].find(\"abc\")) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str()); }