From b85dda77daee7b0ce51585c1bb75523caa03aa4f Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 8 Apr 2018 01:13:44 -0500 Subject: [PATCH] Add a check for identical inner conditions (#1156) --- lib/checkcondition.cpp | 14 ++++++++++++++ lib/checkcondition.h | 3 +++ test/testcondition.cpp | 10 ++++++++++ 3 files changed, 27 insertions(+) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index d68ef40db..3d6a4ef02 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -585,6 +585,8 @@ void CheckCondition::multiCondition2() } else if (isOppositeCond(false, _tokenizer->isCPP(), firstCondition, cond2, _settings->library, true)) { if (!isAliased(vars)) oppositeInnerConditionError(firstCondition, cond2); + } else if(isSameExpression(_tokenizer->isCPP(), true, firstCondition, cond2, _settings->library, true)) { + identicalInnerConditionError(firstCondition, cond2); } } } else { @@ -693,6 +695,18 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* reportError(errorPath, Severity::warning, "oppositeInnerCondition", msg, CWE398, false); } +void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2) +{ + const std::string s1(tok1 ? tok1->expressionString() : "x"); + const std::string s2(tok2 ? tok2->expressionString() : "x"); + ErrorPath errorPath; + errorPath.push_back(ErrorPathItem(tok1, "outer condition: " + s1)); + errorPath.push_back(ErrorPathItem(tok2, "identical inner condition: " + s2)); + const std::string msg("Identical inner 'if' condition is always true.\n" + "Identical inner 'if' condition is always true (outer condition is '" + s1 + "' and inner condition is '" + s2 + "')."); + reportError(errorPath, Severity::warning, "identicalInnerCondition", msg, CWE398, false); +} + void CheckCondition::identicalConditionAfterEarlyExitError(const Token *cond1, const Token* cond2) { const std::string cond(cond1 ? cond1->expressionString() : "x"); diff --git a/lib/checkcondition.h b/lib/checkcondition.h index e330d471f..d05c277ef 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -133,6 +133,8 @@ private: void oppositeInnerConditionError(const Token *tok1, const Token* tok2); + void identicalInnerConditionError(const Token *tok1, const Token* tok2); + void identicalConditionAfterEarlyExitError(const Token *cond1, const Token *cond2); void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive); @@ -156,6 +158,7 @@ private: c.multiConditionError(nullptr,1); c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1); c.oppositeInnerConditionError(nullptr, nullptr); + c.identicalInnerConditionError(nullptr, nullptr); c.identicalConditionAfterEarlyExitError(nullptr, nullptr); c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false); c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 9bd2cf354..8229f4942 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -87,6 +87,8 @@ private: TEST_CASE(oppositeInnerConditionAnd); TEST_CASE(oppositeInnerConditionEmpty); + TEST_CASE(identicalInnerCondition); + TEST_CASE(identicalConditionAfterEarlyExit); TEST_CASE(clarifyCondition1); // if (a = b() < 0) @@ -1876,6 +1878,14 @@ private: check("void f1(const std::string v[10]) { if(v[0].size() > 42) if(v[1].empty()) {}} "); ASSERT_EQUALS("", errout.str()); } + + void identicalInnerCondition() { + check("void f1(int a, int b) { if(a==b) if(a==b) {}}"); + ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Identical inner 'if' condition is always true.\n", errout.str()); + + check("void f2(int a, int b) { if(a!=b) if(a!=b) {}}"); + ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Identical inner 'if' condition is always true.\n", errout.str()); + } void identicalConditionAfterEarlyExit() { check("void f(int x) {\n"