CheckCondition: Improved checking for same conditions

danmar · danmar · commit d82805b7fe49 · 2017-09-04T22:40:34.000+02:00
diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -533,9 +533,20 @@ void CheckCondition::multiCondition2()
                             oppositeInnerConditionError(cond1, cond2);
                     }
                 } else {
-                    if (isSameExpression(_tokenizer->isCPP(), true, cond1, cond2, _settings->library, true)) {
-                        if (!isAliased(vars))
-                            sameConditionAfterEarlyExitError(cond1, cond2);
+                    std::stack<const Token *> tokens2;
+                    tokens2.push(cond2);
+                    while (!tokens2.empty()) {
+                        const Token *secondCondition = tokens2.top();
+                        tokens2.pop();
+                        if (!secondCondition)
+                            continue;
+                        if (secondCondition->str() == "||") {
+                            tokens2.push(secondCondition->astOperand1());
+                            tokens2.push(secondCondition->astOperand2());
+                        } else if (isSameExpression(_tokenizer->isCPP(), true, cond1, secondCondition, _settings->library, true)) {
+                            if (!isAliased(vars))
+                                sameConditionAfterEarlyExitError(cond1, secondCondition);
+                        }
                     }
                 }
             }
@@ -606,10 +617,11 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token*
 
 void CheckCondition::sameConditionAfterEarlyExitError(const Token *cond1, const Token* cond2)
 {
+    const std::string cond(cond1 ? cond1->expressionString() : "x");
     ErrorPath errorPath;
     errorPath.push_back(ErrorPathItem(cond1, "first condition"));
     errorPath.push_back(ErrorPathItem(cond2, "second condition"));
-    reportError(errorPath, Severity::warning, "sameConditionAfterEarlyExit", "Same condition, second condition is always false", CWE398, false);
+    reportError(errorPath, Severity::warning, "sameConditionAfterEarlyExit", "Same condition '" + cond + "', second condition is always false", CWE398, false);
 }
 
 //---------------------------------------------------------------------------
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -1513,13 +1513,6 @@ class TestCondition : public TestFixture {
               "    }\n"
               "}");
         ASSERT_EQUALS("", errout.str());
-
-        check("void f(const int *i) {\n"
-              "  if (!i) return;\n"
-              "  if (!num1tok) { *num1 = *num2; }\n"
-              "  if (!i) {}\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition, second condition is always false\n", errout.str());
     }
 
     void oppositeInnerConditionClass() {
@@ -1761,21 +1754,34 @@ class TestCondition : public TestFixture {
               "  if (x > 100) { return; }\n"
               "  if (x > 100) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Same condition, second condition is always false\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Same condition 'x>100', second condition is always false\n", errout.str());
+
+        check("void f(int x) {\n"
+              "  if (x > 100) { return; }\n"
+              "  if (x > 100 || y > 100) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Same condition 'x>100', second condition is always false\n", errout.str());
 
         check("void f(int x) {\n"
               "  if (x > 100) { return; }\n"
               "  if (abc) {}\n"
               "  if (x > 100) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition, second condition is always false\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition 'x>100', second condition is always false\n", errout.str());
 
         check("void f(int x) {\n"
               "  if (x > 100) { return; }\n"
               "  while (abc) { y = x; }\n"
               "  if (x > 100) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition, second condition is always false\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition 'x>100', second condition is always false\n", errout.str());
+
+        check("void f(const int *i) {\n"
+              "  if (!i) return;\n"
+              "  if (!num1tok) { *num1 = *num2; }\n"
+              "  if (!i) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Same condition '!i', second condition is always false\n", errout.str());
     }
 
     // clarify conditions with = and comparison

Original file line number	Diff line number	Diff line change
`@@ -533,9 +533,20 @@ void CheckCondition::multiCondition2()`
`533`	`533`	`oppositeInnerConditionError(cond1, cond2);`
`534`	`534`	`}`
`535`	`535`	`} else {`
`536`		`- if (isSameExpression(_tokenizer->isCPP(), true, cond1, cond2, _settings->library, true)) {`
`537`		`- if (!isAliased(vars))`
`538`		`- sameConditionAfterEarlyExitError(cond1, cond2);`
	`536`	`+ std::stack<const Token *> tokens2;`
	`537`	`+ tokens2.push(cond2);`
	`538`	`+ while (!tokens2.empty()) {`
	`539`	`+ const Token *secondCondition = tokens2.top();`
	`540`	`+ tokens2.pop();`
	`541`	`+ if (!secondCondition)`
	`542`	`+ continue;`
	`543`	`+ if (secondCondition->str() == "\|\|") {`
	`544`	`+ tokens2.push(secondCondition->astOperand1());`
	`545`	`+ tokens2.push(secondCondition->astOperand2());`
	`546`	`+ } else if (isSameExpression(_tokenizer->isCPP(), true, cond1, secondCondition, _settings->library, true)) {`
	`547`	`+ if (!isAliased(vars))`
	`548`	`+ sameConditionAfterEarlyExitError(cond1, secondCondition);`
	`549`	`+ }`
`539`	`550`	`}`
`540`	`551`	`}`
`541`	`552`	`}`
`@@ -606,10 +617,11 @@ void CheckCondition::oppositeInnerConditionError(const Token tok1, const Token`
`606`	`617`
`607`	`618`	`void CheckCondition::sameConditionAfterEarlyExitError(const Token cond1, const Token cond2)`
`608`	`619`	`{`
	`620`	`+ const std::string cond(cond1 ? cond1->expressionString() : "x");`
`609`	`621`	`ErrorPath errorPath;`
`610`	`622`	`errorPath.push_back(ErrorPathItem(cond1, "first condition"));`
`611`	`623`	`errorPath.push_back(ErrorPathItem(cond2, "second condition"));`
`612`		`- reportError(errorPath, Severity::warning, "sameConditionAfterEarlyExit", "Same condition, second condition is always false", CWE398, false);`
	`624`	`+ reportError(errorPath, Severity::warning, "sameConditionAfterEarlyExit", "Same condition '" + cond + "', second condition is always false", CWE398, false);`
`613`	`625`	`}`
`614`	`626`
`615`	`627`	`//---------------------------------------------------------------------------`