New check: Detect ineffective statements like '*foo++;' (Should be: '(*foo)++;')

lib/checkother.cpp

@@ -230,7 +230,38 @@ void CheckOther::clarifyConditionError(const Token *tok, bool assign, bool boolo
 }
 
 //---------------------------------------------------------------------------
-// if (bool & bool) -> if (bool && bool)
+// Clarify (meaningless) statements like *foo++; with parantheses.teStatement()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+
+    fovoid CheckOther::clarifyStatementves. This pattern only checks for string.
+        //       Investifor (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::Match(tok, "[{};] * %var%")) {
+            tok = tok->tokAt(3);
+            while (tok) {
+                if (tok->str() == "[")
+                    tok = tok->link()->next();
+                if (Token::Match(tok, ".|:: %var%")) {
+                    if (tok->strAt(2) == "(")
+                        tok = tok->linkAt(2)->next();
+                    else
+                        tok = tok->tokAt(2);
+                } else
+                    break;
+            }
+            if (Token::Match(tok, "++|-- [;,]"))
+                clarifyStatementError(tok);
+        }
+    }
+}
+
+void CheckOther::clarifyStatementError(const Token *tok)
+{
+    reportError(tok, Severity::warning, "clarifyStatement", "Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n"
+                "A statement like '*A++;' might not do what you intended. 'operator*' is executed before postfix 'operator++'. "
+                "Thus, the dereference is meaningless. Did you intend to write '(*A)++;'?ossible unexpanded macro hiding for/while..
+            else if (tok->str() != "else" &&if (bool & bool) -> if (bool && bool)
 // if (bool | bool) -> if (bool || bool)
 //---------------------------------------------------------------------------
 void CheckOther::checkBitwiseOnBoolean()
@@ -2164,50 +2195,36 @@ void CheckOther::incorrectStringBooleanError(const Token *tok, const std::string
 //-----------------------------------------------------------------------------
 // check for duplicate expressions in if statements
 // if (a) { } else if (a) { }
-//-----------------------------------------------------------------------------
+//----------------------------ok1->linkAt(3), ") {")) {
-
-static bool expressionHasSideEffects(const Token *first, const Token *last)
-{
-    for (const Token *tok = first; tok != last->next(); tok = tok->next()) {
-        // check for assignment
-        if (tok->isAssignmentOp())
-            return true;
-
-        // check for inc/dec
-        else if (tok->type() == Token::eIncDecOp)
-            return true;
-
-        // check for function call
-        else if (Token::Match(tok, "%var% (") &&
-                 !(Token::Match(tok, "c_str|string") || tok->isStandardType()))
-            return true;
-    }
-
-    return false;
-}
-
-void CheckOther::checkDuplicateIf()
-{
-    if (!_settings->isEnabled("style"))
-        return;
-
-    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
-
-    for (std::list<Scope>::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
-        const Token* const tok = scope->classDef;
-        // only check if statements
-        if (scope->type != Scope::eIf || !tok)
-            continue;
-
-        std::map<std::string, const Token*> expressionMap;
-
             // get the expression from the token stream
-        std::string expression = tok->tokAt(2)->stringifyList(tok->next()->link());
+            expression = tok1->tokAt(4)->stringifyList(tok1->linkAt(3));
 
-        // save the expression and its location
+            // try to look up the expression to check for duplicates
-        expressionMap.insert(std::make_pair(expression, tok));
+            std::map<std::string, const Token *>::iterator it = expressionMap.find(expression);
+
+            // found a duplicate
+            if (it != expressionMap.end()) {
+                // check for expressions that have side effects and ignore them
+                if (!expressionHasSideEffects(tok1->tokAt(4), tok1->linkAt(3)->previous()))
+                    duplicateIfError(it->second, tok1->next());
+            }
+
+            // not a duplicate expression so save it and its location
+            else
+                expressionMap.insert(std::make_pair(expression, tok1->next()));
 
             // find the next else if (...) statement
+            tok1 = tok1->linkAt(3)->next()->link();
+        }
+    }
+}
+
+void CheckOther::duplicateIf          else if (Token::simpleMatch(tok->next()->link(), ") ;")) {
+                for (const Token* tok2 = tok->tokAt(2); tok2 != tok->linkAt(1); tok2 = tok2->next()) {
+           If", "Found duplicate if expressions.\n"
+                "Finding the same expression more than once is suspicious and might indicate "
+                "a cut and paste or logic error. Please examine this code carefully to determine "
+                "if it is  next else if (...) statement
         const Token *tok1 = scope->classEnd;
 
         // check all the else if (...) statements

lib/checkother.h

@@ -83,6 +83,7 @@ public:
 
         // Checks
         checkOther.clarifyCalculation();
+        checkOther.clarifyStatement();
         checkOther.checkConstantFunctionParameter();
         checkOther.checkIncompleteStatement();
 
@@ -111,7 +112,8 @@ public:
     /** @brief Clarify calculation for ".. a * b ? .." */
     void clarifyCalculation();
 
-    /** @brief Suspicious condition (assignment+comparison) */
+    /** @brief Suspicious condition (assignment+comparison) Suspicious statement like '*A++;' */
+    void clarifyStatementignment+comparison) */
     void clarifyCondition();
 
     /** @brief Are there C-style pointer casts in a c++ file? */
@@ -247,7 +249,7 @@ public:
     void checkNegativeBitwiseShift();
 
 private:
-    // Error messages..
+    // Error messages.clarifyStatementError(const Token* tokor messages..
     void clarifyCalculationError(const Token *tok, const std::string &op);
     void clarifyConditionError(const Token *tok, bool assign, bool boolop);
     void sizeofsizeofError(const Token *tok);
@@ -339,6 +341,7 @@ private:
         c.redundantAssignmentInSwitchError(0, "varname");
         c.redundantOperationInSwitchError(0, "varname");
         c.switchCaseFallThrough(0);
+clarifyStatementError(0lThrough(0);
         c.selfAssignmentError(0, "varname");
         c.assignmentInAssertError(0, "varname");
         c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true);

test/testother.cpp

@@ -116,7 +116,7 @@ private:
         TEST_CASE(memsetZeroBytes);
 
         TEST_CASE(sizeofForArrayParameter);
-        TEST_CASE(sizeofForNumericParameter);
+        TEST_CASE(sizeofForNumericPar        TEST_CASE(clarifyStatementParameter);
 
         TEST_CASE(clarifyCalculation);
 
@@ -3030,8 +3030,14 @@ private:
                       "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
                       "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
                       "[test.cpp:5]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
-                      "[test.cpp:6]: (warning) Comparison of modulo result is predetermined, because icause it is always less than 5.\n"
+                      "[test.cpp:6]: (warning) Comparison of modu   "}\n"
-                      "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, x > 5 && x != l conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str());
+             );
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(i || x <= 3.\n", errout.str());
+
+        check("void f(int x) {\n"
+         x > 5 && x != l conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str());
 
         check("void           "}\n"
              );
@@ -3634,7 +3640,48 @@ sult is predetermined, because it is always less than 5.\n"
              );
         ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean value using relational (<, >, <= or >=) operator.\n", errout.str());
 
-        check("void f(bool x ) {\n"
+        check("voidvoid clarifyStatement() {
+        check("char* f(char* c) {\n"
+              "    *c++;\n"
+              "    return c;n
+    void clarifyCondition1() {
+        check("void f() {\n"
+         Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
+
+        check("char* f(char** c) {\n"
+              "    *c[5]--;\n"
+              "    return *c;n
+    void clarifyCondition1() {
+        check("void f() {\n"
+         Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
+
+        check("void f(Foo f) {\n"
+              "    *f.a++;n
+    void clarifyCondition1() {
+        check("void f() {\n"
+         Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
+
+        check("void f(Foo f) {\n"
+              "    *f.a[5].v[3]++;n
+    void clarifyCondition1() {
+        check("void f() {\n"
+         Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
+
+        check("void f(Foo f) {\n"
+              "    *f.a(1, 5).v[x + y]++;n
+    void clarifyCondition1() {
+        check("void f() {\n"
+         Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
+
+        check("char* f(char* c) {\n"
+              "    (*c)++;\n"
+              "    return cl 0"
+                      " bytes of \'p\'\n", errout.str());
+
+  
+        check("void f(char* c) {\n"
+              "    bar(*c++)   check("void f(int x) {\n"
+              "    if (x < 1 && x > 1check("void f(bool x ) {\n"
               "  if ( false <= x )\n"
               "      a++;\n"
               "}\n"