Improved sizeofCalculation check:

- Bailout on expanded macros for conclusive checking
- Support for more operators
- Removed indendation counter
Improved checkSignOfUnsignedVariable:
- Made the patterns more generic
- Improved verbose error message (-> Fixed #3080)
This commit is contained in:
PKEuS 2012-03-25 11:32:00 +02:00
parent 0338153de9
commit e4d92055e7
3 changed files with 28 additions and 30 deletions

View File

@ -3151,16 +3151,10 @@ void CheckOther::sizeofCalculation()
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::simpleMatch(tok, "sizeof (")) { if (Token::simpleMatch(tok, "sizeof (")) {
unsigned int parlevel = 0; const Token* const end = tok->linkAt(1);
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) { for (const Token *tok2 = tok->tokAt(2); tok2 != end; tok2 = tok2->next()) {
if (tok2->str() == "(") if (tok2->isOp() && (!tok2->isExpandedMacro() || _settings->inconclusive) && !Token::Match(tok2, ">|<|&|*")) {
++parlevel; sizeofCalculationError(tok2, tok2->isExpandedMacro());
else if (tok2->str() == ")") {
if (parlevel <= 1)
break;
--parlevel;
} else if (Token::Match(tok2, "+|/")) {
sizeofCalculationError(tok2);
break; break;
} }
} }
@ -3168,8 +3162,12 @@ void CheckOther::sizeofCalculation()
} }
} }
void CheckOther::sizeofCalculationError(const Token *tok) void CheckOther::sizeofCalculationError(const Token *tok, bool inconclusive)
{ {
if (inconclusive)
reportInconclusiveError(tok, Severity::warning,
"sizeofCalculation", "Found calculation inside sizeof()");
else
reportError(tok, Severity::warning, reportError(tok, Severity::warning,
"sizeofCalculation", "Found calculation inside sizeof()"); "sizeofCalculation", "Found calculation inside sizeof()");
} }
@ -3253,22 +3251,22 @@ void CheckOther::checkSignOfUnsignedVariable()
// check all the code in the function // 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->classStart->link(); tok = tok->next()) {
if (Token::Match(tok, ";|(|&&|%oror% %var% <|<= 0 ;|)|&&|%oror%") && tok->next()->varId()) { if (Token::Match(tok, "%var% <|<= 0") && tok->varId() && !Token::Match(tok->previous(), "++|--|)|+|-|*|/|~|<<|>>") && !Token::Match(tok->tokAt(3), "+|-")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->next()->varId()); const Variable * var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && var->typeEndToken()->isUnsigned()) if (var && var->typeEndToken()->isUnsigned())
unsignedLessThanZeroError(tok->next(), tok->next()->str(), inconclusive); unsignedLessThanZeroError(tok, tok->str(), inconclusive);
} else if (Token::Match(tok, ";|(|&&|%oror% 0 > %var% ;|)|&&|%oror%") && tok->tokAt(3)->varId()) { } 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(3)->varId()); const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
if (var && var->typeEndToken()->isUnsigned()) if (var && var->typeEndToken()->isUnsigned())
unsignedLessThanZeroError(tok->tokAt(3), tok->strAt(3), inconclusive); unsignedLessThanZeroError(tok, tok->strAt(2), inconclusive);
} else if (Token::Match(tok, ";|(|&&|%oror% 0 <= %var% ;|)|&&|%oror%") && tok->tokAt(3)->varId()) { } 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(3)->varId()); const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
if (var && var->typeEndToken()->isUnsigned()) if (var && var->typeEndToken()->isUnsigned())
unsignedPositiveError(tok->tokAt(3), tok->strAt(3), inconclusive); unsignedPositiveError(tok, tok->strAt(2), inconclusive);
} else if (Token::Match(tok, ";|(|&&|%oror% %var% >= 0 ;|)|&&|%oror%") && tok->next()->varId()) { } else if (Token::Match(tok, "%var% >= 0") && tok->varId() && !Token::Match(tok->previous(), "++|--|)|+|-|*|/|~|<<|>>") && !Token::Match(tok->tokAt(3), "+|-")) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->next()->varId()); const Variable * var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && var->typeEndToken()->isUnsigned()) if (var && var->typeEndToken()->isUnsigned())
unsignedPositiveError(tok->next(), tok->next()->str(), inconclusive); unsignedPositiveError(tok, tok->str(), inconclusive);
} }
} }
} }
@ -3286,8 +3284,8 @@ void CheckOther::unsignedLessThanZeroError(const Token *tok, const std::string &
} else { } else {
reportError(tok, Severity::style, "unsignedLessThanZero", reportError(tok, Severity::style, "unsignedLessThanZero",
"Checking if unsigned variable '" + varname + "' is less than zero.\n" "Checking if unsigned variable '" + varname + "' is less than zero.\n"
"An unsigned variable will never be negative so it is either pointless or " "The unsigned variable '" + varname + "' will never be negative so it"
"an error to check if it is."); "is either pointless or an error to check if it is.");
} }
} }

View File

@ -259,7 +259,7 @@ private:
void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyCalculationError(const Token *tok, const std::string &op);
void clarifyConditionError(const Token *tok, bool assign, bool boolop); void clarifyConditionError(const Token *tok, bool assign, bool boolop);
void sizeofsizeofError(const Token *tok); void sizeofsizeofError(const Token *tok);
void sizeofCalculationError(const Token *tok); void sizeofCalculationError(const Token *tok, bool inconclusive);
void invalidScanfError(const Token *tok); void invalidScanfError(const Token *tok);
void wrongPrintfScanfArgumentsError(const Token* tok, void wrongPrintfScanfArgumentsError(const Token* tok,
const std::string &function, const std::string &function,
@ -349,7 +349,7 @@ private:
c.variableScopeError(0, "varname"); c.variableScopeError(0, "varname");
c.strPlusCharError(0); c.strPlusCharError(0);
c.sizeofsizeofError(0); c.sizeofsizeofError(0);
c.sizeofCalculationError(0); c.sizeofCalculationError(0, false);
c.redundantAssignmentInSwitchError(0, "varname"); c.redundantAssignmentInSwitchError(0, "varname");
c.switchCaseFallThrough(0); c.switchCaseFallThrough(0);
c.selfAssignmentError(0, "varname"); c.selfAssignmentError(0, "varname");

View File

@ -1297,7 +1297,7 @@ private:
ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof()\n", errout.str()); ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof()\n", errout.str());
check("sizeof(-a)"); check("sizeof(-a)");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof()\n", errout.str());
check("sizeof(void * const)"); check("sizeof(void * const)");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());