From cfdf2b8cfe3502f8d99df6efa6fcf3462299c0da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <hyd_danmar@users.sourceforge.net>
Date: Tue, 29 Dec 2009 09:17:07 +0100
Subject: [PATCH] Fixed #1160 (Null pointer dereference vs. goto)

---
 lib/executionpath.cpp | 26 ++++++++------------------
 lib/executionpath.h   | 31 +++++++++++++++++++++++--------
 test/testother.cpp    | 16 ++++++++++++----
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/lib/executionpath.cpp b/lib/executionpath.cpp
index 774c88449..72b9724ca 100644
--- a/lib/executionpath.cpp
+++ b/lib/executionpath.cpp
@@ -56,16 +56,6 @@ bool ExecutionPath::parseCondition(const Token &tok, std::list<ExecutionPath *>
 }
 
 
-static void dealloc_checks(std::list<ExecutionPath *> &checks)
-{
-    while (!checks.empty())
-    {
-        delete checks.back();
-        checks.pop_back();
-    }
-}
-
-
 static const Token *checkExecutionPaths_(const Token *tok, std::list<ExecutionPath *> &checks)
 {
     const std::auto_ptr<ExecutionPath> check(checks.front()->copy());
@@ -110,8 +100,8 @@ static const Token *checkExecutionPaths_(const Token *tok, std::list<ExecutionPa
                 // parse condition
                 if (checks.size() > 10 || check->parseCondition(*tok->next(), checks))
                 {
-                    dealloc_checks(checks);
-                    dealloc_checks(newchecks);
+                    ExecutionPath::bailOut(checks);
+                    ExecutionPath::bailOut(newchecks);
                     return 0;
                 }
 
@@ -123,8 +113,8 @@ static const Token *checkExecutionPaths_(const Token *tok, std::list<ExecutionPa
 
                 if (!Token::simpleMatch(tok, "{"))
                 {
-                    dealloc_checks(checks);
-                    dealloc_checks(newchecks);
+                    ExecutionPath::bailOut(checks);
+                    ExecutionPath::bailOut(newchecks);
                     return 0;
                 }
 
@@ -137,8 +127,8 @@ static const Token *checkExecutionPaths_(const Token *tok, std::list<ExecutionPa
                     const Token *tokerr = checkExecutionPaths_(tok->next(), c);
                     if (tokerr)
                     {
-                        dealloc_checks(c);
-                        dealloc_checks(newchecks);
+                        ExecutionPath::bailOut(c);
+                        ExecutionPath::bailOut(newchecks);
                         return tokerr;
                     }
                     while (!c.empty())
@@ -164,14 +154,14 @@ static const Token *checkExecutionPaths_(const Token *tok, std::list<ExecutionPa
                 const Token *tokerr = checkExecutionPaths_(tok->next(), checks);
                 if (tokerr)
                 {
-                    dealloc_checks(newchecks);
+                    ExecutionPath::bailOut(newchecks);
                     return tokerr;
                 }
 
                 tok = tok->link();
                 if (!tok)
                 {
-                    dealloc_checks(newchecks);
+                    ExecutionPath::bailOut(newchecks);
                     return 0;
                 }
             }
diff --git a/lib/executionpath.h b/lib/executionpath.h
index d89b08373..00c802e4c 100644
--- a/lib/executionpath.h
+++ b/lib/executionpath.h
@@ -62,11 +62,13 @@ public:
      * bail out all execution paths
      * @param checks the execution paths to bail out on
      **/
-    static void bailOut(const std::list<ExecutionPath *> &checks)
+    static void bailOut(std::list<ExecutionPath *> &checks)
     {
-        std::list<ExecutionPath *>::const_iterator it;
-        for (it = checks.begin(); it != checks.end(); ++it)
-            (*it)->bailout_ = true;
+        while (!checks.empty())
+        {
+            delete checks.back();
+            checks.pop_back();
+        }
     }
 
     /**
@@ -74,11 +76,24 @@ public:
      * @param checks the execution paths to bail out on
      * @param varid the specific variable id
      **/
-    static void bailOutVar(const std::list<ExecutionPath *> &checks, const unsigned int varid)
+    static void bailOutVar(std::list<ExecutionPath *> &checks, const unsigned int varid)
     {
-        std::list<ExecutionPath *>::const_iterator it;
-        for (it = checks.begin(); it != checks.end(); ++it)
-            (*it)->bailout_ |= ((*it)->varId == varid);
+        if (varid == 0)
+            return;
+
+        std::list<ExecutionPath *>::iterator it = checks.begin();
+        while (it != checks.end())
+        {
+            if ((*it)->varId == varid)
+            {
+                delete *it;
+                checks.erase(it++);
+            }
+            else
+            {
+                ++it;
+            }
+        }
     }
 
     /**
diff --git a/test/testother.cpp b/test/testother.cpp
index daa0f393b..478c432ff 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -932,6 +932,14 @@ private:
                          "    p->abcd();\n"
                          "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        checkNullPointer("static void foo(int a)\n"
+                         "{\n"
+                         "    Foo *p = 0;\n"
+                         "    if (a && p)\n"
+                         "        p->do_something();\n"
+                         "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void nullpointer7()
@@ -1406,10 +1414,10 @@ private:
                        "}\n");
         ASSERT_EQUALS("", errout.str());
 
-        checkUninitVar("void foo()\n"
-                       "{\n"
-                       "    const std::string s(x());\n"
-                       "    strchr(s.c_str(), ',');\n"
+        checkUninitVar("void foo()\n"
+                       "{\n"
+                       "    const std::string s(x());\n"
+                       "    strchr(s.c_str(), ',');\n"
                        "}\n");
         ASSERT_EQUALS("", errout.str());
     }