walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Use return instead of condition for alwaysTrueFalse check (#4446)

This commit is contained in:
Paul Fultz II 2022-10-09 14:03:48 -05:00 committed by GitHub
parent fa7e08a29f
commit bd22ea565f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/checkcondition.cpp
View File

@ -1571,16 +1571,17 @@ void CheckCondition::alwaysTrueFalse()
if (hasSizeof) if (hasSizeof)
continue; continue;
alwaysTrueFalseError(tok, &tok->values().front()); alwaysTrueFalseError(tok, condition, &tok->values().front());
} }
} }
} }
void CheckCondition::alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value) void CheckCondition::alwaysTrueFalseError(const Token* tok, const Token* condition, const ValueFlow::Value* value)
{ {
const bool alwaysTrue = value && (value->intvalue != 0); const bool alwaysTrue = value && (value->intvalue != 0);
const std::string expr = tok ? tok->expressionString() : std::string("x"); const std::string expr = tok ? tok->expressionString() : std::string("x");
const std::string errmsg = "Condition '" + expr + "' is always " + (alwaysTrue ? "true" : "false"); const std::string conditionStr = (Token::simpleMatch(condition, "return") ? "Return" : "Condition");
const std::string errmsg = conditionStr + " '" + expr + "' is always " + (alwaysTrue ? "true" : "false");
const ErrorPath errorPath = getErrorPath(tok, value, errmsg); const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
reportError(errorPath, reportError(errorPath,
Severity::style, Severity::style,

4
lib/checkcondition.h
View File

@ -160,7 +160,7 @@ private:
void clarifyConditionError(const Token *tok, bool assign, bool boolop); void clarifyConditionError(const Token *tok, bool assign, bool boolop);
void alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value); void alwaysTrueFalseError(const Token* tok, const Token* condition, const ValueFlow::Value* value);
void invalidTestForOverflow(const Token* tok, const ValueType *valueType, const std::string &replace); void invalidTestForOverflow(const Token* tok, const ValueType *valueType, const std::string &replace);
void pointerAdditionResultNotNullError(const Token *tok, const Token *calc); void pointerAdditionResultNotNullError(const Token *tok, const Token *calc);
@ -190,7 +190,7 @@ private:
c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false);
c.moduloAlwaysTrueFalseError(nullptr, "1"); c.moduloAlwaysTrueFalseError(nullptr, "1");
c.clarifyConditionError(nullptr, true, false); c.clarifyConditionError(nullptr, true, false);
c.alwaysTrueFalseError(nullptr, nullptr); c.alwaysTrueFalseError(nullptr, nullptr, nullptr);
c.invalidTestForOverflow(nullptr, nullptr, "false"); c.invalidTestForOverflow(nullptr, nullptr, "false");
c.pointerAdditionResultNotNullError(nullptr, nullptr); c.pointerAdditionResultNotNullError(nullptr, nullptr);
c.duplicateConditionalAssignError(nullptr, nullptr); c.duplicateConditionalAssignError(nullptr, nullptr);

24
test/testcondition.cpp
View File

@ -1309,7 +1309,7 @@ private:
check("int f(char c) {\n" check("int f(char c) {\n"
" return (c <= 'a' && c >= 'z');\n" " return (c <= 'a' && c >= 'z');\n"
"}", "test.cpp", false); "}", "test.cpp", false);
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'c>='z'' is always false\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Return 'c>='z'' is always false\n", errout.str());
} }
void incorrectLogicOperator7() { // opposite expressions void incorrectLogicOperator7() { // opposite expressions
@ -1885,7 +1885,7 @@ private:
" return a % 5 > 5;\n" " return a % 5 > 5;\n"
"}"); "}");
ASSERT_EQUALS( ASSERT_EQUALS(
"[test.cpp:7]: (style) Condition 'a%5>5' is always false\n" "[test.cpp:7]: (style) Return 'a%5>5' is always false\n"
"[test.cpp:2]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:2]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
"[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
"[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
@ -3205,7 +3205,7 @@ private:
" if(x == 0) { x++; return x == 0; }\n" " if(x == 0) { x++; return x == 0; }\n"
" return false;\n" " return false;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==0' is always false\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Return 'x==0' is always false\n", errout.str());
check("void f() {\n" // #6898 (Token::expressionString) check("void f() {\n" // #6898 (Token::expressionString)
" int x = 0;\n" " int x = 0;\n"
@ -3426,7 +3426,7 @@ private:
" const int b = 52;\n" " const int b = 52;\n"
" return a+b;\n" " return a+b;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'a+b' is always true\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) Return 'a+b' is always true\n", errout.str());
check("int f() {\n" check("int f() {\n"
" int a = 50;\n" " int a = 50;\n"
@ -3990,7 +3990,7 @@ private:
check("bool f(bool a, bool b) {\n" check("bool f(bool a, bool b) {\n"
" return a || ! b || ! a;\n" " return a || ! b || ! a;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition '!a' is always true\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Return '!a' is always true\n", errout.str());
// #10148 // #10148
check("void f(int i) {\n" check("void f(int i) {\n"
@ -4419,7 +4419,7 @@ private:
"[test.cpp:4]: (style) Condition 's.empty()' is always true\n" "[test.cpp:4]: (style) Condition 's.empty()' is always true\n"
"[test.cpp:5]: (style) Condition 's.empty()' is always true\n" "[test.cpp:5]: (style) Condition 's.empty()' is always true\n"
"[test.cpp:6]: (style) Condition '(bool)0' is always false\n" "[test.cpp:6]: (style) Condition '(bool)0' is always false\n"
"[test.cpp:7]: (style) Condition 's.empty()' is always true\n", "[test.cpp:7]: (style) Return 's.empty()' is always true\n",
errout.str()); errout.str());
check("int f(bool b) {\n" check("int f(bool b) {\n"
@ -4430,8 +4430,8 @@ private:
" if (b) return static_cast<int>(1);\n" " if (b) return static_cast<int>(1);\n"
" return (int)0;\n" " return (int)0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:6]: (style) Condition 'static_cast<int>(1)' is always true\n" ASSERT_EQUALS("[test.cpp:6]: (style) Return 'static_cast<int>(1)' is always true\n"
"[test.cpp:7]: (style) Condition '(int)0' is always false\n", "[test.cpp:7]: (style) Return '(int)0' is always false\n",
errout.str()); errout.str());
check("int f() { return 3; }\n" check("int f() { return 3; }\n"
@ -4439,7 +4439,7 @@ private:
"int h() { if (f()) {} }\n" "int h() { if (f()) {} }\n"
"int i() { return f() == 3; }\n"); "int i() { return f() == 3; }\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'f()' is always true\n" ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'f()' is always true\n"
"[test.cpp:4]: (style) Condition 'f()==3' is always true\n", "[test.cpp:4]: (style) Return 'f()==3' is always true\n",
errout.str()); errout.str());
check("int f() {\n" check("int f() {\n"
@ -4489,7 +4489,7 @@ private:
" return (index++) >= s;\n" " return (index++) >= s;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (style) Condition '(index++)>=s' is always false\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (style) Return '(index++)>=s' is always false\n", errout.str());
check("struct a {\n" check("struct a {\n"
" a *b() const;\n" " a *b() const;\n"
@ -4818,7 +4818,7 @@ private:
check("bool f(const int *p, const int *q) {\n" check("bool f(const int *p, const int *q) {\n"
" return p != NULL && q != NULL && p == NULL;\n" " return p != NULL && q != NULL && p == NULL;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'p==NULL' is always false\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Return 'p==NULL' is always false\n", errout.str());
} }
void alwaysTrueContainer() { void alwaysTrueContainer() {
@ -5370,7 +5370,7 @@ private:
check("bool f(const std::string &s) {\n" check("bool f(const std::string &s) {\n"
" return s.size()>2U && s[0]=='4' && s[0]=='2';\n" " return s.size()>2U && s[0]=='4' && s[0]=='2';\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 's[0]=='2'' is always false\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Return 's[0]=='2'' is always false\n", errout.str());
} }
void pointerAdditionResultNotNull() { void pointerAdditionResultNotNull() {