Fixed "Improved handling of 0 initializations (#4604)"

This fixes commit 1201e417ec.

lib/checkother.cpp

@@ -612,7 +612,6 @@ void CheckOther::checkRedundantAssignment()
 
         std::map<unsigned int, const Token*> varAssignments;
         std::map<unsigned int, const Token*> memAssignments;
-        std::set<unsigned int> initialized;
         const Token* writtenArgumentsEnd = 0;
 
         for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
@@ -629,17 +628,6 @@ void CheckOther::checkRedundantAssignment()
                 varAssignments.clear();
                 memAssignments.clear();
             } else if (tok->type() == Token::eVariable) {
-                // Set initialization flag
-                if (!Token::Match(tok, "%var% ["))
-                    initialized.insert(tok->varId());
-                else {
-                    const Token *tok2 = tok->next();
-                    while (tok2 && tok2->str() == "[")
-                        tok2 = tok2->link()->next();
-                    if (tok2 && tok2->str() != ";")
-                        initialized.insert(tok->varId());
-                }
-
                 std::map<unsigned int, const Token*>::iterator it = varAssignments.find(tok->varId());
                 if (tok->next()->isAssignmentOp() && Token::Match(tok->previous(), "[;{}]")) { // Assignment
                     if (it != varAssignments.end()) {
@@ -657,15 +645,14 @@ void CheckOther::checkRedundantAssignment()
                             }
                         }
                         if (error) {
-                            if (scope->typ!Token::simpleMatch(tok->tokAt(2), "0 ;") || (tok->variable() && tok->variable()->nameToken() != tok->tokAt(-2)))AssignmentInSwitchError(it->second, tok, tok->str());
+                            if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok) && warning)
+                                redundantAssignmentInSwitchError(it->second, tok, tok->str());
                             else if (performance)
                                 redundantAssignmentError(it->second, tok, tok->str(), nonLocal(it->second->variable())); // Inconclusive for non-local variables
                         }
                         it->second = tok;
                     }
-                    if (Token::simpleMatch(tok->tokAt(2), "0 ;"))
+                    if (!Token::simpleMatch(tok->tokAt(2), "0 ;") || (tok->variable() && tok->variable()->nameToken() != tok->tokAt(-2)))
-                        varAssignments.erase(tok->varId());
-                    else
                         varAssignments[tok->varId()] = tok;
                     memAssignments.erase(tok->varId());
                 } else if (tok->next()->type() == Token::eIncDecOp || (tok->previous()->type() == Token::eIncDecOp && !Token::Match(tok->next(), ".|[|("))) { // Variable incremented/decremented
@@ -683,12 +670,10 @@ void CheckOther::checkRedundantAssignment()
                     const Token* param1 = tok->tokAt(2);
                     writtenArgumentsEnd = param1->next();
                     if (param1->varId() && param1->strAt(1) == "," && !Token::Match(tok, "strcat|strncat|wcscat|wcsncat")) {
-                        if (tok->str() == "memset" && initialized.find(param1->varId()) == initialized.end() && param1->variable() && param1->variable()->isLocal() && param1->variable()->isArray())
+                        std::map<unsigned int, const Token*>::iterator it = memAssignments.find(param1->varId());
-                            initialized.insert(param1->varId());
+                        if (it == memAssignments.end())
-                        else if (memAssignments.find(param1->varId()) == memAssignments.end())
                             memAssignments[param1->varId()] = tok;
                         else {
-                            const std::map<unsigned int, const Token*>::iterator it = memAssignments.find(param1->varId());
                             if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok) && warning)
                                 redundantCopyInSwitchError(it->second, tok, param1->str());
                             else if (performance)
@@ -1169,6 +1154,46 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
                 "selfAssignment", "Redundant assignment of '" + varname + "' to itself.");
 }
 
