Try again to clarify warning message for new strcmp() checker
This commit is contained in:
parent
a6983dd279
commit
d292434e76
|
@ -336,7 +336,7 @@ void CheckString::incorrectStringBooleanError(const Token *tok, const std::strin
|
||||||
// always true: strcmp(str,"a")==0 || strcmp(str,"b")
|
// always true: strcmp(str,"a")==0 || strcmp(str,"b")
|
||||||
// TODO: Library configuration for string comparison functions
|
// TODO: Library configuration for string comparison functions
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
void CheckString::deadStrcmp()
|
void CheckString::overlappingStrcmp()
|
||||||
{
|
{
|
||||||
if (!_settings->isEnabled(Settings::WARNING))
|
if (!_settings->isEnabled(Settings::WARNING))
|
||||||
return;
|
return;
|
||||||
|
@ -399,29 +399,24 @@ void CheckString::deadStrcmp()
|
||||||
args2[1]->isLiteral() &&
|
args2[1]->isLiteral() &&
|
||||||
args1[1]->str() != args2[1]->str() &&
|
args1[1]->str() != args2[1]->str() &&
|
||||||
isSameExpression(_tokenizer->isCPP(), true, args1[0], args2[0], _settings->library, true))
|
isSameExpression(_tokenizer->isCPP(), true, args1[0], args2[0], _settings->library, true))
|
||||||
deadStrcmpError(tok1, tok2);
|
overlappingStrcmpError(tok1, tok2);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckString::deadStrcmpError(const Token *eq0, const Token *ne0)
|
void CheckString::overlappingStrcmpError(const Token *eq0, const Token *ne0)
|
||||||
{
|
{
|
||||||
std::string eq0Expr(eq0 ? eq0->expressionString() : std::string("strcmp(x,\"abc\")"));
|
std::string eq0Expr(eq0 ? eq0->expressionString() : std::string("strcmp(x,\"abc\")"));
|
||||||
if (eq0 && eq0->astParent()->str() == "!")
|
if (eq0 && eq0->astParent()->str() == "!")
|
||||||
eq0Expr = "!" + eq0Expr;
|
eq0Expr = "!" + eq0Expr;
|
||||||
else
|
else
|
||||||
eq0Expr += " == 0";
|
eq0Expr += " == 0";
|
||||||
|
|
||||||
const std::string ne0Expr = (ne0 ? ne0->expressionString() : std::string("strcmp(x,\"def\")")) + " != 0";
|
const std::string ne0Expr = (ne0 ? ne0->expressionString() : std::string("strcmp(x,\"def\")")) + " != 0";
|
||||||
|
|
||||||
const Token *ortok = eq0;
|
reportError(ne0, Severity::warning, "overlappingStrcmp", "The expression '" + ne0Expr + "' is suspicious. It overlaps '" + eq0Expr + "'.");
|
||||||
while (ortok && ortok->str() != "||")
|
|
||||||
ortok = ortok->astParent();
|
|
||||||
|
|
||||||
const std::string expression = ortok ? ortok->expressionString() : (eq0Expr + " || " + ne0Expr);
|
|
||||||
|
|
||||||
reportError(ne0, Severity::warning, "deadStrcmp", "The expression '" + expression + "' has a dead string comparison. The expression is logically the same as \'" + ne0Expr + "\'.");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
|
@ -57,7 +57,7 @@ public:
|
||||||
checkString.strPlusChar();
|
checkString.strPlusChar();
|
||||||
checkString.checkSuspiciousStringCompare();
|
checkString.checkSuspiciousStringCompare();
|
||||||
checkString.stringLiteralWrite();
|
checkString.stringLiteralWrite();
|
||||||
checkString.deadStrcmp();
|
checkString.overlappingStrcmp();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @brief Run checks against the simplified token list */
|
/** @brief Run checks against the simplified token list */
|
||||||
|
@ -85,8 +85,8 @@ public:
|
||||||
/** @brief %Check for suspicious code that compares string literals for equality */
|
/** @brief %Check for suspicious code that compares string literals for equality */
|
||||||
void checkAlwaysTrueOrFalseStringCompare();
|
void checkAlwaysTrueOrFalseStringCompare();
|
||||||
|
|
||||||
/** @brief %Check for dead string comparison */
|
/** @brief %Check for overlapping strcmp() */
|
||||||
void deadStrcmp();
|
void overlappingStrcmp();
|
||||||
|
|
||||||
/** @brief %Check for overlapping source and destination passed to sprintf() */
|
/** @brief %Check for overlapping source and destination passed to sprintf() */
|
||||||
void sprintfOverlappingData();
|
void sprintfOverlappingData();
|
||||||
|
@ -101,7 +101,7 @@ private:
|
||||||
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
|
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
|
||||||
void suspiciousStringCompareError(const Token* tok, const std::string& var);
|
void suspiciousStringCompareError(const Token* tok, const std::string& var);
|
||||||
void suspiciousStringCompareError_char(const Token* tok, const std::string& var);
|
void suspiciousStringCompareError_char(const Token* tok, const std::string& var);
|
||||||
void deadStrcmpError(const Token* tok1, const Token *tok2);
|
void overlappingStrcmpError(const Token* tok1, const Token *tok2);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
||||||
CheckString c(nullptr, settings, errorLogger);
|
CheckString c(nullptr, settings, errorLogger);
|
||||||
|
@ -115,7 +115,7 @@ private:
|
||||||
c.incorrectStringBooleanError(nullptr, "\"Hello World\"");
|
c.incorrectStringBooleanError(nullptr, "\"Hello World\"");
|
||||||
c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2");
|
c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2");
|
||||||
c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2");
|
c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2");
|
||||||
c.deadStrcmpError(nullptr, nullptr);
|
c.overlappingStrcmpError(nullptr, nullptr);
|
||||||
}
|
}
|
||||||
|
|
||||||
static std::string myName() {
|
static std::string myName() {
|
||||||
|
@ -130,7 +130,7 @@ private:
|
||||||
"- suspicious condition (string literals as boolean)\n"
|
"- suspicious condition (string 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"
|
||||||
"- dead string comparison; (strcmp(str,\"x\")==0 || strcmp(str,\"y\")) is logically the same as (strcmp(str,\"y\") != 0)\n";
|
"- overlapping strcmp() expression\n";
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
/// @}
|
/// @}
|
||||||
|
|
|
@ -585,7 +585,7 @@ private:
|
||||||
check("void f(const char *str) {\n"
|
check("void f(const char *str) {\n"
|
||||||
" if (strcmp(str, \"abc\") == 0 || strcmp(str, \"def\")) {}\n"
|
" if (strcmp(str, \"abc\") == 0 || strcmp(str, \"def\")) {}\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:2]: (warning) The expression 'strcmp(str,\"abc\")==0||strcmp(str,\"def\")' has a dead string comparison. The expression is logically the same as 'strcmp(str,\"def\") != 0'.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:2]: (warning) The expression 'strcmp(str,\"def\") != 0' is suspicious. It overlaps 'strcmp(str,\"abc\") == 0'.\n", errout.str());
|
||||||
|
|
||||||
check("struct X {\n"
|
check("struct X {\n"
|
||||||
" char *str;\n"
|
" char *str;\n"
|
||||||
|
@ -594,7 +594,7 @@ private:
|
||||||
"void f(const struct X *x) {\n"
|
"void f(const struct X *x) {\n"
|
||||||
" if (strcmp(x->str, \"abc\") == 0 || strcmp(x->str, \"def\")) {}\n"
|
" if (strcmp(x->str, \"abc\") == 0 || strcmp(x->str, \"def\")) {}\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:6]: (warning) The expression 'strcmp(x->str,\"abc\")==0||strcmp(x->str,\"def\")' has a dead string comparison. The expression is logically the same as 'strcmp(x->str,\"def\") != 0'.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:6]: (warning) The expression 'strcmp(x->str,\"def\") != 0' is suspicious. It overlaps 'strcmp(x->str,\"abc\") == 0'.\n", errout.str());
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue