Clarify warnings when char literals are converted to bool in conditions

This commit is contained in:
Daniel Marjamäki 2018-07-21 18:40:06 +02:00
parent f862cf603f
commit 77b653bf94
5 changed files with 28 additions and 13 deletions

View File

@ -1184,16 +1184,18 @@ void CheckCondition::alwaysTrueFalse()
continue; continue;
if (!tok->hasKnownIntValue()) if (!tok->hasKnownIntValue())
continue; continue;
if (Token::Match(tok, "%num%|%bool%")) if (Token::Match(tok, "%num%|%bool%|%char%"))
continue; continue;
if (Token::Match(tok, "! %num%|%bool%")) if (Token::Match(tok, "! %num%|%bool%|%char%"))
continue;
if (Token::Match(tok, "%oror%|&&"))
continue; continue;
const bool constIfWhileExpression = const bool constIfWhileExpression =
tok->astParent() tok->astParent()
&& Token::Match(tok->astParent()->astOperand1(), "if|while") && Token::Match(tok->astParent()->astOperand1(), "if|while")
&& !tok->isBoolean(); && !tok->isBoolean();
const bool constValExpr = Token::Match(tok, "%num%|%char%") && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number or char in boolean expression const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
if (!(constIfWhileExpression || constValExpr || compExpr)) if (!(constIfWhileExpression || constValExpr || compExpr))

View File

@ -305,9 +305,9 @@ void CheckString::checkIncorrectStringCompare()
incorrectStringCompareError(tok->next(), "substr", end->strAt(1)); incorrectStringCompareError(tok->next(), "substr", end->strAt(1));
} }
} }
} else if (Token::Match(tok, "&&|%oror%|( %str% &&|%oror%|)") && !Token::Match(tok, "( %str% )")) { } else if (Token::Match(tok, "&&|%oror%|( %str%|%char% &&|%oror%|)") && !Token::Match(tok, "( %str%|%char% )")) {
incorrectStringBooleanError(tok->next(), tok->strAt(1)); incorrectStringBooleanError(tok->next(), tok->strAt(1));
} else if (Token::Match(tok, "if|while ( %str% )")) { } else if (Token::Match(tok, "if|while ( %str%|%char% )")) {
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2)); incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
} }
} }
@ -321,7 +321,11 @@ void CheckString::incorrectStringCompareError(const Token *tok, const std::strin
void CheckString::incorrectStringBooleanError(const Token *tok, const std::string& string) void CheckString::incorrectStringBooleanError(const Token *tok, const std::string& string)
{ {
reportError(tok, Severity::warning, "incorrectStringBooleanError", "Conversion of string literal " + string + " to bool always evaluates to true.", CWE571, false); const bool charLiteral = string[0] == '\'';
reportError(tok,
Severity::warning,
charLiteral ? "incorrectCharBooleanError" : "incorrectStringBooleanError",
"Conversion of " + std::string(charLiteral ? "char" : "string") + " literal " + string + " to bool always evaluates to true.", CWE571, false);
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------

View File

@ -113,6 +113,7 @@ private:
c.suspiciousStringCompareError(nullptr, "foo"); c.suspiciousStringCompareError(nullptr, "foo");
c.suspiciousStringCompareError_char(nullptr, "foo"); c.suspiciousStringCompareError_char(nullptr, "foo");
c.incorrectStringBooleanError(nullptr, "\"Hello World\""); c.incorrectStringBooleanError(nullptr, "\"Hello World\"");
c.incorrectStringBooleanError(nullptr, "\'x\'");
c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2"); c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2");
c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2"); c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2");
c.overlappingStrcmpError(nullptr, nullptr); c.overlappingStrcmpError(nullptr, nullptr);
@ -127,7 +128,7 @@ private:
"- overlapping buffers passed to sprintf as source and destination\n" "- overlapping buffers passed to sprintf as source and destination\n"
"- incorrect length arguments for 'substr' and 'strncmp'\n" "- incorrect length arguments for 'substr' and 'strncmp'\n"
"- suspicious condition (runtime comparison of string literals)\n" "- suspicious condition (runtime comparison of string literals)\n"
"- suspicious condition (string literals as boolean)\n" "- suspicious condition (string/char literals as boolean)\n"
"- suspicious comparison of a string literal with a char* variable\n" "- suspicious comparison of a string literal with a char* variable\n"
"- suspicious comparison of '\\0' with a char* variable\n" "- suspicious comparison of '\\0' with a char* variable\n"
"- overlapping strcmp() expression\n"; "- overlapping strcmp() expression\n";

View File

@ -2419,18 +2419,14 @@ private:
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// #7750 warn about char literals in boolean expressions // #7750 char literals in boolean expressions
check("void f() {\n" check("void f() {\n"
" if('a'){}\n" " if('a'){}\n"
" if(L'b'){}\n" " if(L'b'){}\n"
" if(1 && 'c'){}\n" " if(1 && 'c'){}\n"
" int x = 'd' ? 1 : 2;\n" " int x = 'd' ? 1 : 2;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n" ASSERT_EQUALS("", errout.str());
"[test.cpp:3]: (style) Condition 'L'b'' is always true\n"
"[test.cpp:4]: (style) Condition '1&&'c'' is always true\n"
"[test.cpp:4]: (style) Condition ''c'' is always true\n"
"[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str());
// Skip literals // Skip literals
check("void f() { if(true) {} }"); check("void f() { if(true) {} }");

View File

@ -579,6 +579,18 @@ private:
" return f2(\"Hello\");\n" " return f2(\"Hello\");\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// #7750 warn about char literals in boolean expressions
check("void f() {\n"
" if('a'){}\n"
" if(L'b'){}\n"
" if(1 && 'c'){}\n"
" int x = 'd' ? 1 : 2;\n" // <- TODO
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of char literal 'a' to bool always evaluates to true.\n"
"[test.cpp:3]: (warning) Conversion of char literal 'b' to bool always evaluates to true.\n"
"[test.cpp:4]: (warning) Conversion of char literal 'c' to bool always evaluates to true.\n"
, errout.str());
} }
void deadStrcmp() { void deadStrcmp() {