fix #5591

Author: clock999

lib/checkother.cpp

@@ -4387,6 +4387,43 @@ void CheckOther::overlappingWriteFunction(const Token *tok, const std::string& f
     reportError(tok, Severity::error, "overlappingWriteFunction", "Overlapping read/write in " + funcname + "() is undefined behavior");
 }
 
+void CheckOther::checkSuspiciousComma()
+{
+    if (!mSettings->severity.isEnabled(Severity::style)) {
+        return;
+    }
+
+    logChecker("CheckOther::suspiciousComma");
+
+    for (const Token* tok = mTokenizer->list.front(); tok; tok = tok->next()) {
+        if (tok->str() == "," && tok->isBinaryOp()) {
+            const Token * parent = tok->astParent();
+            if (parent && Token::Match(parent->previous(), "if|while (")) {
+                if (tok->strAt(-1) == ")") {
+                    const Function * func = tok->linkAt(-1)->previous()->function();
+                    if (func && func->initArgCount > 0 && !tok->astOperand2()->hasKnownValue()) {
+                        std::vector<const Token*> arg_vec = getArguments(tok->linkAt(-1)->previous());
+                        if (arg_vec.size() < func->argCount() && arg_vec.size() > 0) {
+                            const ValueType * type1 = tok->astOperand2()->valueType();
+                            std::vector<Variable> v(func->argumentList.begin(), func->argumentList.end());
+                            const ValueType * type2 = v[arg_vec.size()].valueType();
+                            if (type1 && type2 && type1->isTypeEqual(type2)) {
+                                checkSuspiciousCommaError(tok);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+void CheckOther::checkSuspiciousCommaError(const Token *tok)
+{
+    reportError(tok, Severity::style, "suspiciousComma", "There is a suspicious comma expression directly after a function call "
+                "in an if/while condition statement, is there a misplaced paranthesis?");
+}
+
 void CheckOther::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
 {
     CheckOther checkOther(&tokenizer, &tokenizer.getSettings(), errorLogger);
@@ -4434,6 +4471,7 @@ void CheckOther::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
     checkOther.checkAccessOfMovedVariable();
     checkOther.checkModuloOfOne();
     checkOther.checkOverlappingWrite();
+    checkOther.checkSuspiciousComma();
 }
 
 void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const
@@ -4506,6 +4544,7 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
     c.comparePointersError(nullptr, nullptr, nullptr);
     c.redundantAssignmentError(nullptr, nullptr, "var", false);
     c.redundantInitializationError(nullptr, nullptr, "var", false);
+    c.checkSuspiciousCommaError(nullptr);
 
     const std::vector<const Token *> nullvec;
     c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);

lib/checkother.h

@@ -192,6 +192,8 @@ class CPPCHECKLIB CheckOther : public Check {
     void overlappingWriteUnion(const Token *tok);
     void overlappingWriteFunction(const Token *tok, const std::string& funcname);
 
+    void checkSuspiciousComma();
+
     // Error messages..
     void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, bool result);
     void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName);
@@ -249,6 +251,7 @@ class CPPCHECKLIB CheckOther : public Check {
     void knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value);
     void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
     void checkModuloOfOneError(const Token *tok);
+    void checkSuspiciousCommaError(const Token *tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
 
@@ -312,7 +315,8 @@ class CPPCHECKLIB CheckOther : public Check {
                "- shadow variable.\n"
                "- variable can be declared const.\n"
                "- calculating modulo of one.\n"
-               "- known function argument, suspicious calculation.\n";
+               "- known function argument, suspicious calculation.\n"
+               "- suspicious comma immediately after function call in if/while condition statement.\n";
     }
 
     bool diag(const Token* tok) {

releasenotes.txt

@@ -4,6 +4,7 @@ New checks:
 -
 
 Improved checking:
+- Check the suspicious comma expression directly after a function call in an if/while condition statement.
 -
 
 GUI:

test/testother.cpp

@@ -306,6 +306,8 @@ class TestOther : public TestFixture {
 
         TEST_CASE(knownConditionFloating);
         TEST_CASE(knownConditionPrefixed);
+
+        TEST_CASE(suspiciousComma);
     }
 
 #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
@@ -13093,6 +13095,43 @@ class TestOther : public TestFixture {
             "[test.cpp:2:13] -> [test.cpp:3:11]: (style) The comparison 'i > +1' is always false. [knownConditionTrueFalse]\n",
             errout_str());
     }
+
+    void suspiciousComma()
+    {
+        check("void f(int a, int b = 0);\n"
+              "void foo(int var) {\n"
+              "    if (f(100), var) {}\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:3:15]: (style) There is a suspicious comma expression directly after a function call "
+            "in an if/while condition statement, is there a misplaced paranthesis? [suspiciousComma]\n",
+            errout_str());
+
+        check("void f(int a, int b = 0);\n"
+              "void foo(int var) {\n"
+              "    while (f(100), var) {}\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:3:18]: (style) There is a suspicious comma expression directly after a function call "
+            "in an if/while condition statement, is there a misplaced paranthesis? [suspiciousComma]\n",
+            errout_str());
+
+        check("void f(int a, int b = 0);\n"
+                  "void foo(int * var) {\n"
+                  "    if (f(100), var) {}\n"
+                  "}\n");
+        ASSERT_EQUALS(
+            "",
+            errout_str());
+
+        check("void f(int a);\n"
+              "void foo(int var) {\n"
+              "    if (f(100), var) {}\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "",
+            errout_str());
+    }
 };
 
 REGISTER_TEST(TestOther)