+//---------------------------------------------------------------------------
+//    int a = 1;
+//    assert(a = 2);            // <- assert should not have a side-effect
+//---------------------------------------------------------------------------
+static inline const Token *findAssertPattern(const Token *start)
+{
+    return Token::findmatch(start, "assert ( %any%");
+}
+
+void CheckOther::checkAssignmentInAssert()
+{
+    if (!_settings->isEnabled("warning"))
+        return;
+
+    const Token *tok = findAssertPattern(_tokenizer->tokens());
+    const Token *endTok = tok ? tok->next()->link() : NULL;
+
+    while (tok && endTok) {
+        for (tok = tok->tokAt(2); tok != endTok; tok = tok->next()) {
+            if (tok->isName() && (tok->next()->isAssignmentOp() || tok->next()->type() == Token::eIncDecOp))
+                assignmentInAssertError(tok, tok->str());
+            else if (Token::Match(tok, "--|++ %var%"))
+                assignmentInAssertError(tok, tok->strAt(1));
+        }
+
+        tok = findAssertPattern(endTok->next());
+        endTok = tok ? tok->next()->link() : NULL;
+    }
+}
+
+void CheckOther::assignmentInAssertError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::warning,
+                "assignmentInAssert", "Assert statement modifies '" + varname + "'.\n"
+                "Variable '" + varname + "' is modified insert assert statement. "
+                "Assert statements are removed from release builds so the code inside "
+                "assert statement is not executed. If the code is needed also in release "
+                "builds, this is a bug.");
+}
+
 //---------------------------------------------------------------------------
 //    if ((x != 1) || (x != 3))            // expression always true
 //    if ((x == 1) && (x == 3))            // expression always false
@@ -2118,7 +2143,7 @@ void CheckOther::checkMathFunctions()
     const std::size_t functions = symbolDatabase->functionScopes.size();
     for (std::size_t i = 0; i < functions; ++i) {
         const Scope * scope = symbolDatabase->functionScopes[i];
-   ch(tok, "div|st Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+        for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
             if (tok->varId())
                 continue;
             if (Token::Match(tok, "log|log10 ( %num% )")) {

test/testother.cpp

@@ -106,6 +106,9 @@ private:
         TEST_CASE(trac2071);
         TEST_CASE(trac2084);
         TEST_CASE(trac3693);
+
+        TEST_CASE(assignmentInAssert);
+
         TEST_CASE(modulo);
 
         TEST_CASE(incorrectLogicOperator1);
@@ -1756,17 +1759,18 @@ private:
               "    }\n"
               "}", 0, false, false, false, false);
         ASSERT_EQUALS("", errout.str());
+    }
 
-        check("void f() {\n"  // Ticket     }
+    void switchRedundantOperationTest() {
-
+        check("void foo()\n"
-    void switchRedundantOperationTest() {   "{\n"
+              "{\n"
-              "    std::cout <<  log(-0.1) << std::endl;\n"
+              "    int y = 1;\n"
-a)\n"
+              "    switch (a)\n"
               "    {\n"
               "    case 2:\n"
-              "      {\n"
               "        ++y;\n"
-                  "        y = 3;\n"
+              "    case 3:\n"
+              "        y = 3;\n"
               "    }\n"
               "    bar(y);\n"
               "}");
@@ -2932,7 +2936,7 @@ a)\n"
               "{\n"
               "        std::string var = var = \"test\";\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3]: (warning) Redundant assignment of 'var' to itself.\n", errout.ssignment of 'var' to itself.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Redundant assignment of 'var' to itself.\n", errout.str());
 
         // #4073 (segmentation fault)
         check("void Foo::myFunc( int a )\n"
@@ -2940,7 +2944,11 @@ a)\n"
               "    if (a == 42)\n"
               "    a = a;\n"
               "}");
-ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\n"
+
+        check("void foo()\n"
+              "{\n"
+              "        int x = 1;\n"
+              "        x = x + 1;\n"
               "        return 0;\n"
               "}");
         ASSERT_EQUALS("", errout.str());
@@ -3221,6 +3229,62 @@ ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\
         ASSERT_EQUALS("", errout.str());
     }
 
