Fixed #1564 (CppCheck does not recognise redundant IF)

lib/checkother.cpp

@@ -89,7 +89,6 @@ void CheckOther::warningRedundantCode()
         if (! Token::simpleMatch(tok, "if ("))
             continue;
 
-        std::string varname;
         const Token *tok2 = tok->tokAt(2);
 
         /*
@@ -100,18 +99,11 @@ void CheckOther::warningRedundantCode()
          *   if (Foo::var)
          *
          **/
-
+        std::string varname = concatNames(&tok2);
-        while (Token::Match(tok2, "%var% .|::"))
-        {
-            varname.append(tok2->str());
-            varname.append(tok2->next()->str());
-            tok2 = tok2->tokAt(2);
-        }
 
         if (!Token::Match(tok2, "%var% ) {"))
             continue;
 
-        varname.append(tok2->str());
         tok2 = tok2->tokAt(3);
 
         /*
@@ -147,15 +139,8 @@ void CheckOther::warningRedundantCode()
             }
         }
 
-        std::string varname2;
+        std::string varname2 = concatNames(&tok2);
-        while (Token::Match(tok2, "%var% ::|."))
-        {
-            varname2.append(tok2->str());
-            varname2.append(tok2->next()->str());
-            tok2 = tok2->tokAt(2);
-        }
 
-        varname2.append(tok2->str());
         if (Token::Match(tok2, "%var%") && varname == varname2)
             tok2 = tok2->next();
         else
@@ -173,12 +158,26 @@ void CheckOther::warningRedundantCode()
             }
         }
 
-        if (!Token::Match(tok2, "; } !!else"))
+        /*
+         * Possible constructions:
+         *
+         * - if (%var%) { delete %var%; }
+         * - if (%var%) { delete %var%; %var% = 0; }
+         *
+         **/
+        if (Token::Match(tok2, "; } !!else"))
         {
-            continue;
+            redundantIfDelete0Error(tok);
+        }
+        else if (Token::Match(tok2, "; %var%"))
+        {
+            tok2 = tok2->next();
+            std::string varname3 = concatNames(&tok2);
+            if (Token::Match(tok2, "%var% = 0 ; } !!else") && varname2 == varname3)
+            {
+                redundantIfDelete0Error(tok);
+            }
         }
-
-        redundantIfDelete0Error(tok);
     }
 
 

lib/checkother.h

@@ -287,6 +287,26 @@ private:
      * -# dereference pointer
      */
     void nullPointerConditionalAssignment();
+
+    /**
+     * @brief Used in warningRedundantCode()
+     * Iterates through the %var% tokens in a fully qualified name and concatenates them.
+     */
+    std::string concatNames(const Token **tok) const
+    {
+        std::string varname;
+        while (Token::Match(*tok, "%var% ::|."))
+        {
+            varname.append((*tok)->str());
+            varname.append((*tok)->next()->str());
+            *tok = (*tok)->tokAt(2);
+        }
+
+        if (Token::Match(*tok, "%var%"))
+            varname.append((*tok)->str());
+
+        return varname;
+    }
 };
 /// @}
 //---------------------------------------------------------------------------

test/testother.cpp

@@ -237,7 +237,7 @@ private:
               "    if (p)\n"
               "    {\n"
               "        delete p;\n"
-              "        p = 0;\n"
+              "        z = 0;\n"
               "    }\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
@@ -258,6 +258,16 @@ private:
               "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        check("void foo()\n"
+              "{\n"
+              "    if (0 != g->p)\n"
+              "    {\n"
+              "        delete g->p;\n"
+              "        g->z = 0;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
         check("void foo()\n"
               "{\n"
               "    if (0 != this->g->a)\n"
@@ -277,6 +287,16 @@ private:
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
 
+        check("void foo()\n"
+              "{\n"
+              "    if (p)\n"
+              "    {\n"
+              "        delete p;\n"
+              "        p = 0;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
+
         check("void foo()\n"
               "{\n"
               "    if (p)\n"
@@ -291,6 +311,16 @@ private:
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
 
+        check("void foo()\n"
+              "{\n"
+              "    if (p != NULL)\n"
+              "    {\n"
+              "        delete p;\n"
+              "        p = NULL;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
+
         check("void foo()\n"
               "{\n"
               "    if (p)\n"
@@ -319,6 +349,16 @@ private:
               "        delete Foo::instance;\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
+
+        check("void Foo::deleteInstance()\n"
+              "{\n"
+              "    if (Foo::instance != NULL)\n"
+              "    {\n"
+              "        delete Foo::instance;\n"
+              "        Foo::instance = NULL;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition. It is safe to deallocate a NULL pointer\n", errout.str());
     }
 
     void unreachable1()