Fixed cppcheck-opensource#3518 (False negative: Possible null pointer dereference (in the same condition))

eriklax · Daniel Marjamäki · commit f1dbd1a89a35 · 2012-02-11T16:15:38.000+01:00
diff --git a/AUTHORS b/AUTHORS
@@ -3,6 +3,7 @@ The cppcheck team, in alphabetical order:
 Bill Egert
 Daniel Marjamäki
 Edoardo Prezioso
+Erik Lax
 Gianluca Scacco
 Greg Hewgill
 Hoang Tuan Su
diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
@@ -839,31 +839,60 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
 
         // vartok : token for the variable
         const Token *vartok = 0;
-        if (Token::Match(tok, "( ! %var% )|&&"))
+        const Token *checkConditionStart = 0;
+        if (Token::Match(tok, "( ! %var% )|&&")) {
             vartok = tok->tokAt(2);
-        else if (Token::Match(tok, "( %var% )|&&"))
+            checkConditionStart = vartok->next();
+        } else if (Token::Match(tok, "( %var% )|&&")) {
             vartok = tok->next();
-        else if (Token::Match(tok, "( ! ( %var% ="))
+        } else if (Token::Match(tok, "( ! ( %var% =")) {
             vartok = tok->tokAt(3);
-        else
+            if (Token::Match(tok->tokAt(2)->link(), ") &&"))
+                checkConditionStart = tok->tokAt(2)->link();
+        } else
             continue;
 
         // variable id for pointer
         const unsigned int varid(vartok->varId());
         if (varid == 0)
             continue;
 
-        const Variable* var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid);
         // Check if variable is a pointer
+        const Variable* var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid);
         if (!var || !var->isPointer())
             continue;
 
         if (Token::Match(vartok->next(), "&& ( %varid% =", varid))
             continue;
 
+        // Name and line of the pointer
+        const std::string &pointerName = vartok->str();
+        const unsigned int linenr = vartok->linenr();
+
         // if this is true then it is known that the pointer is null
         bool null = true;
 
+        // Check the condition (eg. ( !x && x->i )
+        if (checkConditionStart) {
+            const Token * const conditionEnd = tok->link();
+            for (const Token *tok2 = checkConditionStart; tok2 != conditionEnd; tok2 = tok2->next()) {
+                // If we hit a || operator, abort
+                if (Token::Match(tok2, "%oror%", varid)) {
+                    break;
+                }
+                // Pointer is used
+                else if (Token::Match(tok2, "* %varid%", varid)) {
+                    nullPointerError(tok2->tokAt(2), pointerName, linenr, false);
+                    break;
+                }
+                // Pointer is used
+                else if (Token::Match(tok2, "%varid% .", varid)) {
+                    nullPointerError(tok2, pointerName, linenr, false);
+                    break;
+                }
+            }
+        }
+
         // start token = inside the if-body
         const Token *tok1 = i->classStart;
 
@@ -877,10 +906,6 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
                 continue;
         }
 
-        // Name and line of the pointer
-        const std::string &pointerName = vartok->str();
-        const unsigned int linenr = vartok->linenr();
-
         unsigned int indentlevel = 0;
 
         // Set to true if we would normally bail out the check.
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
@@ -1478,6 +1478,59 @@ class TestNullPointer : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        // check, assign and use
+        check("void f() {\n"
+              "    char *p;\n"
+              "    if (p == 0 && (p = malloc(10)) != a && (*p = a)) {\n"
+              "        *p = 0;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        // check, and use
+        check("void f() {\n"
+              "    char *p;\n"
+              "    if (p == 0 && (*p = 0)) {\n"
+              "        return;\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 3\n", errout.str());
+
+        // check, and use
+        check("void f() {\n"
+              "    struct foo *p;\n"
+              "    if (p == 0 && p->x == 10) {\n"
+              "        return;\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 3\n", errout.str());
+
+        // check, and use
+        check("void f() {\n"
+              "    struct foo *p;\n"
+              "    if (p == 0 || p->x == 10) {\n"
+              "        return;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        // check, and use
+        check("void f() {\n"
+              "    char *p;\n"
+              "    if ((p = malloc(10)) == NULL && (*p = a)) {\n"
+              "        return;\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 3\n", errout.str());
+
+        // check, and use
+        check("void f(struct X *p, int x) {\n"
+              "    if (!p && x==1 || p && p->x==0) {\n"
+              "        return;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
         {
             const char code[] = "void f(Fred *fred) {\n"
                                 "    if (fred == NULL) { }\n"
