From f8a4fb91fe76de32c2f1942482a0ba2312f9cb33 Mon Sep 17 00:00:00 2001 From: Pranav Khanna Date: Mon, 3 Mar 2014 18:27:45 +0100 Subject: [PATCH] Fixed #3796 (new check: redundant initialization with empty string) --- lib/checkstl.cpp | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/checkstl.h | 10 +++++- test/teststl.cpp | 67 +++++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e96799bdd..669758bc8 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1553,3 +1553,90 @@ void CheckStl::dereferenceInvalidIteratorError(const Token* deref, const std::st "derefInvalidIterator", "Possible dereference of an invalid iterator: " + iterName + "\n" + "Make sure to check that the iterator is valid before dereferencing it - not after."); } + + + + + +void CheckStl::readingEmptyStlContainer() +{ + if (!_settings->isEnabled("style")) + return; + + if (!_settings->inconclusive) // check only if inconclusive true; + return; + + std::map empty; //map to keep track of whether the StlContainer has been given value atleast once. + + static const char *STL_CONTAINERS[] = {"map", "vector"}; + + const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); + std::size_t varListSize = symbolDatabase->getVariableListSize(); + for (std::size_t i = 1; i < varListSize; ++i) { + // to include stl containers declared in file in used map + const Variable* var = symbolDatabase->getVariableFromVarId(i); + if (var && var->isLocal() && var->isStlType(STL_CONTAINERS)) + empty[var->declarationId()] = true; + } + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + bool stl1_exist = false; + bool stl2_exist = false; + bool stl2_isEmpty = true; + + if (tok->variable() && tok->variable()->isStlType(STL_CONTAINERS)) { + if (empty.find(tok->varId()) != empty.end()) + stl1_exist = true; + } + + // to check for various conditions for the way stl containers and variables can be used + if (Token::Match(tok, "%var% =|[")) { + const Token *tok2; + + if (Token::Match(tok->next(), "=")) + tok2 = tok->tokAt(2); + + // to check cases like Cmap[1]; or i = Cmap[1] -- the right wld evaluate true when token reaches it. + else if (Token::Match(tok->next()," [") && Token::Match(tok->linkAt(1),"] =")) + tok2 = tok->next()->link()->tokAt(2); + + else + continue; + + // rhs variable + if (tok2->variable() && tok2->variable()->isStlType(STL_CONTAINERS)) { + if (empty.find(tok2->varId()) != empty.end()) { + stl2_exist = true; + stl2_isEmpty = empty[tok2->varId()]; + } + } + + // if both var are Stl Container... + if (stl1_exist && stl2_exist) { + if (stl2_isEmpty) + readingEmptyStlContainerError(tok2); + else + empty[tok->varId()] = false; + } + + // if only second var is Stl Container + else if (stl2_exist && stl2_isEmpty) + readingEmptyStlContainerError(tok); + + // if only first var is stl container + else if (stl1_exist) + empty[tok->varId()] = false; + } else if (stl1_exist && Token::Match(tok, "%var% [ %any% ] = %any%")) { + empty[tok->varId()] = false; + } else if (stl1_exist && Token::Match(tok, "%var% . %type% (")) { + //static const char STL_CONTAINER_INSERT_FUNCTIONS[] = "%var% . insert|push_back|push_front|assign|set|reset|emplace_after|insert_after|push ("; + empty[tok->varId()] = false; + } + } +} + +void CheckStl::readingEmptyStlContainerError(const Token *tok) +{ + reportError(tok, Severity::style, "reademptycontainer", "Reading from empty STL container", true); +} + diff --git a/lib/checkstl.h b/lib/checkstl.h index 91103aa6e..a1b86e2df 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -66,6 +66,7 @@ public: checkStl.size(); checkStl.redundantCondition(); checkStl.missingComparison(); + checkStl.readingEmptyStlContainer(); } @@ -146,6 +147,9 @@ public: */ void dereferenceErasedError(const Token* erased, const Token* deref, const std::string &itername); + /** @brief Reading from empty stl container */ + void readingEmptyStlContainer(); + private: /** @@ -184,6 +188,8 @@ private: void dereferenceInvalidIteratorError(const Token* deref, const std::string &itername); + void readingEmptyStlContainerError(const Token *tok); + void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckStl c(0, settings, errorLogger); c.invalidIteratorError(0, "iterator"); @@ -211,6 +217,7 @@ private: c.uselessCallsEmptyError(0); c.uselessCallsRemoveError(0, "remove"); c.dereferenceInvalidIteratorError(0, "i"); + c.readingEmptyStlContainerError(0); } static std::string myName() { @@ -230,7 +237,8 @@ private: "* common mistakes when using string::c_str()\n" "* using auto pointer (auto_ptr)\n" "* useless calls of string and STL functions\n" - "* dereferencing an invalid iterator\n"; + "* dereferencing an invalid iterator\n" + "* reading from empty STL container\n"; } }; /// @} diff --git a/test/teststl.cpp b/test/teststl.cpp index c4ca10451..1c523d98c 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -128,6 +128,7 @@ private: TEST_CASE(dereferenceInvalidIterator); TEST_CASE(dereference_auto); + TEST_CASE(readingEmptyStlContainer); } void check(const char code[], const bool inconclusive=false) { @@ -2394,6 +2395,72 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void readingEmptyStlContainer() { + check("void f() {\n" + " std::map CMap;\n" + " std::string strValue = CMap[1]; \n" + " std::cout << strValue << CMap.size() << std::endl;\n" + "}\n",true); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container\n", errout.str()); + + check("void f() {\n" + " std::map CMap;\n" + " std::string strValue = CMap[1];" + "}\n",true); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container\n", errout.str()); + + check("void f() {\n" + " std::map CMap;\n" + " CMap[1] = \"123\";\n" + " std::string strValue = CMap[1];" + "}\n",true); + ASSERT_EQUALS("", errout.str()); + + check("std::vector f() {\n" + " try {\n" + " std::vector Vector;\n" + " std::vector v2 = Vector;\n" + " std::string strValue = v2[1]; \n" + " }\n" + " return Vector;\n" + "}\n",true); + ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Reading from empty STL container\n" + "[test.cpp:5]: (style, inconclusive) Reading from empty STL container\n", errout.str()); + + check("f() {\n" + " try {\n" + " std::vector Vector;\n" + " Vector.push_back(\"123\");\n" + " std::vector v2 = Vector;\n" + " std::string strValue = v2[0]; \n" + " }\n" + " return Vector;\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " std::map mymap;\n" + " mymap[\"Bakery\"] = \"Barbara\";\n" + " std:string bakery_name = mymap[\"Bakery\"];\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " std::vector v;\n" + " v.insert(1);\n" + " int i = v[0];\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " std::map CMap;\n" + " std::string strValue = CMap[1];\n" + " std::string strValue2 = CMap[1];\n" + "}\n", true); + ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Reading from empty STL container\n" + "[test.cpp:4]: (style, inconclusive) Reading from empty STL container\n", errout.str()); + } }; REGISTER_TEST(TestStl)