Improved isVariableChangedByFunctionCall, better logic when parameter…

… might be passed by reference
danmar · Feb 28, 2019 · 4f5a426 · 4f5a426
1 parent 7ccf4b9
commit 4f5a426
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 4 deletions.
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -811,6 +811,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
     if (!tok)
         return false;
 
+    const Token * const tok1 = tok;
+
     // address of variable
     const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");
 
@@ -844,10 +846,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
             tok = tok->link();
         else if (Token::Match(tok->previous(), "%name% ("))
             break;
+        else if (Token::simpleMatch(tok->previous(), "> (") && tok->previous()->link())
+            break;
         tok = tok->previous();
     }
     if (!tok || tok->str() != "(")
         return false;
+    const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)]"));
     tok = tok->previous();
     if (tok && tok->link() && tok->str() == ">")
         tok = tok->link()->previous();
@@ -879,12 +884,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
         // => it is assumed that parameter is an in parameter (TODO: this is a bad heuristic)
         if (!addressOf && settings && settings->library.isnullargbad(tok, 1+argnr))
             return false;
-        // addressOf => inconclusive
-        if (!addressOf) {
+        // possible pass-by-reference => inconclusive
+        if (possiblyPassedByReference) {
             if (inconclusive != nullptr)
                 *inconclusive = true;
             return false;
         }
+        // Safe guess: Assume that parameter is changed by function call
         return true;
     }
 

diff --git a/test/testastutils.cpp b/test/testastutils.cpp
@@ -135,7 +135,7 @@ class TestAstUtils : public TestFixture {
                "}";
         inconclusive = false;
         ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "x ) ;", &inconclusive));
-        // FIXME : ASSERT_EQUALS(true, inconclusive);
+        ASSERT_EQUALS(true, inconclusive);
     }
 
     bool nextAfterAstRightmostLeaf(const char code[], const char parentPattern[], const char rightPattern[]) {

diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
@@ -2536,7 +2536,7 @@ class TestNullPointer : public TestFixture {
         ASSERT_EQUALS("", errout.str());
 
         check("void f(int *p = 0) {\n"
-              "    printf(\"%d\", p);\n"
+              "    printf(\"%p\", p);\n"
               "    *p = 0;\n"
               "}", true);
         ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());