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

Improved bitwise on boolean check to make it working on more code patterns 

Refactorizations in checkother.cpp:
- Make use of symboldabase instead of: indentation counters, manual detection of variable declarations
- Removed some indexing variables to reduce calls to tokAt and the numbers given to this function
- Use tok->nextArgument() to jump to a specific argument
This commit is contained in:
PKEuS 2012-03-03 21:14:20 +01:00
parent fbef32d27b
commit ef6e381d47
2 changed files with 101 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

173
lib/checkother.cpp
View File

@ -232,13 +232,17 @@ void CheckOther::checkBitwiseOnBoolean()
return; return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "(|.|return|&&|%oror% %var% [&|]")) { if (Token::Match(tok, "(|.|return|&&|%oror%|throw|, %var% [&|]")) {
if (tok->next()->varId()) { const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->next()->varId());
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->next()->varId()); if (var && var->typeEndToken()->str() == "bool") {
if (var && (var->typeStartToken() == var->typeEndToken()) && bitwiseOnBooleanError(tok->next(), var->name(), tok->strAt(2) == "&" ? "&&" : "||");
var->typeStartToken()->str() == "bool") { tok = tok->tokAt(2);
bitwiseOnBooleanError(tok->next(), tok->next()->str(), tok->strAt(2) == "&" ? "&&" : "||"); }
} } else if (Token::Match(tok, "[&|] %var% )|.|return|&&|%oror%|throw|,") && (!tok->previous() || !tok->previous()->isExtendedOp() || tok->strAt(-1) == ")")) {
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->next()->varId());
if (var && var->typeEndToken()->str() == "bool") {
bitwiseOnBooleanError(tok->next(), var->name(), tok->str() == "&" ? "&&" : "||");
tok = tok->tokAt(2);
} }
} }
} }
@ -287,13 +291,9 @@ void CheckOther::SuspiciousSemicolonError(const Token* tok)
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::warningOldStylePointerCast() void CheckOther::warningOldStylePointerCast()
{ {
if (!_settings->isEnabled("style")) { // Only valid on C++ code
if (!_settings->isEnabled("style") || !_tokenizer->isCPP())
return; return;
}
if (!_tokenizer->isCPP()) {
return;
}
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
// Old style pointer casting.. // Old style pointer casting..
@ -301,19 +301,16 @@ void CheckOther::warningOldStylePointerCast()
!Token::Match(tok, "( const| %type% * ) (| new")) !Token::Match(tok, "( const| %type% * ) (| new"))
continue; continue;
unsigned char addToIndex = 0;
if (tok->strAt(1) == "const") if (tok->strAt(1) == "const")
addToIndex = 1; tok = tok->next();
if (tok->strAt(4 + addToIndex) == "const") if (tok->strAt(4) == "const")
continue; continue;
// Is "type" a class? // Is "type" a class?
const std::string pattern("class " + tok->strAt(1 + addToIndex)); const std::string pattern("class|struct " + tok->strAt(1));
if (!Token::findmatch(_tokenizer->tokens(), pattern.c_str())) if (Token::findmatch(_tokenizer->tokens(), pattern.c_str()))
continue; cstyleCastError(tok);
cstyleCastError(tok);
} }
} }
@ -466,25 +463,24 @@ void CheckOther::checkSizeofForArrayParameter()
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "sizeof ( %var% )") || Token::Match(tok, "sizeof %var%")) { if (Token::Match(tok, "sizeof ( %var% )") || Token::Match(tok, "sizeof %var%")) {
unsigned short tokIdx = 1; const Token* varTok = tok->next();
if (tok->strAt(tokIdx) == "(") { if (varTok->str() == "(") {
++tokIdx; varTok = varTok->next();
} }
if (tok->tokAt(tokIdx)->varId() > 0) { if (varTok->varId() > 0) {
const Variable *var = symbolDatabase->getVariableFromVarId(tok->tokAt(tokIdx)->varId()); const Variable *var = symbolDatabase->getVariableFromVarId(varTok->varId());
if (var) { if (var) {
const Token *declTok = var->nameToken(); const Token *declTok = var->nameToken();
if (Token::simpleMatch(declTok->next(), "[")) { if (declTok && declTok->next()->str() == "[") {
declTok = declTok->next()->link(); declTok = declTok->next()->link()->next();
// multidimensional array // multidimensional array
while (Token::simpleMatch(declTok->next(), "[")) { while (declTok->str() == "[") {
declTok = declTok->next()->link(); declTok = declTok->link()->next();
} }
if (!(Token::Match(declTok->next(), "= %str%")) && !(Token::simpleMatch(declTok->next(), "= {")) && !(Token::simpleMatch(declTok->next(), ";"))) { if (!(Token::Match(declTok, "= %str%")) && !(Token::simpleMatch(declTok, "= {")) && !(Token::simpleMatch(declTok, ";"))) {
if (Token::simpleMatch(declTok->next(), ",")) { if (declTok->str() == ",") {
declTok = declTok->next(); while (declTok->str() != ";") {
while (!Token::simpleMatch(declTok, ";")) { if (declTok->str() == ")") {
if (Token::simpleMatch(declTok, ")")) {
sizeofForArrayParameterError(tok); sizeofForArrayParameterError(tok);
break; break;
} }
@ -495,7 +491,7 @@ void CheckOther::checkSizeofForArrayParameter()
} }
} }
} }
if (Token::simpleMatch(declTok->next(), ")")) { if (declTok->str() == ")") {
sizeofForArrayParameterError(tok); sizeofForArrayParameterError(tok);
} }
} }
@ -524,16 +520,16 @@ void CheckOther::sizeofForArrayParameterError(const Token *tok)
void CheckOther::checkSizeofForStrncmpSize() void CheckOther::checkSizeofForStrncmpSize()
{ {
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
static const char pattern0[] = "strncmp (";
static const char pattern1[] = "sizeof ( %var% ) )";
static const char pattern2[] = "sizeof %var% )";
// danmar : this is inconclusive in case the size parameter is // danmar : this is inconclusive in case the size parameter is
// sizeof(char *) by intention. // sizeof(char *) by intention.
if (!_settings->inconclusive || !_settings->isEnabled("style")) if (!_settings->inconclusive || !_settings->isEnabled("style"))
return; return;
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
static const char pattern0[] = "strncmp (";
static const char pattern1[] = "sizeof ( %var% ) )";
static const char pattern2[] = "sizeof %var% )";
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, pattern0)) { if (Token::Match(tok, pattern0)) {
const Token *tokVar = tok->tokAt(2); const Token *tokVar = tok->tokAt(2);
@ -580,20 +576,21 @@ void CheckOther::checkRedundantAssignmentInSwitch()
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
const char switchPattern[] = "switch ( %any% ) { case";
const char breakPattern[] = "break|continue|return|exit|goto|throw"; const char breakPattern[] = "break|continue|return|exit|goto|throw";
const char functionPattern[] = "%var% ("; const char functionPattern[] = "%var% (";
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
// Find the beginning of a switch. E.g.: // Find the beginning of a switch. E.g.:
// switch (var) { ... // switch (var) { ...
const Token *tok = Token::findmatch(_tokenizer->tokens(), switchPattern); for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
while (tok) { if (i->type != Scope::eSwitch || !i->classStart)
continue;
// Check the contents of the switch statement // Check the contents of the switch statement
std::map<unsigned int, const Token*> varsAssigned; std::map<unsigned int, const Token*> varsAssigned;
std::map<unsigned int, const Token*> stringsCopied; std::map<unsigned int, const Token*> stringsCopied;
int indentLevel = 0; for (const Token *tok2 = i->classStart->next(); tok2 != i->classEnd; tok2 = tok2->next()) {
for (const Token *tok2 = tok->tokAt(5); tok2; tok2 = tok2->next()) {
if (tok2->str() == "{") { if (tok2->str() == "{") {
// Inside a conditional or loop. Don't mark variable accesses as being redundant. E.g.: // Inside a conditional or loop. Don't mark variable accesses as being redundant. E.g.:
// case 3: b = 1; // case 3: b = 1;
@ -612,14 +609,7 @@ void CheckOther::checkRedundantAssignmentInSwitch()
} }
} }
tok2 = endOfConditional; tok2 = endOfConditional;
} else }
++ indentLevel;
} else if (tok2->str() == "}") {
-- indentLevel;
// End of the switch block
if (indentLevel < 0)
break;
} }
// Variable assignment. Report an error if it's assigned to twice before a break. E.g.: // Variable assignment. Report an error if it's assigned to twice before a break. E.g.:
@ -658,10 +648,7 @@ void CheckOther::checkRedundantAssignmentInSwitch()
if (tok2->str() != "strcpy" && tok2->str() != "strncpy") if (tok2->str() != "strcpy" && tok2->str() != "strncpy")
stringsCopied.clear(); stringsCopied.clear();
} }
} }
tok = Token::findmatch(tok->next(), switchPattern);
} }
} }
@ -1160,26 +1147,18 @@ void CheckOther::invalidFunctionUsage()
if (!Token::Match(tok, "strtol|strtoul (")) if (!Token::Match(tok, "strtol|strtoul ("))
continue; continue;
tok = tok->tokAt(2);
// Locate the third parameter of the function call.. // Locate the third parameter of the function call..
unsigned int param = 1; for (int i = 0; i < 2 && tok; i++)
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) { tok = tok->nextArgument();
if (tok2->str() == "(")
tok2 = tok2->link(); if (Token::Match(tok, "%num% )")) {
else if (tok2->str() == ")") const MathLib::bigint radix = MathLib::toLongNumber(tok->str());
break; if (!(radix == 0 || (radix >= 2 && radix <= 36))) {
else if (tok2->str() == ",") { dangerousUsageStrtolError(tok);
++param;
if (param == 3) {
if (Token::Match(tok2, ", %num% )")) {
const MathLib::bigint radix = MathLib::toLongNumber(tok2->next()->str());
if (!(radix == 0 || (radix >= 2 && radix <= 36))) {
dangerousUsageStrtolError(tok2);
}
}
break;
}
} }
} } else
break;
} }
// sprintf|snprintf overlapping data // sprintf|snprintf overlapping data
@ -1561,20 +1540,27 @@ void CheckOther::invalidPrintfArgTypeError_float(const Token* tok, unsigned int
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// if (!x==3) <- Probably meant to be "x!=3" // if (!x==3) <- Probably meant to be "x!=3"
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
static bool isBool(const Variable* var)
{
return(var && var->typeEndToken()->str() == "bool");
}
static bool isNonBoolStdType(const Variable* var)
{
return(var && var->typeEndToken()->isStandardType() && var->typeEndToken()->str() != "bool");
}
void CheckOther::checkComparisonOfBoolWithInt() void CheckOther::checkComparisonOfBoolWithInt()
{ {
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
std::map<unsigned int, bool> boolvars; // Contains all declarated standard type variables and indicates whether its a bool or not. const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "[{};(,] %type% %var% [;=,)]") && tok->next()->isStandardType() && tok->tokAt(2)->varId() > 0) { // Declaration of standard type variable if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %num%")) { // Comparing variable with number
boolvars[tok->tokAt(2)->varId()] = (tok->strAt(1) == "bool");
} else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %num%")) { // Comparing variable with number
const Token *varTok = tok; const Token *varTok = tok;
const Token *numTok = tok->tokAt(2); const Token *numTok = tok->tokAt(2);
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId()); if (isBool(symbolDatabase->getVariableFromVarId(varTok->varId())) && // Variable has to be a boolean
if (iVar != boolvars.end() && iVar->second && // Variable has to be a boolean
((tok->strAt(1) != "==" && tok->strAt(1) != "!=") || ((tok->strAt(1) != "==" && tok->strAt(1) != "!=") ||
(MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1 (MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1
comparisonOfBoolWithIntError(varTok, numTok->str()); comparisonOfBoolWithIntError(varTok, numTok->str());
@ -1582,8 +1568,7 @@ void CheckOther::checkComparisonOfBoolWithInt()
} else if (Token::Match(tok, "%num% >|>=|==|!=|<=|< %var%")) { // Comparing number with variable } else if (Token::Match(tok, "%num% >|>=|==|!=|<=|< %var%")) { // Comparing number with variable
const Token *varTok = tok->tokAt(2); const Token *varTok = tok->tokAt(2);
const Token *numTok = tok; const Token *numTok = tok;
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId()); if (isBool(symbolDatabase->getVariableFromVarId(varTok->varId())) && // Variable has to be a boolean
if (iVar != boolvars.end() && iVar->second && // Variable has to be a boolean
((tok->strAt(1) != "==" && tok->strAt(1) != "!=") || ((tok->strAt(1) != "==" && tok->strAt(1) != "!=") ||
(MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1 (MathLib::toLongNumber(numTok->str()) != 0 && MathLib::toLongNumber(numTok->str()) != 1))) { // == 0 and != 0 are allowed, for C also == 1 and != 1
comparisonOfBoolWithIntError(varTok, numTok->str()); comparisonOfBoolWithIntError(varTok, numTok->str());
@ -1591,29 +1576,23 @@ void CheckOther::checkComparisonOfBoolWithInt()
} else if (Token::Match(tok, "true|false >|>=|==|!=|<=|< %var%")) { // Comparing boolean constant with variable } else if (Token::Match(tok, "true|false >|>=|==|!=|<=|< %var%")) { // Comparing boolean constant with variable
const Token *varTok = tok->tokAt(2); const Token *varTok = tok->tokAt(2);
const Token *constTok = tok; const Token *constTok = tok;
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId()); if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(varTok->varId()))) { // Variable has to be of non-boolean standard type
if (iVar != boolvars.end() && !iVar->second) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(varTok, constTok->str()); comparisonOfBoolWithIntError(varTok, constTok->str());
} }
} else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< true|false")) { // Comparing variable with boolean constant } else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< true|false")) { // Comparing variable with boolean constant
const Token *varTok = tok; const Token *varTok = tok;
const Token *constTok = tok->tokAt(2); const Token *constTok = tok->tokAt(2);
std::map<unsigned int, bool>::const_iterator iVar = boolvars.find(varTok->varId()); if (isNonBoolStdType(symbolDatabase->getVariableFromVarId(varTok->varId()))) { // Variable has to be of non-boolean standard type
if (iVar != boolvars.end() && !iVar->second) { // Variable has to be of non-boolean standard type
comparisonOfBoolWithIntError(varTok, constTok->str()); comparisonOfBoolWithIntError(varTok, constTok->str());
} }
} else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %var%") && } else if (Token::Match(tok, "%var% >|>=|==|!=|<=|< %var%") &&
!Token::Match(tok->tokAt(3), ".|::|(")) { // Comparing two variables, one of them boolean, one of them integer !Token::Match(tok->tokAt(3), ".|::|(")) { // Comparing two variables, one of them boolean, one of them integer
const Token *var1Tok = tok->tokAt(2); const Variable* var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
const Token *var2Tok = tok; const Variable* var2 = symbolDatabase->getVariableFromVarId(tok->varId());
std::map<unsigned int, bool>::const_iterator iVar1 = boolvars.find(var1Tok->varId()); if (isBool(var1) && isNonBoolStdType(var2)) // Comparing boolean with non-bool standard type
std::map<unsigned int, bool>::const_iterator iVar2 = boolvars.find(var2Tok->varId()); comparisonOfBoolWithIntError(tok, var1->name());
if (iVar1 != boolvars.end() && iVar2 != boolvars.end()) { else if (isNonBoolStdType(var1) && isBool(var2)) // Comparing non-bool standard type with boolean
if (iVar1->second && !iVar2->second) // Comparing boolean with non-bool standard type comparisonOfBoolWithIntError(tok, var2->name());
comparisonOfBoolWithIntError(var2Tok, var1Tok->str());
else if (!iVar1->second && iVar2->second) // Comparing non-bool standard type with boolean
comparisonOfBoolWithIntError(var2Tok, var2Tok->str());
}
} else if (Token::Match(tok, "( ! %var% ==|!= %num% )")) { } else if (Token::Match(tok, "( ! %var% ==|!= %num% )")) {
const Token *numTok = tok->tokAt(4); const Token *numTok = tok->tokAt(4);
if (numTok && numTok->str() != "0" && numTok->str() != "1") { if (numTok && numTok->str() != "0" && numTok->str() != "1") {

25
test/testother.cpp
View File

@ -3501,6 +3501,31 @@ private:
" if(a | !b) {}\n" " if(a | !b) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean || ?\n", errout.str());
check("void f(bool a, int b) {\n"
" if(a & b) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str());
check("void f(int a, bool b) {\n"
" if(a & b) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'b' is used in bitwise operation. Did you mean && ?\n", errout.str());
check("void f(bool a, bool b) {\n"
" if(a & b) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Boolean variable 'a' is used in bitwise operation. Did you mean && ?\n", errout.str());
check("void f(int a, int b) {\n"
" if(a & b) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(bool b) {\n"
" foo(bar, &b);\n"
"}");
ASSERT_EQUALS("", errout.str());
} }
void incorrectStringCompare() { void incorrectStringCompare() {