- Improved nullpointer check: Fixed #1171

- Improved accuracy of function analysis in symboldatabase
- Code cleanups

lib/checknullpointer.cpp

@@ -115,28 +115,31 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
         functionNames2_nullptr.insert("modf");
     }
 
+    if (Token::Match(&tok, "%var% ( )") || !tok.tokAt(2))
+        return;
+
+    const Token* firstParam = tok.tokAt(2);
+    const Token* secondParam = firstParam->nextArgument();
+
     // 1st parameter..
-    if ((Token::Match(&tok, "%var% ( %var% ,|)") && tok.tokAt(2)->varId() > 0) ||
+    if ((Token::Match(firstParam, "%var% ,|)") && firstParam->varId() > 0) ||
-        (value == 0 && Token::Match(&tok, "%var% ( 0 ,|)"))) {
+        (value == 0 && Token::Match(firstParam, "0 ,|)"))) {
         if (functionNames1_all.find(tok.str()) != functionNames1_all.end())
-            var.push_back(tok.tokAt(2));
+            var.push_back(firstParam);
         else if (value == 0 && functionNames1_nullptr.find(tok.str()) != functionNames1_nullptr.end())
-            var.push_back(tok.tokAt(2));
+            var.push_back(firstParam);
-        else if (value != 0 && Token::simpleMatch(&tok, "fflush"))
+        else if (value != 0 && tok.str() == "fflush")
-            var.push_back(tok.tokAt(2));
+            var.push_back(firstParam);
-        else if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && tok.strAt(4) != "0") // Only if length is not zero
+        else if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && secondParam && secondParam->str() != "0") // Only if length (second parameter) is not zero
-            var.push_back(tok.tokAt(2));
+            var.push_back(firstParam);
     }
 
     // 2nd parameter..
-    if (Token::Match(&tok, "%var% ( !!)")) {
+    if (secondParam && ((value == 0 && secondParam->str() == "0") || (Token::Match(secondParam, "%var%") && secondParam->varId() > 0))) {
-        const Token* secondParameter = tok.tokAt(2)->nextArgument();
+        if (functionNames2_all.find(tok.str()) != functionNames2_all.end())
-        if (secondParameter && ((value == 0 && secondParameter->str() == "0") || (Token::Match(secondParameter, "%var%") && secondParameter->varId() > 0))) {
+            var.push_back(secondParam);
-            if (functionNames2_all.find(tok.str()) != functionNames2_all.end())
+        else if (value == 0 && functionNames2_nullptr.find(tok.str()) != functionNames2_nullptr.end())
-                var.push_back(secondParameter);
+            var.push_back(secondParam);
-            else if (value == 0 && functionNames2_nullptr.find(tok.str()) != functionNames2_nullptr.end())
-                var.push_back(secondParameter);
-        }
     }
 
     if (Token::Match(&tok, "printf|sprintf|snprintf|fprintf|fnprintf|scanf|sscanf|fscanf")) {
@@ -145,25 +148,22 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
         bool scan = Token::Match(&tok, "scanf|sscanf|fscanf");
 
         if (Token::Match(&tok, "printf|scanf ( %str%")) {
-            formatString = tok.strAt(2);
+            formatString = firstParam->strValue();
-            if (tok.strAt(3) == ",")
+            argListTok = secondParam;
-                argListTok = tok.tokAt(4);
+        } else if (Token::Match(&tok, "sprintf|fprintf|sscanf|fscanf")) {
-            else
+            const Token* formatStringTok = secondParam; // Find second parameter (format string)
-                argListTok = 0;
+            if (formatStringTok && formatStringTok->str()[0] == '"') {
-        } else if (Token::Match(&tok, "sprintf|fprintf|sscanf|fscanf ( %any%")) {
-            const Token* formatStringTok = tok.tokAt(2)->nextArgument(); // Find second parameter (format string)
-            if (formatStringTok && Token::Match(formatStringTok, "%str%")) {
                 argListTok = formatStringTok->nextArgument(); // Find third parameter (first argument of va_args)
-                formatString = formatStringTok->str();
+                formatString = formatStringTok->strValue();
             }
-        } else if (Token::Match(&tok, "snprintf|fnprintf ( %any%")) {
+        } else if (Token::Match(&tok, "snprintf|fnprintf")) {
-            const Token* formatStringTok = tok.tokAt(2);
+            const Token* formatStringTok = secondParam;
-            for (int i = 0; i < 2 && formatStringTok; i++) {
+            for (int i = 0; i < 1 && formatStringTok; i++) {
                 formatStringTok = formatStringTok->nextArgument(); // Find third parameter (format string)
             }
-            if (formatStringTok && Token::Match(formatStringTok, "%str%")) {
+            if (formatStringTok && formatStringTok->str()[0] == '"') {
                 argListTok = formatStringTok->nextArgument(); // Find fourth parameter (first argument of va_args)
-                formatString = formatStringTok->str();
+                formatString = formatStringTok->strValue();
             }
         }
 
@@ -191,7 +191,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
                         continue;
 
                     if ((*i == 'n' || *i == 's' || scan) && (!scan || value == 0)) {
-                        if ((value == 0 && argListTok->str() == "0") || (Token::Match(argListTok, "%var%") && argListTok->varId() > 0)) {
+                        if ((value == 0 && argListTok->str() == "0") || (argListTok->varId() > 0)) {
                             var.push_back(argListTok);
                         }
                     }
@@ -1026,7 +1026,13 @@ void CheckNullPointer::nullConstantDereference()
             else if (Token::Match(tok, "0 [") && (tok->previous()->str() != "&" || !Token::Match(tok->next()->link()->next(), "[.(]")))
                 nullPointerError(tok);
 
-            else if (Token::Match(tok->previous(), "[={};] %var% (")) {
+            else if (Token::Match(tok->previous(), "!!. %var% (") && (tok->previous()->str() != "::" || tok->strAt(-2) == "std")) {
+                if (Token::Match(tok->tokAt(2), "0 )")) {
+                    const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId());
+                    if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::"))
+                        nullPointerError(tok);
+                }
+
                 std::list<const Token *> var;
                 parseFunctionCall(*tok, var, 0);
 
@@ -1038,11 +1044,6 @@ void CheckNullPointer::nullConstantDereference()
                 }
             } else if (Token::simpleMatch(tok, "std :: string ( 0 )"))
                 nullPointerError(tok);
-            else if (Token::Match(tok, "%var% ( 0 )")) {
-                const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId());
-                if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::"))
-                    nullPointerError(tok);
-            }
 
             unsigned int ovarid = 0;
             if (Token::Match(tok, "0 ==|!= %var%"))

lib/symboldatabase.cpp

@@ -199,14 +199,14 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                         scope->access = Private;
                     else if (tok->str() == "protected")
                         scope->access = Protected;
-                    else if (tok->str() == "public")
+                    else
                         scope->access = Public;
 
                     tok = tok->tokAt(2);
                 }
 
                 // class function?
-                else if (tok->previous()->str() != "::" && isFunction(tok, &funcStart, &argStart)) {
+                else if (tok->previous()->str() != "::" && isFunction(tok, scope, &funcStart, &argStart)) {
                     Function function;
 
                     // save the function definition argument start '('
@@ -347,12 +347,12 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                 }
 
                 // nested class or friend function?
-                else if (tok->previous()->str() == "::" && isFunction(tok, &funcStart, &argStart)) {
+                else if (tok->previous()->str() == "::" && isFunction(tok, scope, &funcStart, &argStart)) {
                     /** @todo check entire qualification for match */
                     Scope * nested = scope->findInNestedListRecursive(tok->strAt(-2));
 
                     if (nested)
-                        addFunction(&scope, &tok, argStart);
+                        addClassFunction(&scope, &tok, argStart);
                     else {
                         /** @todo handle friend functions */
                     }
@@ -373,54 +373,23 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                 const Token *argStart = 0;
 
                 // function?
-                if (isFunction(tok, &funcStart, &argStart)) {
+                if (isFunction(tok, scope, &funcStart, &argStart)) {
                     // has body?
                     if (Token::Match(argStart->link(), ") const| {|:")) {
                         Scope *old_scope = scope;
 
                         // class function
                         if (tok->previous() && tok->previous()->str() == "::")
-                            addFunction(&scope, &tok, argStart);
+                            addClassFunction(&scope, &tok, argStart);
 
                         // class destructor
                         else if (tok->previous() && tok->previous()->str() == "~" &&
                                  tok->tokAt(-2) && tok->strAt(-2) == "::")
-                            addFunction(&scope, &tok, argStart);
+                            addClassFunction(&scope, &tok, argStart);
 
                         // regular function
-                        else {
+                        else
-                            Function function;
+                            addGlobalFunction(scope, tok, argStart, funcStart);
-
-                            // save the function definition argument start '('
-                            function.argDef = argStart;
-
-                            // save the access type
-                            function.access = Public;
-
-                            // save the function name location
-                            function.tokenDef = funcStart;
-                            function.token = funcStart;
-
-                            function.isInline = false;
-                            function.hasBody = true;
-                            function.arg = function.argDef;
-                            function.type = Function::eFunction;
-
-                            // find start of function '{'
-                            const Token *start = tok;
-                            while (start && start->str() != "{")
-                                start = start->next();
-
-                            // save start of function
-                            function.start = start;
-
-                            addNewFunction(&scope, &tok);
-
-                            if (scope) {
-                                old_scope->functionList.push_back(function);
-                                scope->function = &old_scope->functionList.back();
-                            }
-                        }
 
                         // syntax error
                         if (!scope) {
@@ -432,47 +401,17 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                     // function returning function pointer with body
                     else if (Token::simpleMatch(argStart->link(), ") ) (") &&
                              Token::Match(argStart->link()->linkAt(2), ") const| {")) {
-                        const Token *tok1 = funcStart;
+                        tok = funcStart;
                         Scope *old_scope = scope;
 
                         // class function
-                        if (tok1->previous()->str() == "::")
+                        if (tok->previous()->str() == "::")
-                            addFunction(&scope, &tok1, argStart);
+                            addClassFunction(&scope, &tok, argStart);
 
                         // regular function
                         else {
-                            Function function;
+                            Function* function = addGlobalFunction(scope, tok, argStart, funcStart);
-
+                            function->retFuncPtr = true;
-                            // save the function definition argument start '('
-                            function.argDef = argStart;
-
-                            // save the access type
-                            function.access = Public;
-
-                            // save the function name location
-                            function.tokenDef = funcStart;
-                            function.token = funcStart;
-
-                            function.isInline = false;
-                            function.hasBody = true;
-                            function.arg = function.argDef;
-                            function.type = Function::eFunction;
-                            function.retFuncPtr = true;
-
-                            // find start of function '{'
-                            const Token *start = tok;
-                            while (start && start->str() != "{")
-                                start = start->next();
-
-                            // save start of function
-                            function.start = start;
-
-                            addNewFunction(&scope, &tok1);
-
-                            if (scope) {
-                                old_scope->functionList.push_back(function);
-                                scope->function = &old_scope->functionList.back();
-                            }
                         }
 
                         // syntax error?
@@ -480,8 +419,6 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
                             scope = old_scope;
                             break;
                         }
-
-                        tok = tok1;
                     }
 
                     // function prototype
@@ -810,7 +747,7 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
     }
 }
 
-bool SymbolDatabase::isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const
+bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart) const
 {
     // function returning function pointer? '... ( ... %var% ( ... ))( ... ) {'
     if (tok->str() == "(" &&
@@ -825,7 +762,10 @@ bool SymbolDatabase::isFunction(const Token *tok, const Token **funcStart, const
     }
 
     // regular function?
-    else if (Token::Match(tok, "%var% (") &&
+    else if (Token::Match(tok, "%var% (") && tok->previous() &&
+             (tok->previous()->isName() || tok->previous()->str() == "&" || tok->previous()->str() == "*" || // Either a return type in front of tok
+              tok->previous()->str() == "::" || tok->previous()->str() == "~" || // or a scope qualifier in front of tok
+              outerScope->isClassOrStruct()) && // or a ctor/dtor
              (Token::Match(tok->next()->link(), ") const| ;|{|=") ||
               Token::Match(tok->next()->link(), ") : %var% (|::"))) {
         *funcStart = tok;
@@ -848,10 +788,18 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok
 
         // skip default value assignment
         else if (first->next()->str() == "=") {
-            first = first->tokAt(2);
+            first = first->nextArgument();
 
-            if (second->next()->str() == "=")
+            if (second->next()->str() == "=") {
-                second = second->tokAt(2);
+                second = second->nextArgument();
+                if (!first || !second) { // End of argument list (first or second)
+                    match = !first && !second;
+                    break;
+                }
+            } else if (!first) { // End of argument list (first)
+                match = second->next() && second->next()->str() == ")";
+                break;
+            }
         }
 
         // definition missing variable name
@@ -877,10 +825,6 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok
             // skip variable names
             first = first->next();
             second = second->next();
-
-            // skip default value assignment
-            if (first->next()->str() == "=")
-                first = first->tokAt(2);
         }
 
         // variable with class path
@@ -920,7 +864,45 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok
     return match;
 }
 
-void SymbolDatabase::addFunction(Scope **scope, const Token **tok, const Token *argStart)
+Function* SymbolDatabase::addGlobalFunction(Scope*& scope, const Token*& tok, const Token *argStart, const Token* funcStart)
+{
+    Function function;
+
+    // save the function definition argument start '('
+    function.argDef = argStart;
+
+    // save the access type
+    function.access = Public;
+
+    // save the function name location
+    function.tokenDef = funcStart;
+    function.token = funcStart;
+
+    function.isInline = false;
+    function.hasBody = true;
+    function.arg = function.argDef;
+    function.type = Function::eFunction;
+
+    // find start of function '{'
+    const Token *start = tok;
+    while (start && start->str() != "{")
+        start = start->next();
+
+    // save start of function
+    function.start = start;
+
+    Scope* old_scope = scope;
+    addNewFunction(&scope, &tok);
+
+    if (scope) {
+        old_scope->functionList.push_back(function);
+        scope->function = &old_scope->functionList.back();
+        return scope->function;
+    }
+    return 0;
+}
+
+void SymbolDatabase::addClassFunction(Scope **scope, const Token **tok, const Token *argStart)
 {
     int count = 0;
     bool added = false;
@@ -1880,29 +1862,25 @@ const Variable *Scope::getVariable(const std::string &varname) const
     return NULL;
 }
 
-inline const Token* skipScopeIdentifiers(const Token* tok)
+static const Token* skipScopeIdentifiers(const Token* tok)
 {
-    const Token* ret = tok;
+    if (Token::simpleMatch(tok, "::")) {
-
+        tok = tok->next();
-    if (Token::simpleMatch(ret, "::")) {
-        ret = ret->next();
     }
-    while (Token::Match(ret, "%type% ::")) {
+    while (Token::Match(tok, "%type% ::")) {
-        ret = ret->tokAt(2);
+        tok = tok->tokAt(2);
     }
 
-    return ret;
+    return tok;
 }
 
-inline const Token* skipPointers(const Token* tok)
+static const Token* skipPointers(const Token* tok)
 {
-    const Token* ret = tok;
+    while (Token::Match(tok, "*|&")) {
-
+        tok = tok->next();
-    while (Token::Match(ret, "*|&")) {
-        ret = ret->next();
     }
 
-    return ret;
+    return tok;
 }
 
 bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok, bool &isArray, bool &isPointer, bool &isReference) const
@@ -1928,11 +1906,11 @@ bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const
         localVarTok = skipPointers(localTypeTok->next());
     }
 
-    if (isSimpleVariable(localVarTok)) {
+    if (Token::Match(localVarTok, "%var% ;|=")) {
         vartok = localVarTok;
         typetok = localTypeTok;
         isArray = false;
-    } else if (isArrayVariable(localVarTok)) {
+    } else if (Token::Match(localVarTok, "%var% [") && localVarTok->str() != "operator") {
         vartok = localVarTok;
         typetok = localTypeTok;
         isArray = true;
@@ -1956,16 +1934,6 @@ bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const
     return NULL != vartok;
 }
 
-bool Scope::isSimpleVariable(const Token* tok) const
-{
-    return Token::Match(tok, "%var% ;|=");
-}
-
-bool Scope::isArrayVariable(const Token* tok) const
-{
-    return Token::Match(tok, "%var% [") && tok->next()->str() != "operator";
-}
-
 bool Scope::findClosingBracket(const Token* tok, const Token*& close) const
 {
     bool found = false;

lib/symboldatabase.h

@@ -530,8 +530,6 @@ private:
      * @return true if tok points to a variable declaration, false otherwise
      */
     bool isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok, bool &isArray, bool &isPointer, bool &isReference) const;
-    bool isSimpleVariable(const Token* tok) const;
-    bool isArrayVariable(const Token* tok) const;
     bool findClosingBracket(const Token* tok, const Token*& close) const;
 };
 
@@ -589,10 +587,11 @@ private:
     // Needed by Borland C++:
     friend class Scope;
 
-    void addFunction(Scope **info, const Token **tok, const Token *argStart);
+    void addClassFunction(Scope **info, const Token **tok, const Token *argStart);
+    Function* addGlobalFunction(Scope*& scope, const Token*& tok, const Token *argStart, const Token* funcStart);
     void addNewFunction(Scope **info, const Token **tok);
     const Token *initBaseInfo(Scope *info, const Token *tok);
-    bool isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const;
+    bool isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart) const;
 
     /** class/struct types */
     std::set<std::string> classAndStructTypes;

test/testnullpointer.cpp

@@ -1521,6 +1521,13 @@ private:
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (error) Null pointer dereference\n", errout.str());
 
+        // Ticket #1171
+        check("void foo(void* bar) {\n"
+              "  if(strcmp(0, bar) == 0)\n"
+              "    func();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
         // Ticket #2413 - it's ok to pass NULL to fflush
         check("void foo() {\n"
               "  fflush(NULL);\n"

test/testobsoletefunctions.cpp

@@ -226,7 +226,7 @@ private:
 
     // ticket #3121
     void test_declared_function() {
-        check("ftime ( int a )\n"
+        check("int ftime ( int a )\n"
               "{\n"
               "    return a;\n"
               "}\n"