Fixed #1173 (Improve check: memory leak not detected in constructor)

lib/checkmemoryleak.cpp

@@ -1957,25 +1957,26 @@ void CheckMemoryLeakInFunction::checkScope(const Token *Tok1, const std::string
 // Checks for memory leaks inside function..
 //---------------------------------------------------------------------------
 
-void CheckMemoryLeakInFunction::check()
+void CheckMemoryLeakInFunction::parseFunctionScope(const Token *tok, const bool classmember)
 {
-    // Parse the tokens and fill the "noreturn"
+    // Check locking/unlocking of global resources..
-    parse_noreturn();
+    checkScope(tok->next(), "", 0, classmember, 1);
 
-    bool classmember = false;
+    // Locate variable declarations and check their usage..
-    bool beforeParameters = false;
+    unsigned int indentlevel = 0;
-    bool infunc = false;
+    do
-    int indentlevel = 0;
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
         if (tok->str() == "{")
             ++indentlevel;
-
         else if (tok->str() == "}")
+        {
+            if (indentlevel <= 1)
+                break;
             --indentlevel;
+        }
 
         // Skip these weird blocks... "( { ... } )"
-        else if (Token::simpleMatch(tok, "( {"))
+        if (Token::simpleMatch(tok, "( {"))
         {
             tok = tok->link();
             if (!tok)
@@ -1983,64 +1984,75 @@ void CheckMemoryLeakInFunction::check()
             continue;
         }
 
-        // In function..
+        if (!Token::Match(tok, "[{};] %type%"))
-        if (indentlevel == 0)
+            continue;
+
+        // Don't check static/extern variables
+        if (Token::Match(tok->next(), "static|extern"))
+            continue;
+
+        // return/else is not part of a variable declaration..
+        if (Token::Match(tok->next(), "return|else"))
+            continue;
+
+        unsigned int sz = _tokenizer->sizeOfType(tok->next());
+        if (sz < 1)
+            sz = 1;
+
+        if (Token::Match(tok, "[{};] %type% * const| %var% [;=]"))
         {
-            if (Token::simpleMatch(tok, ") {"))
+            const Token *vartok = tok->tokAt(tok->tokAt(3)->str() != "const" ? 3 : 4);
-            {
+            checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
-                infunc = true;
-                checkScope(tok->tokAt(2), "", 0, classmember, 1);
-            }
-
-            else if (tok->str() == "(")
-                beforeParameters = false;
-
-            else if (tok->str() == "::" && beforeParameters)
-                classmember = true;
-
-            else if (Token::Match(tok, "[;}]"))
-            {
-                infunc = classmember = false;
-                beforeParameters = true;
-            }
         }
 
-        // Declare a local variable => Check
+        else if (Token::Match(tok, "[{};] %type% %type% * const| %var% [;=]"))
-        if (indentlevel > 0 && infunc)
         {
-            unsigned int sz = _tokenizer->sizeOfType(tok->tokAt(1));
+            const Token *vartok = tok->tokAt(tok->tokAt(4)->str() != "const" ? 4 : 5);
-            if (sz < 1)
+            checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
-                sz = 1;
-
-            if (!Token::Match(tok, "[{};] %type%"))
-                continue;
-
-            // Don't check static/extern variables
-            if (Token::Match(tok->next(), "static|extern"))
-                continue;
-
-            // return/else is not part of a variable declaration..
-            if (Token::Match(tok->next(), "return|else"))
-                continue;
-
-            if (Token::Match(tok, "[{};] %type% * const| %var% [;=]"))
-            {
-                const Token *vartok = tok->tokAt(tok->tokAt(3)->str() != "const" ? 3 : 4);
-                checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
-            }
-
-            else if (Token::Match(tok, "[{};] %type% %type% * const| %var% [;=]"))
-            {
-                const Token *vartok = tok->tokAt(tok->tokAt(4)->str() != "const" ? 4 : 5);
-                checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
-            }
-
-            else if (Token::Match(tok, "[{};] int %var% [;=]"))
-            {
-                const Token *vartok = tok->tokAt(2);
-                checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
-            }
         }
+
+        else if (Token::Match(tok, "[{};] int %var% [;=]"))
+        {
+            const Token *vartok = tok->tokAt(2);
+            checkScope(tok->next(), vartok->str(), vartok->varId(), classmember, sz);
+        }
+    }
+    while (0 != (tok = tok->next()));
+}
+
+void CheckMemoryLeakInFunction::check()
+{
+    // Parse the tokens and fill the "noreturn"
+    parse_noreturn();
+
+    bool classmember = false;
+    bool beforeParameters = false;
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        // Found a function scope
+        if (Token::Match(tok, ") const| {"))
+        {
+            tok = tok->next();
+            if (tok->str() != "{")
+                tok = tok->next();
+            parseFunctionScope(tok, classmember);
+            tok = tok->link();
+            continue;
+        }
+
+        else if (tok->str() == "(")
+            beforeParameters = false;
+
+        else if (tok->str() == "::" && beforeParameters)
+            classmember = true;
+
+        else if (Token::Match(tok, "[;}]"))
+        {
+            classmember = false;
+            beforeParameters = true;
+        }
+
+
     }
 }
 //---------------------------------------------------------------------------

lib/checkmemoryleak.h

@@ -168,6 +168,13 @@ private:
 #endif
 
 
+    /**
+     * Check all variables in function scope
+     * @param tok The first '{' token of the function scope
+     * @param classmember Is this function a class member?
+     */
+    void parseFunctionScope(const Token *tok, const bool classmember);
+
     bool matchFunctionsThatReturnArg(const Token *tok, unsigned int varid) const;
 
     /**

test/testmemleak.cpp

@@ -322,6 +322,9 @@ private:
         TEST_CASE(vcl1);
         TEST_CASE(vcl2);
 
+        // detect leak in class member function..
+        TEST_CASE(class1);
+
         TEST_CASE(autoptr1);
         TEST_CASE(if_with_and);
         TEST_CASE(assign_pclose);
@@ -2079,6 +2082,20 @@ private:
     }
 
 
+    void class1()
+    {
+        check("class Fred\n"
+              "{\n"
+              "public:\n"
+              "    Fred()\n"
+              "    {\n"
+              "        int *p = new int[100];\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Memory leak: p\n", errout.str());
+    }
+
+
     void autoptr1()
     {
         check("std::auto_ptr<int> foo()\n"