+    void assignmentInAssert() {
+        check("void f() {\n"
+              "    int a; a = 0;\n"
+              "    assert(a = 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+
+        check("void f(int a) {\n"
+              "    assert(a == 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int a, int b) {\n"
+              "    assert(a == 2 && b = 1);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Assert statement modifies 'b'.\n", errout.str());
+
+        check("void f() {\n"
+              "    int a; a = 0;\n"
+              "    assert(a += 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+
+        check("void f() {\n"
+              "    int a; a = 0;\n"
+              "    assert(a *= 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+
+        check("void f() {\n"
+              "    int a; a = 0;\n"
+              "    assert(a -= 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a--);\n" // don't simplify and verify
+              "    return a;\n"
+              "}\n", 0, false, false, false, false
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+    }
+
     void modulo() {
         check("bool f(bool& b1, bool& b2, bool& b3) {\n"
               "    b1 = a % 5 == 4;\n"
@@ -5573,7 +5637,8 @@ ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\
               "    memcpy(a, b, 5);\n"
               "    memmove(a, b, 5);\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (performance) Buffer 'a' is being written before its old content has been used.\n"
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n"
+                      "[test.cpp:3] -> [test.cpp:5]: (performance) Buffer 'a' is being written before its old content has been used.\n"
                       "[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n"
                       "[test.cpp:4]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memcpy()' with 'sizeof(*a)'?\n"
                       "[test.cpp:5]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memmove()' with 'sizeof(*a)'?\n", errout.str());
@@ -5781,21 +5846,35 @@ ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\
               "    x = 1;\n"
               "    return x + 1;\n"
               "}", NULL, false, false, false, false);
-        ASSERT_EQUALS("[test.cpp:3] -> [test. "    i = 1;\n"
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str());
-              "    bar();\n"
+
-              "    i = 1;\n"
+        // from #3103 (avoid a false positive)
-              "}");
+        check("int foo(){\n"
-        ASS// #4513
+              "    int x;\n"
-        check("int x;\n"
+              "    x = 1;\n"
-              "int g() {\n"
-              "    return x*x;\n"
-              "}\n"
-              x = 1;\n"
               "    if (y)\n" // <-- cppcheck does not know anything about 'y'
               "        x = 2;\n"
               "    return x + 1;\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"  // Ticket #4356
+              "    int x = 0;\n"  // <- ignore assignment with 0
+              "    x = 3;\n"
+              "}", 0, false, false, false, false);
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    int i = 54;\n"
+              "    i = 0;\n"
+              "}", 0, false, false, false, false);
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
+
+        check("void f() {\n"
+              "    int i = 54;\n"
+              "    i = 1;\n"
+              "}", 0, false, false, false, false);
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
     }
 
     void redundantMemWrite() {
@@ -5847,23 +5926,7 @@ ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\
         check("void f(void* a) {\n"
               "    snprintf(a, foo, bar);\n"
               "    bar();\n"
-              "    mems
+              "    memset(a, 0, size);\n"
-        check("void f() {\n"  // Ticket #4356
-              "    int x = 0;\n"  // <- ignore assignment with 0
-              "    x = 3;\n"
-              "}", 0, false, false, false, falseerrout.str());
-
-
-        // check fgetc
-        check("voif f (FILE * pFile){\n"
-            int i = 54;\n"
-              "    i = 0;\n"
-              "}", 0, false, false, false, false);
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (perfocpp:4]: (performance) Variable 'x' is reassigned a value before the old one has been used.\n", errout.strcheck("void f() {\n"
-              "    int i = 54;\n"
-              "    i = 1;\n"
-              "}", 0, false, false, false, false);
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (perfocpp:4]: (performance) Variable 'x' is reassigned a value before the old one has been used.\n", eet(a, 0, size);\n"
               "}");
         ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (performance) Buffer 'a' is being written before its old content has been used.\n", errout.str());
 
@@ -5898,21 +5961,6 @@ ning, inconclusive) Array 'a' is filled incompletely. Did you forget x = x + 1;\
               "        memset(a, 0, size);\n"
               "}");
         ASSERT_EQUALS("", errout.str());
-
-        // #4455 - initialization of local buffer
-        check("void f(void) {"
-              "    char buf[10];\n"
-              "    memset(buf, 0, 10);\n"
-              "    strcpy(buf, string);\n"
-              "}");
-        ASSERT_EQUALS("", errout.str());
-
-        check("void f(void) {\n"
-              "    char buf[10] = {0};\n"
-              "    memset(buf, 0, 10);\n"
-              "    strcpy(buf, string);\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Buffer 'buf' is being written before its old content has been used.\n", errout.str());
     }
 
     void varFuncNullUB() { // #4482