dangerous usage of erase: added check

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);
+}
+
+

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);
 };
 
 //---------------------------------------------------------------------------

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

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<ErrorLogger::ErrorMessage::FileLocation> &callStack);
 

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)

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"));