From c8e661f61faffa97c60a4bddebe8d0772fa8a6fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 7 Apr 2021 19:46:00 +0200 Subject: [PATCH] assignmentInCondition: Clarify error message --- lib/checkcondition.cpp | 6 +++++- lib/checkcondition.h | 2 +- test/testcondition.cpp | 7 ++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 4bacccf1d..c6513dddb 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1716,6 +1716,8 @@ void CheckCondition::checkAssignmentInCondition() // Is this assignment of container/iterator? if (!tok->valueType()) continue; + if (tok->valueType()->pointer > 0) + continue; if (tok->valueType()->type != ValueType::Type::CONTAINER && tok->valueType()->type != ValueType::Type::ITERATOR) continue; @@ -1732,11 +1734,13 @@ void CheckCondition::checkAssignmentInCondition() void CheckCondition::assignmentInCondition(const Token *eq) { + std::string expr = eq ? eq->expressionString() : "x=y"; + reportError( eq, Severity::style, "assignmentInCondition", - "Assignment in condition should probably be comparison.", + "Suspicious assignment in condition. Condition '" + expr + "' is always true.", CWE571, Certainty::normal); } diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 51c2e04ee..a08994b16 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -211,7 +211,7 @@ private: "- Comparisons of modulo results that are always true/false.\n" "- Known variable values => condition is always true/false\n" "- Invalid test for overflow. Some mainstream compilers remove such overflow tests when optimising code.\n" - "- Assignment of container/iterator in condition should probably be comparison.\n"; + "- Suspicious assignment of container/iterator in condition => condition is always true.\n"; } }; /// @} diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 8282d3178..259bda59c 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4160,7 +4160,12 @@ private: check("void f(std::string s) {\n" " if (s=\"123\"){}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Assignment in condition should probably be comparison.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious assignment in condition. Condition 's=\"123\"' is always true.\n", errout.str()); + + check("void f(std::string *p) {\n" + " if (p=foo()){}\n" + "}"); + ASSERT_EQUALS("", errout.str()); } };