From 825dce5c4e37ab30b519dee3433cc09147f13767 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Tue, 2 Aug 2011 11:20:09 +0200
Subject: [PATCH] Fixed #2954 (False negative: Null pointer dereference not
 detected '*p=4; if (p) { }')

---
 lib/checknullpointer.cpp | 19 +++++++++++++++----
 test/testnullpointer.cpp | 18 +++++++-----------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
index cf529a37f..3c4d42621 100644
--- a/lib/checknullpointer.cpp
+++ b/lib/checknullpointer.cpp
@@ -513,15 +513,19 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
         // TODO: false negatives.
         // - logical operators
         // - while
-        if (tok->str() == "if" && Token::Match(tok->previous(), "; if ( ! %var% )"))
+        if (tok->str() == "if" && Token::Match(tok->previous(), "; if ( !| %var% )"))
         {
+            const Token * vartok = tok->tokAt(2);
+            if (vartok->str() == "!")
+                vartok = vartok->next();
+
             // Variable id for pointer
-            const unsigned int varid(tok->tokAt(3)->varId());
+            const unsigned int varid(vartok->varId());
             if (varid == 0)
                 continue;
 
             // Name of pointer
-            const std::string varname(tok->strAt(3));
+            const std::string varname(vartok->str());
 
             // Check that variable is a pointer..
             if (!isPointer(varid))
@@ -535,8 +539,10 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
                 if (tok1->str() == ")" && Token::Match(tok1->link()->previous(), "%var% ("))
                 {
                     const Token *tok2 = tok1->link();
-                    while (tok2 && !Token::Match(tok2, "[;{}]"))
+                    while (tok2 && !Token::Match(tok2, "[;{}?:]"))
                         tok2 = tok2->previous();
+                    if (Token::Match(tok2, "[?:]"))
+                        break;
                     if (Token::Match(tok2, "[;{}] %varid% = %var%", varid))
                         break;
 
@@ -559,6 +565,11 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
                             break;
                         }
                     }
+
+                    // calling unknown function => it might initialize the pointer
+                    const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid);
+                    if (!var || !(var->isLocal() || var->isArgument()))
+                        break;
                 }
 
                 if (tok1->str() == "break")
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index f6bff5771..df1ef762a 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -306,17 +306,6 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         // loops..
-        check("void freeAbc(struct ABC *abc)\n"
-              "{\n"
-              "    while (abc)\n"
-              "    {\n"
-              "        struct ABC *next = abc->next;\n"
-              "        if (abc) delete abc;\n"
-              "        abc = next;\n"
-              "    }\n"
-              "}\n");
-        ASSERT_EQUALS("", errout.str());
-
         check("void foo(struct ABC *abc)\n"
               "{\n"
               "    int a = abc->a;"
@@ -410,6 +399,13 @@ private:
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str());
 
+        check("void foo(int *p)\n"
+              "{\n"
+              "    *p = 0;\n"
+              "    if (p) { }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str());
+
         check("void foo(int *p)\n"
               "{\n"
               "    bar(*p);\n"