From 2da958efb563f9838fbdefd4e6bdcc32a9220f2c Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 5 Sep 2018 23:55:36 -0500 Subject: [PATCH] Fix issue 8722: Avoid duplicate messages due for followVar (#1367) --- lib/checkcondition.cpp | 15 +++++++++------ test/testcondition.cpp | 26 ++++++++++++++++++++++++++ test/testother.cpp | 29 +++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 3b4104040..44215d422 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -578,11 +578,13 @@ void CheckCondition::multiCondition2() if (firstCondition->str() == "&&") { tokens1.push(firstCondition->astOperand1()); tokens1.push(firstCondition->astOperand2()); - } else if (isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, &errorPath)) { - if (!isAliased(vars)) - oppositeInnerConditionError(firstCondition, cond2, errorPath); - } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true, &errorPath)) { - identicalInnerConditionError(firstCondition, cond2, errorPath); + } else if (!firstCondition->hasKnownValue()) { + if(isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, &errorPath)) { + if (!isAliased(vars)) + oppositeInnerConditionError(firstCondition, cond2, errorPath); + } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true, &errorPath)) { + identicalInnerConditionError(firstCondition, cond2, errorPath); + } } } } else { @@ -596,7 +598,8 @@ void CheckCondition::multiCondition2() if (secondCondition->str() == "||" || secondCondition->str() == "&&") { tokens2.push(secondCondition->astOperand1()); tokens2.push(secondCondition->astOperand2()); - } else if (isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true, &errorPath)) { + } else if ((!cond1->hasKnownValue() || !secondCondition->hasKnownValue()) && + isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true, &errorPath)) { if (!isAliased(vars)) identicalConditionAfterEarlyExitError(cond1, secondCondition, errorPath); } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index d8eb48a2d..862c98539 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -104,6 +104,7 @@ private: TEST_CASE(clarifyCondition8); TEST_CASE(alwaysTrue); + TEST_CASE(multiConditionAlwaysTrue); TEST_CASE(checkInvalidTestForOverflow); TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile); @@ -2518,6 +2519,31 @@ private: ASSERT_EQUALS("[test.cpp:4]: (style) Condition '!b' is always true\n", errout.str()); } + void multiConditionAlwaysTrue() { + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) continue;\n" + " if (val > 0) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) {\n" + " if (val > 0) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) {\n" + " if (val < 0) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void checkInvalidTestForOverflow() { check("void f(char *p, unsigned int x) {\n" " assert((p + x) < p);\n" diff --git a/test/testother.cpp b/test/testother.cpp index ab006538e..ac8de2f12 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -141,6 +141,7 @@ private: TEST_CASE(duplicateVarExpressionUnique); TEST_CASE(duplicateVarExpressionAssign); TEST_CASE(duplicateVarExpressionCrash); + TEST_CASE(multiConditionSameExpression); TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfPointer); @@ -4596,6 +4597,34 @@ private: } + void multiConditionSameExpression() { + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) continue;\n" + " if ((val > 0)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); + + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) {\n" + " if ((val > 0)) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); + + check("void f() {\n" + " int val = 0;\n" + " if (val < 0) {\n" + " if ((val < 0)) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val < 0' is always false.\n", errout.str()); + } + void checkSignOfUnsignedVariable() { check( "void foo() {\n"