From 0b751dcc1b35afdf9ae1f7fcaa233620569fae80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 1 Sep 2017 23:24:15 +0200 Subject: [PATCH] Opposite conditions: Better matching when outer condition uses && --- lib/astutils.cpp | 8 +++++++- lib/checkcondition.cpp | 4 ++-- test/testcondition.cpp | 10 ++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 608374731..bb923df57 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -256,8 +256,14 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token if (cond2->str() == "!") return isOppositeCond(isNot, cpp, cond2, cond1, library, pure); - if (!cond1->isComparisonOp() || !cond2->isComparisonOp()) + if (!cond1->isComparisonOp() || !cond2->isComparisonOp()) { + if (!isNot && cond1->str() == "&&" && cond2->isComparisonOp()) { + return isOppositeCond(isNot, cpp, cond1->astOperand1(), cond2, library, pure) || + isOppositeCond(isNot, cpp, cond1->astOperand2(), cond2, library, pure); + } + return false; + } const std::string &comp1 = cond1->str(); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index bbb09bb6f..e8ac2d8e5 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -540,7 +540,7 @@ void CheckCondition::oppositeInnerCondition() const Token *cond2 = ifToken->next()->astOperand2(); if (isOppositeCond(false, _tokenizer->isCPP(), cond1, cond2, _settings->library, true)) - oppositeInnerConditionError(scope->classDef, cond2); + oppositeInnerConditionError(cond1, cond2); } } @@ -548,7 +548,7 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* { ErrorPath errorPath; errorPath.push_back(ErrorPathItem(tok1, "outer condition")); - errorPath.push_back(ErrorPathItem(tok2, "opposite inner condition => always false when outer condition is true")); + errorPath.push_back(ErrorPathItem(tok2, "opposite inner condition")); reportError(errorPath, Severity::warning, "oppositeInnerCondition", "Opposite inner 'if' condition leads to a dead code block.", CWE398, false); } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 5598de32c..62fac42b2 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -77,6 +77,7 @@ private: TEST_CASE(oppositeInnerCondition); TEST_CASE(oppositeInnerCondition2); + TEST_CASE(oppositeInnerConditionAnd); TEST_CASE(clarifyCondition1); // if (a = b() < 0) TEST_CASE(clarifyCondition2); // if (a & b == c) @@ -1699,6 +1700,15 @@ private: , errout.str()); } + void oppositeInnerConditionAnd() { + check("void f(int x) {\n" + " if (a>3 && x > 100) {\n" + " if (x < 10) {}\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); + } + // clarify conditions with = and comparison void clarifyCondition1() { check("void f() {\n"