diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 9d448592d..009d94cca 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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); } } } diff --git a/lib/token.cpp b/lib/token.cpp index 4948abacf..84a6ddd46 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -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()) { diff --git a/test/testother.cpp b/test/testother.cpp index 279edfe52..2a3d55995 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -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) {