Improved check: Added message when checking sign of a pointer.

This commit is contained in:
PKEuS 2012-08-21 03:28:02 -07:00
parent a5bca705a5
commit b4b5c80db9
3 changed files with 105 additions and 6 deletions

View File

@ -2921,7 +2921,7 @@ void CheckOther::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n
//---------------------------------------------------------------------------
// Check testing sign of unsigned variables.
// Check testing sign of unsigned variables and pointers.
//---------------------------------------------------------------------------
void CheckOther::checkSignOfUnsignedVariable()
{
@ -2942,23 +2942,31 @@ void CheckOther::checkSignOfUnsignedVariable()
continue;
// check all the code in the function
for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next()) {
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
if (Token::Match(tok, "%var% <|<= 0") && tok->varId() && !Token::Match(tok->previous(), "++|--|)|+|-|*|/|~|<<|>>") && !Token::Match(tok->tokAt(3), "+|-")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && var->typeEndToken()->isUnsigned())
unsignedLessThanZeroError(tok, tok->str(), inconclusive);
unsignedLessThanZeroError(tok, var->name(), inconclusive);
else if (var && var->isPointer() && tok->strAt(-1) != "*")
pointerLessThanZeroError(tok, inconclusive);
} else if (Token::Match(tok, "0 >|>= %var%") && tok->tokAt(2)->varId() && !Token::Match(tok->tokAt(3), "+|-|*|/") && !Token::Match(tok->previous(), "+|-|<<|>>|~")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
if (var && var->typeEndToken()->isUnsigned())
unsignedLessThanZeroError(tok, tok->strAt(2), inconclusive);
unsignedLessThanZeroError(tok, var->name(), inconclusive);
else if (var && var->isPointer() && !Token::Match(tok->tokAt(3), "[.[]"))
pointerLessThanZeroError(tok, inconclusive);
} else if (Token::Match(tok, "0 <= %var%") && tok->tokAt(2)->varId() && !Token::Match(tok->tokAt(3), "+|-|*|/") && !Token::Match(tok->previous(), "+|-|<<|>>|~")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
if (var && var->typeEndToken()->isUnsigned())
unsignedPositiveError(tok, tok->strAt(2), inconclusive);
unsignedPositiveError(tok, var->name(), inconclusive);
else if (var && var->isPointer() && !Token::Match(tok->tokAt(3), "[.[]"))
pointerPositiveError(tok, inconclusive);
} else if (Token::Match(tok, "%var% >= 0") && tok->varId() && !Token::Match(tok->previous(), "++|--|)|+|-|*|/|~|<<|>>") && !Token::Match(tok->tokAt(3), "+|-")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && var->typeEndToken()->isUnsigned())
unsignedPositiveError(tok, tok->str(), inconclusive);
unsignedPositiveError(tok, var->name(), inconclusive);
else if (var && var->isPointer() && tok->strAt(-1) != "*")
pointerPositiveError(tok, inconclusive);
}
}
}
@ -2981,6 +2989,12 @@ void CheckOther::unsignedLessThanZeroError(const Token *tok, const std::string &
}
}
void CheckOther::pointerLessThanZeroError(const Token *tok, bool inconclusive)
{
reportError(tok, Severity::style, "pointerLessThanZero",
"A pointer can not be negative so it is either pointless or an error to check if it is.", inconclusive);
}
void CheckOther::unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive)
{
if (inconclusive) {
@ -2995,6 +3009,12 @@ void CheckOther::unsignedPositiveError(const Token *tok, const std::string &varn
}
}
void CheckOther::pointerPositiveError(const Token *tok, bool inconclusive)
{
reportError(tok, Severity::style, "pointerPositive",
"A pointer can not be negative so it is either pointless or an error to check if it is not.", inconclusive);
}
/*
This check rule works for checking the "const A a = getA()" usage when getA() returns "const A &" or "A &".
In most scenarios, "const A & a = getA()" will be more efficient.

View File

@ -236,6 +236,8 @@ public:
/** @brief %Check for double free or double close operations */
void checkDoubleFree();
void doubleFreeError(const Token *tok, const std::string &varname);
/** @brief %Check for code creating redundant copies */
void checkRedundantCopy();
private:
@ -285,7 +287,9 @@ private:
void unreachableCodeError(const Token* tok, bool inconclusive);
void assignBoolToPointerError(const Token *tok);
void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive);
void pointerLessThanZeroError(const Token *tok, bool inconclusive);
void unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive);
void pointerPositiveError(const Token *tok, bool inconclusive);
void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op);
void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1);
void SuspiciousSemicolonError(const Token *tok);
@ -346,6 +350,8 @@ private:
c.unreachableCodeError(0, false);
c.unsignedLessThanZeroError(0, "varname", false);
c.unsignedPositiveError(0, "varname", false);
c.pointerLessThanZeroError(0, false);
c.pointerPositiveError(0, false);
c.bitwiseOnBooleanError(0, "varname", "&&");
c.comparisonOfBoolExpressionWithIntError(0, true);
c.SuspiciousSemicolonError(0);

View File

@ -153,6 +153,7 @@ private:
TEST_CASE(alwaysTrueFalseStringCompare);
TEST_CASE(checkPointerSizeof);
TEST_CASE(checkSignOfUnsignedVariable);
TEST_CASE(checkSignOfPointer);
TEST_CASE(checkForSuspiciousSemicolon1);
TEST_CASE(checkForSuspiciousSemicolon2);
@ -4830,6 +4831,78 @@ private:
}
}
void checkSignOfPointer() {
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (x >= 0)"
" bar();\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) A pointer can not be negative so it is either pointless or an error to check if it is not.\n", errout.str());
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (*x >= 0)"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (x < 0)"
" bar();\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) A pointer can not be negative so it is either pointless or an error to check if it is.\n", errout.str());
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (*x < 0)"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
check_signOfUnsignedVariable(
"bool foo(Bar* x) {\n"
" if (0 <= x)"
" bar();\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) A pointer can not be negative so it is either pointless or an error to check if it is not.\n", errout.str());
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (0 <= x[0])"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
check_signOfUnsignedVariable(
"bool foo(Bar* x) {\n"
" if (0 <= x.y)"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
check_signOfUnsignedVariable(
"bool foo(Bar* x) {\n"
" if (0 > x)"
" bar();\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) A pointer can not be negative so it is either pointless or an error to check if it is.\n", errout.str());
check_signOfUnsignedVariable(
"bool foo(int* x) {\n"
" if (0 > x[0])"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
check_signOfUnsignedVariable(
"bool foo(Bar* x) {\n"
" if (0 > x.y)"
" bar();\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void checkForSuspiciousSemicolon1() {
check(
"void foo() {\n"