Execution Path: some refactorings of the checking

lib/checkother.cpp

@@ -1124,11 +1124,21 @@ class CheckNullpointer : public ExecutionPath
 {
 public:
     // Startup constructor
-    CheckNullpointer(unsigned int v) : ExecutionPath(), varId(v), null(false)
+    CheckNullpointer(CheckOther *c) : ExecutionPath(), varId(0), null(false), checkOther(c)
     {
     }
 
 private:
+    // Create checking of specific variable:
+    CheckNullpointer(CheckOther *c, const unsigned int id, const std::string &name)
+            : ExecutionPath(),
+            varId(id),
+            varname(name),
+            null(false),
+            checkOther(c)
+    {
+    }
+
     ExecutionPath *copy()
     {
         return new CheckNullpointer(*this);
@@ -1138,41 +1148,73 @@ private:
     void operator=(const CheckNullpointer &);
 
     const unsigned int varId;
+    const std::string varname;
     bool null;
+    CheckOther * const checkOther;
 
-    static void setnull(std::list<ExecutionPath *> &checks)
+    static void setnull(std::list<ExecutionPath *> &checks, const unsigned int varid)
-    {
-        std::list<ExecutionPath *>::iterator it;
-        for (it = checks.begin(); it != checks.end(); ++it)
-            dynamic_cast<CheckNullpointer *>(*it)->null = true;
-    }
-
-    static void dereference(bool &foundError, std::list<ExecutionPath *> &checks)
     {
         std::list<ExecutionPath *>::iterator it;
         for (it = checks.begin(); it != checks.end(); ++it)
         {
-            if (dynamic_cast<CheckNullpointer *>(*it)->null)
+            CheckNullpointer *c = dynamic_cast<CheckNullpointer *>(*it);
+            if (c && c->varId == varid)
+                c->null = true;
+        }
+    }
+
+    static void dereference(bool &foundError, std::list<ExecutionPath *> &checks, const Token *tok)
+    {
+        const unsigned int varid(tok->varId());
+
+        std::list<ExecutionPath *>::iterator it;
+        for (it = checks.begin(); it != checks.end(); ++it)
+        {
+            CheckNullpointer *c = dynamic_cast<CheckNullpointer *>(*it);
+            if (c && c->varId == varid && c->null)
             {
                 foundError = true;
+                c->checkOther->nullPointerError(tok, c->varname);
                 break;
             }
         }
     }
 
+    static void bailOutVar(std::list<ExecutionPath *> &checks, const unsigned int varid)
+    {
+        std::list<ExecutionPath *>::iterator it;
+        for (it = checks.begin(); it != checks.end(); ++it)
+        {
+            CheckNullpointer *c = dynamic_cast<CheckNullpointer *>(*it);
+            if (c && c->varId == varid)
+            {
+                c->bailOut(true);
+            }
+        }
+    }
+
     const Token *parse(const Token &tok, bool &foundError, std::list<ExecutionPath *> &checks) const
     {
-        if (tok.varId() == varId)
+        if (Token::Match(tok.previous(), "[;{}] %type% * %var% ;"))
         {
-            if (Token::Match(tok.previous(), "[;{}=] %varid% = 0 ;", varId))
+            const Token * vartok = tok.tokAt(2);
-                setnull(checks);
+            if (vartok->varId() != 0)
-            else if (Token::Match(tok.tokAt(-2), "[;{}=] * %varid%", varId))
+                checks.push_back(new CheckNullpointer(checkOther, vartok->varId(), vartok->str()));
-                dereference(foundError, checks);
+            return vartok->next();
-            else if (Token::Match(tok.next(), ". %var%"))
-                dereference(foundError, checks);
-            else
-                bailOut(checks);
         }
+
+        if (tok.varId() != 0)
+        {
+            if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;"))
+                setnull(checks, tok.varId());
+            else if (Token::Match(tok.tokAt(-2), "[;{}=] * %var%"))
+                dereference(foundError, checks, &tok);
+            else if (Token::Match(tok.next(), ". %var%"))
+                dereference(foundError, checks, &tok);
+            else
+                bailOutVar(checks, tok.varId());
+        }
+
         return &tok;
     }
 };
@@ -1386,6 +1428,18 @@ void CheckOther::executionPaths()
     const Token *tok = _tokenizer->tokens();
     while (0 != (tok = Token::findmatch(tok, ") const| {")))
     {
+        // check for null pointer errors..
+        {
+            std::list<ExecutionPath *> checks;
+            checks.push_back(new CheckNullpointer(this));
+            checkExecutionPaths(tok->next(), checks);
+            while (!checks.empty())
+            {
+                delete checks.back();
+                checks.pop_back();
+            }
+        }
+
         // Scan through this scope and check all variables..
         unsigned int indentlevel = 0;
         for (; tok; tok = tok->next())
@@ -1477,21 +1531,6 @@ void CheckOther::executionPaths()
                     if (tokerr)
                         uninitvarError(tokerr, tok->str());
                 }
-
-                // check if variable is accessed uninitialized..
-                if (pointer)
-                {
-                    std::list<ExecutionPath *> checks;
-                    checks.push_back(new CheckNullpointer(tok->varId()));
-                    const Token *tokerr = checkExecutionPaths(tok->next(), checks);
-                    while (!checks.empty())
-                    {
-                        delete checks.back();
-                        checks.pop_back();
-                    }
-                    if (tokerr)
-                        nullPointerError(tokerr, tok->str());
-                }
             }
         }
     }

lib/executionpath.h

@@ -30,7 +30,13 @@ class Token;
 class ExecutionPath
 {
 private:
-    mutable bool bailout_;
+    bool bailout_;
+
+protected:
+    void bailOut(bool b)
+    {
+        bailout_ |= b;
+    }
 
 public:
     ExecutionPath() : bailout_(false)

test/testmemleak.cpp

@@ -40,6 +40,8 @@ private:
     {
         TEST_CASE(test1);
         TEST_CASE(test2);
+        TEST_CASE(test3);
+        TEST_CASE(test4);
     }
 
     void check(const char code[])
@@ -78,6 +80,32 @@ private:
               "}\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void test3()
+    {
+        check("void foo(int x)\n"
+              "{\n"
+              "    char *p = 0;\n"
+              "    if (x == 1)\n"
+              "        p = new char[100];\n"
+              "    if (x == 2)\n"
+              "        delete [] p;\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory leak: p\n", errout.str());
+    }
+
+    void test4()
+    {
+        check("void foo(int x)\n"
+              "{\n"
+              "    char *p = 0;\n"
+              "    if (x == 1)\n"
+              "        p = new char[100];\n"
+              "    if (x == 1)\n"
+              "        delete [] p;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 static TestLocalLeaks testLocalLeaks;