From 87fe5c060e2f080a8708182f2be4e4c775633ad8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sat, 16 Mar 2019 21:21:30 +0100
Subject: [PATCH] Refactoring of Null Pointer Checker

---
 lib/checknullpointer.cpp | 78 ++++++++++++++++++----------------------
 test/cfg/wxwidgets.cpp   |  4 +--
 2 files changed, 37 insertions(+), 45 deletions(-)

diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
index 8fe7d11c5..7dd38d0f5 100644
--- a/lib/checknullpointer.cpp
+++ b/lib/checknullpointer.cpp
@@ -61,17 +61,17 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
     if (Token::Match(&tok, "%name% ( )") || !tok.tokAt(2))
         return;
 
-    const Token* firstParam = tok.tokAt(2);
-    const Token* secondParam = firstParam->nextArgument();
+    const std::vector<const Token *> args = getArguments(&tok);
+    const Token* firstParam = args.size() > 0 ? args[0] : nullptr;
+    const Token* secondParam = args.size() > 1 ? args[1] : nullptr;
 
     // 1st parameter..
     if (Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && secondParam && secondParam->str() != "0") // Only if length (second parameter) is not zero
         var.push_back(firstParam);
 
     if (library || tok.function() != nullptr) {
-        const Token *param = firstParam;
-        int argnr = 1;
-        while (param) {
+        for (int argnr = 1; argnr <= args.size(); ++argnr) {
+            const Token *param = args[argnr - 1];
             if (library && library->isnullargbad(&tok, argnr) && checkNullpointerFunctionCallPlausibility(tok.function(), argnr))
                 var.push_back(param);
             else if (tok.function()) {
@@ -79,65 +79,58 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
                 if (argVar && argVar->isStlStringType() && !argVar->isArrayOrPointer())
                     var.push_back(param);
             }
-            param = param->nextArgument();
-            argnr++;
         }
     }
 
     if (Token::Match(&tok, "printf|sprintf|snprintf|fprintf|fnprintf|scanf|sscanf|fscanf|wprintf|swprintf|fwprintf|wscanf|swscanf|fwscanf")) {
-        const Token* argListTok = nullptr; // Points to first va_list argument
         std::string formatString;
+        int argnr = args.size();
         const bool scan = Token::Match(&tok, "scanf|sscanf|fscanf|wscanf|swscanf|fwscanf");
 
         if (Token::Match(&tok, "printf|scanf|wprintf|wscanf ( %str%")) {
             formatString = firstParam->strValue();
-            argListTok = secondParam;
+            argnr = 1;
         } else if (Token::Match(&tok, "sprintf|fprintf|sscanf|fscanf|fwprintf|fwscanf|swscanf")) {
             const Token* formatStringTok = secondParam; // Find second parameter (format string)
             if (formatStringTok && formatStringTok->tokType() == Token::eString) {
-                argListTok = formatStringTok->nextArgument(); // Find third parameter (first argument of va_args)
+                argnr = 2; // third parameter (first argument of va_args)
                 formatString = formatStringTok->strValue();
             }
         } else if (Token::Match(&tok, "snprintf|fnprintf|swprintf") && secondParam) {
-            const Token* formatStringTok = secondParam->nextArgument(); // Find third parameter (format string)
+            const Token* formatStringTok = args.size() > 2 ? args[2] : nullptr; // third parameter (format string)
             if (formatStringTok && formatStringTok->tokType() == Token::eString) {
-                argListTok = formatStringTok->nextArgument(); // Find fourth parameter (first argument of va_args)
+                argnr = 3; // fourth parameter (first argument of va_args)
                 formatString = formatStringTok->strValue();
             }
         }
 
-        if (argListTok) {
-            bool percent = false;
-            for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) {
-                if (*i == '%') {
-                    percent = !percent;
-                } else if (percent) {
-                    percent = false;
+        bool percent = false;
+        for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) {
+            if (*i == '%') {
+                percent = !percent;
+            } else if (percent) {
+                percent = false;
 
-                    bool _continue = false;
-                    while (!std::isalpha((unsigned char)*i)) {
-                        if (*i == '*') {
-                            if (scan)
-                                _continue = true;
-                            else
-                                argListTok = argListTok->nextArgument();
-                        }
-                        ++i;
-                        if (!argListTok || i == formatString.end())
-                            return;
+                bool _continue = false;
+                while (!std::isalpha((unsigned char)*i)) {
+                    if (*i == '*') {
+                        if (scan)
+                            _continue = true;
+                        else
+                            argnr++;
                     }
-                    if (_continue)
-                        continue;
-
-                    if ((*i == 'n' || *i == 's' || scan)) {
-                        var.push_back(argListTok);
-                    }
-
-                    if (*i != 'm') // %m is a non-standard glibc extension that requires no parameter
-                        argListTok = argListTok->nextArgument(); // Find next argument
-                    if (!argListTok)
-                        break;
+                    ++i;
+                    if (i == formatString.end())
+                        return;
                 }
+                if (_continue)
+                    continue;
+
+                if (argnr < args.size() && (*i == 'n' || *i == 's' || scan))
+                    var.push_back(args[argnr]);
+
+                if (*i != 'm') // %m is a non-standard glibc extension that requires no parameter
+                    argnr++;
             }
         }
     }
@@ -423,9 +416,8 @@ void CheckNullPointer::nullConstantDereference()
 
                     // is one of the var items a NULL pointer?
                     for (const Token *vartok : var) {
-                        if (Token::Match(vartok, "0|NULL|nullptr [,)]")) {
+                        if (vartok->hasKnownIntValue() && vartok->getKnownIntValue() == 0)
                             nullPointerError(vartok);
-                        }
                     }
                 }
             } else if (Token::Match(tok, "std :: string|wstring ( 0|NULL|nullptr )"))
diff --git a/test/cfg/wxwidgets.cpp b/test/cfg/wxwidgets.cpp
index 117d9507d..763cc5133 100644
--- a/test/cfg/wxwidgets.cpp
+++ b/test/cfg/wxwidgets.cpp
@@ -52,9 +52,9 @@ void validGuiCode()
 
 void nullPointer(const wxString &str)
 {
-    // TODO cppcheck-suppress nullPointer
+    // cppcheck-suppress nullPointer
     wxLogGeneric(wxLOG_Message, (char*)NULL);
-    // TODO cppcheck-suppress nullPointer
+    // cppcheck-suppress nullPointer
     wxLogMessage((char*)NULL);
 
     double *doublePtr = NULL;