invalid pointer usage: broke out CheckOther::invalidPointer from the CheckOther::nullPointer (#485)

src/checkother.cpp

@@ -1028,7 +1028,11 @@ void CheckOther::nullPointer()
             tok2 = tok2->next();
         }
     }
+}
 
+
+void CheckOther::invalidPointer()
+{
     // Dereferencing a pointer and then checking if it's NULL..
     for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next())
     {
@@ -1037,11 +1041,16 @@ void CheckOther::nullPointer()
             if (std::string(tok1->strAt(1)) == tok1->strAt(3))
                 continue;
 
+            const unsigned int varid1(tok1->next()->varId());
+
             tok1 = tok1->tokAt(3);
-            const unsigned int varid1(tok1->varId());
+            const unsigned int varid2(tok1->varId());
-            if (varid1 == 0)
+            if (varid1 == 0 || varid2 == 0)
                 continue;
 
+            // token where the variable is dereferenced..
+            const Token * dereferenced = 0;
+
             unsigned int indentlevel2 = 0;
             for (const Token *tok2 = tok1->tokAt(3); tok2; tok2 = tok2->next())
             {
@@ -1055,13 +1064,26 @@ void CheckOther::nullPointer()
                     --indentlevel2;
                 }
 
-                else if (Token::Match(tok2, "%varid% =", varid1))
+                else if (Token::Match(tok2, "* %varid%", varid1))
+                    dereferenced = tok2;
+
+                // Reassignment of the struct
+                else if (tok2->varId() == varid1)
+                {
+                    if (Token::Match(tok2->next(), "="))
+                        break;
+                    if (Token::Match(tok2->tokAt(-2), "[,(] &"))
+                        break;
+                }
+
+                // return at base level => stop checking
+                else if (indentlevel2 == 0 && tok2->str() == "return")
                     break;
 
-                else if (tok2->str() == "if")
+                else if (dereferenced && tok2->str() == "if")
                 {
-                    if (Token::Match(tok2, "if ( !| %varid% )", varid1))
+                    if (Token::Match(tok2, "if ( !| %varid% )", varid2))
-                        nullPointerError(tok1);
+                        invalidPointerError(dereferenced);
                 }
             }
         }
@@ -1167,6 +1189,11 @@ void CheckOther::nullPointerError(const Token *tok)
     reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference");
 }
 
+void CheckOther::invalidPointerError(const Token *tok)
+{
+    reportError(tok, Severity::error, "invalidPointer", "Possible invalid pointer dereference");
+}
+
 void CheckOther::zerodivError(const Token *tok)
 {
     reportError(tok, Severity::error, "zerodiv", "Division by zero");

src/checkother.h

@@ -72,6 +72,7 @@ public:
         checkOther.strPlusChar();
         checkOther.invalidFunctionUsage();
         checkOther.nullPointer();
+        checkOther.invalidPointer();
         checkOther.checkZeroDivision();
     }
 
@@ -111,6 +112,9 @@ public:
     /** possible null pointer dereference */
     void nullPointer();
 
+    /** possible invalid pointer dereference */
+    void invalidPointer();
+
     /** Check zero division*/
     void checkZeroDivision();
 
@@ -141,6 +145,7 @@ private:
     void conditionAlwaysTrueFalse(const Token *tok, const std::string &truefalse);
     void strPlusChar(const Token *tok);
     void nullPointerError(const Token *tok);
+    void invalidPointerError(const Token *tok);
     void zerodivError(const Token *tok);
 
     void getErrorMessages()
@@ -163,6 +168,7 @@ private:
         conditionAlwaysTrueFalse(0, "true/false");
         strPlusChar(0);
         nullPointerError(0);
+        invalidPointerError(0);
         zerodivError(0);
     }
 
@@ -186,6 +192,7 @@ private:
                " * condition that is always true/false\n"
                " * unusal pointer arithmetic. For example: \"abc\" + 'd'\n"
                " * dereferencing a null pointer\n"
+               " * dereferencing an invalid pointer\n"
                " * [[IncompleteStatement|Incomplete statement]]\n";
     }
 };

test/testother.cpp

@@ -58,9 +58,8 @@ private:
 
         TEST_CASE(nullpointer1);
         TEST_CASE(nullpointer2);
-        TEST_CASE(nullpointer3);
+
-        TEST_CASE(nullpointer4);
+        TEST_CASE(invalidpointer);
-        TEST_CASE(nullpointer5);
 
         TEST_CASE(oldStylePointerCast);
     }
@@ -447,32 +446,40 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
-    void nullpointer3()
+
+
+    void checkInvalidPointer(const char code[])
     {
-        checkNullPointer("void foo(struct ABC *abc)\n"
+        // Tokenize..
+        Tokenizer tokenizer;
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.setVarId();
+
+        // Clear the error buffer..
+        errout.str("");
+
+        // Check for redundant code..
+        Settings settings;
+        settings._checkCodingStyle = true;
+        CheckOther checkOther(&tokenizer, &settings, this);
+        checkOther.invalidPointer();
+    }
+
+    void invalidpointer()
+    {
+        // errors..
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
                             "{\n"
                             "    int *a = abc->a;\n"
+                            "    *a;\n"
                             "    if (!abc)\n"
                             "        ;\n"
                             "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (error) Possible invalid pointer dereference\n", errout.str());
-    }
 
-    void nullpointer4()
-    {
-        checkNullPointer("void foo(struct ABC *abc)\n"
-                         "{\n"
-                         "    int *a = abc->a;\n"
-                         "    if (abc)\n"
-                         "        ;\n"
-                         "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference\n", errout.str());
-    }
-
-    void nullpointer5()
-    {
         // ok dereferencing in a condition
-        checkNullPointer("void foo(struct ABC *abc)\n"
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
                             "{\n"
                             "    if (abc && abc->a);\n"
                             "    if (!abc)\n"
@@ -481,7 +488,7 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         // ok to use a linked list..
-        checkNullPointer("void foo(struct ABC *abc)\n"
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
                             "{\n"
                             "    abc = abc->next;\n"
                             "    if (!abc)\n"
@@ -490,14 +497,39 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         // reassign struct..
-        checkNullPointer("void foo(struct ABC *abc)\n"
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
                             "{\n"
                             "    a = abc->a;\n"
+                            "    *a;\n"
                             "    abc = abc->next;\n"
                             "    if (!abc)\n"
                             "        ;\n"
                             "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
+                            "{\n"
+                            "    a = abc->a;\n"
+                            "    *a;\n"
+                            "    f(&abc);\n"
+                            "    if (!abc)\n"
+                            "        ;\n"
+                            "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // goto..
+        checkInvalidPointer("void foo(struct ABC *abc)\n"
+                            "{\n"
+                            "    if (!abc)\n"
+                            "        goto out;"
+                            "    a = abc->a;\n"
+                            "    *a;\n"
+                            "    return;\n"
+                            "out:\n"
+                            "    if (!abc)\n"
+                            "        ;\n"
+                            "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void checkOldStylePointerCast(const char code[])