Null pointer: improved inconclusive checking in CheckNullPointer::nullPointerAfterLoop

lib/checknullpointer.cpp

@@ -228,15 +228,38 @@ void CheckNullPointer::nullPointerAfterLoop()
         // Locate the end of the while loop body..
         const Token *tok2 = tok->next()->link()->next()->link();
 
+        // Is this checking inconclusive?
+        bool inconclusive = false;
+
         // Check if the variable is dereferenced after the while loop
         while (0 != (tok2 = tok2 ? tok2->next() : 0))
         {
-            // Don't check into inner scopes or outer scopes. Stop checking if "break" is found
+            // inner and outer scopes
-            if (tok2->str() == "{" || tok2->str() == "}" || tok2->str() == "break")
+            if (tok2->str() == "{" || tok2->str() == "}")
+            {
+                // Not inconclusive: bail out
+                if (!_settings->inconclusive)
+                    break;
+
+                inconclusive = true;
+
+                if (tok2->str() == "}")
+                {
+                    // "}" => leaving function? then break.
+                    const Token *tok3 = tok2->link()->previous();
+                    if (!tok3 || !Token::Match(tok3, "[);]"))
+                        break;
+                    if (tok3->str() == ")" && !Token::Match(tok3->link()->previous(), "if|for|while"))
+                        break;
+                }
+            }
+
+            // Stop checking if "break" is found
+            else if (tok2->str() == "break")
                 break;
 
             // loop variable is found..
-            if (tok2->varId() == varid)
+            else if (tok2->varId() == varid)
             {
                 // dummy variable.. is it unknown if pointer is dereferenced or not?
                 bool unknown = false;
@@ -244,7 +267,7 @@ void CheckNullPointer::nullPointerAfterLoop()
                 // Is the loop variable dereferenced?
                 if (CheckNullPointer::isPointerDeRef(tok2, unknown))
                 {
-                    nullPointerError(tok2, varname, tok->linenr());
+                    nullPointerError(tok2, varname, tok->linenr(), inconclusive);
                 }
                 break;
             }
@@ -1178,8 +1201,12 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
     reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname);
 }
 
-void CheckNullPointer::nullPointerError(const Token *tok, const std::string &varname, const unsigned int line)
+void CheckNullPointer::nullPointerError(const Token *tok, const std::string &varname, const unsigned int line, bool inconclusive)
 {
-    reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString<unsigned int>(line));
+    const std::string errmsg("Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString<unsigned int>(line));
+    if (inconclusive)
+        reportInconclusiveError(tok, Severity::error, "nullPointer", errmsg);
+    else
+        reportError(tok, Severity::error, "nullPointer", errmsg);
 }
 

lib/checknullpointer.h

@@ -102,7 +102,7 @@ public:
 
     void nullPointerError(const Token *tok);  // variable name unknown / doesn't exist
     void nullPointerError(const Token *tok, const std::string &varname);
-    void nullPointerError(const Token *tok, const std::string &varname, const unsigned int line);
+    void nullPointerError(const Token *tok, const std::string &varname, const unsigned int line, bool inconclusive = false);
 
     /** Get error messages. Used by --errorlist */
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)

test/testnullpointer.cpp

@@ -34,6 +34,7 @@ private:
 
     void run()
     {
+        TEST_CASE(nullpointerAfterLoop);
         TEST_CASE(nullpointer1);
         TEST_CASE(nullpointer2);
         TEST_CASE(structDerefAndCheck);    // dereferencing struct and then checking if it's null
@@ -53,13 +54,14 @@ private:
         TEST_CASE(snprintf_with_non_zero_size);
     }
 
-    void check(const char code[])
+    void check(const char code[], bool inconclusive = false)
     {
         // Clear the error buffer..
         errout.str("");
 
         Settings settings;
         settings.addEnabled("style");
+        settings.inconclusive = inconclusive;
 
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -75,7 +77,7 @@ private:
     }
 
 
-    void nullpointer1()
+    void nullpointerAfterLoop()
     {
         check("int foo(const Token *tok)\n"
               "{\n"
@@ -151,6 +153,25 @@ private:
               "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        // dereference in outer scope..
+        {
+            const char code[] = "void foo(int x, const Token *tok) {\n"
+                                "    if (x == 123) {\n"
+                                "        while (tok) tok = tok->next();\n"
+                                "    }\n"
+                                "    tok->str();\n"
+                                "}\n";
+
+            check(code, false);
+            ASSERT_EQUALS("", errout.str());
+
+            check(code, true);
+            ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: tok - otherwise it is redundant to check if tok is null at line 3\n", errout.str());
+        }
+    }
+
+    void nullpointer1()
+    {
         // ticket #1923 - no false positive when using else if
         check("void f(A *a)\n"
               "{\n"