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

Suspicious string comparison: Refactoring using AST. Fixed FP in Lac.

This commit is contained in:
Daniel Marjamäki 2014-07-28 14:27:35 +02:00
parent 90bc59e0fa
commit fdfea717c6
3 changed files with 68 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
lib/checkother.cpp
View File

@ -2886,32 +2886,50 @@ void CheckOther::checkSuspiciousStringCompare()
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
if (tok->next()->type() != Token::eComparisonOp)
continue;
const Token* varTok = tok;
const Token* litTok = tok->tokAt(2);
if (!_tokenizer->isC() && (varTok->strAt(-1) == "+" || litTok->strAt(1) == "+"))
continue;
// rough filter for index access (#5734). Might cause false negatives in multidimensional structures
if (Token::simpleMatch(varTok->tokAt(1), "[") || Token::simpleMatch(litTok->tokAt(1), "["))
if (tok->type() != Token::eComparisonOp)
continue;
const Token* varTok = tok->astOperand1();
const Token* litTok = tok->astOperand2();
if (varTok->type() == Token::eString || varTok->type() == Token::eNumber)
std::swap(varTok, litTok);
const Variable *var = varTok->variable();
if (!var)
else if (litTok->type() != Token::eString && litTok->type() != Token::eNumber)
continue;
// Pointer addition?
if (varTok->str() == "+" && _tokenizer->isC()) {
const Token *tokens[2] = { varTok->astOperand1(), varTok->astOperand2() };
for (int nr = 0; nr < 2; nr++) {
const Token *t = tokens[nr];
while (t && t->str() == ".")
t = t->astOperand2();
if (t && t->variable() && t->variable()->isPointer())
varTok = t;
}
}
if (varTok->str() == "*") {
if (!_tokenizer->isC() || varTok->astOperand2() != nullptr || litTok->type() != Token::eString)
continue;
varTok = varTok->astOperand1();
}
while (varTok && varTok->str() == ".")
varTok = varTok->astOperand2();
if (!varTok || !varTok->isName())
continue;
const Variable *var = varTok->variable();
while (Token::Match(varTok->astParent(), "[.*]"))
varTok = varTok->astParent();
const std::string varname = varTok->expressionString();
if (litTok->type() == Token::eString) {
if (_tokenizer->isC() ||
(var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]")))
suspiciousStringCompareError(tok, var->name());
} else if (litTok->type() == Token::eNumber && litTok->originalName() == "'\\0'") {
if (var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]"))
suspiciousStringCompareError_char(tok, var->name());
if (_tokenizer->isC() || (var && var->isPointer()))
suspiciousStringCompareError(tok, varname);
} else if (litTok->originalName() == "'\\0'" && var && var->isPointer()) {
suspiciousStringCompareError_char(tok, varname);
}
}
}

20
lib/token.cpp
View File

@ -1059,6 +1059,22 @@ bool Token::isCalculation() const
return true;
}
static bool isUnaryPreOp(const Token *op)
{
if (!op->astOperand1() || op->astOperand2())
return false;
if (!Token::Match(op, "++|--"))
return true;
const Token *tok = op->astOperand1();
for (int distance = 1; distance < 10; distance++) {
if (tok == op->tokAt(-distance))
return false;
if (tok == op->tokAt(distance))
return true;
}
return false; // <- guess
}
std::string Token::expressionString() const
{
const Token * const top = this;
@ -1066,12 +1082,12 @@ std::string Token::expressionString() const
while (start->astOperand1() && start->astOperand2())
start = start->astOperand1();
const Token *end = top;
while (end->astOperand1() && end->astOperand2()) {
while (end->astOperand1() && (end->astOperand2() || isUnaryPreOp(end))) {
if (Token::Match(end,"(|[")) {
end = end->link();
break;
}
end = end->astOperand2();
end = end->astOperand2() ? end->astOperand2() : end->astOperand1();
}
std::string ret;
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {

16
test/testother.cpp
View File

@ -5239,7 +5239,7 @@ private:
check("bool foo(Foo c) {\n"
" return \"x\" == c.foo;\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c.foo'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(const std::string& c) {\n"
" return \"x\" == c;\n"
@ -5255,7 +5255,7 @@ private:
check("bool foo() {\n"
"MyString *str=Getter();\n"
"return *str==\"bug\"; }\n", "test.c");
ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable 'str'. Did you intend to use strcmp() instead?\n", errout.str());
ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable '*str'. Did you intend to use strcmp() instead?\n", errout.str());
// Ticket #4257
check("bool foo() {\n"
@ -5334,6 +5334,18 @@ private:
" if(c == '\\0') bar();\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", "", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return a.x.str == '\\0';"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Char literal compared with pointer 'a.x.str'. Did you intend to dereference it?\n", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return *a.x.str == '\\0';"
"}");
ASSERT_EQUALS("", errout.str());
}
void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {