From d19eabde42cb25bcbb136d2232b906e335f7d051 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Wed, 21 May 2014 19:10:14 +0200 Subject: [PATCH] New Check: Compare pointer with '\0' (#4070) --- lib/checkother.cpp | 26 +++++++++++++++++--------- lib/checkother.h | 3 +++ lib/tokenize.cpp | 10 +++++----- test/testother.cpp | 28 ++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 84add5554..4e260dc7f 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2900,16 +2900,18 @@ void CheckOther::checkSuspiciousStringCompare() if (Token::simpleMatch(varTok->tokAt(1), "[") || Token::simpleMatch(litTok->tokAt(1), "[")) continue; - if ((varTok->type() == Token::eString || varTok->type() == Token::eVariable) && (litTok->type() == Token::eString || litTok->type() == Token::eVariable) && litTok->type() != varTok->type()) { - if (varTok->type() == Token::eString) - std::swap(varTok, litTok); + if (varTok->type() == Token::eString || varTok->type() == Token::eNumber) + std::swap(varTok, litTok); + const Variable *var = varTok->variable(); + if (!var) + continue; - const Variable *var = varTok->variable(); - if (var) { - if (_tokenizer->isC() || - (var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]"))) - suspiciousStringCompareError(tok, var->name()); - } + if (_tokenizer->isC() || + (var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]"))) { + if (litTok->type() == Token::eString) + suspiciousStringCompareError(tok, var->name()); + else if (litTok->type() == Token::eNumber && litTok->originalName() == "'\\0'") + suspiciousStringCompareError_char(tok, var->name()); } } } @@ -2921,6 +2923,12 @@ void CheckOther::suspiciousStringCompareError(const Token* tok, const std::strin "String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?"); } +void CheckOther::suspiciousStringCompareError_char(const Token* tok, const std::string& var) +{ + reportError(tok, Severity::warning, "charLiteralWithCharPtrCompare", + "Char literal compared with pointer '" + var + "'. Did you intend to dereference it?"); +} + //----------------------------------------------------------------------------- // Check is a comparison of two variables leads to condition, which is // always true or false. diff --git a/lib/checkother.h b/lib/checkother.h index ed96853bb..d55853ad0 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -316,6 +316,7 @@ private: void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2); void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2); void suspiciousStringCompareError(const Token* tok, const std::string& var); + void suspiciousStringCompareError_char(const Token* tok, const std::string& var); void duplicateBreakError(const Token *tok, bool inconclusive); void unreachableCodeError(const Token* tok, bool inconclusive); void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive); @@ -380,6 +381,7 @@ private: c.clarifyStatementError(0); c.incorrectStringCompareError(0, "substr", "\"Hello World\""); c.suspiciousStringCompareError(0, "foo"); + c.suspiciousStringCompareError_char(0, "foo"); c.incorrectStringBooleanError(0, "\"Hello World\""); c.duplicateBranchError(0, 0); c.duplicateExpressionError(0, 0, "&&"); @@ -456,6 +458,7 @@ private: "* suspicious condition (runtime comparison of string literals)\n" "* suspicious condition (string literals as boolean)\n" "* suspicious comparison of a string literal with a char* variable\n" + "* suspicious comparison of '\\0' with a char* variable\n" "* duplicate break statement\n" "* unreachable code\n" "* testing if unsigned variable is negative\n" diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index c214d52cf..4c0ef597b 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -1833,11 +1833,12 @@ void Tokenizer::simplifyNull() for (Token *tok = list.front(); tok; tok = tok->next()) { if (tok->str() == "NULL" && !Token::Match(tok->previous(), "[(,] NULL [,)]")) tok->str("0"); - else if (tok->str() == "__null" || tok->str() == "'\\0'" || tok->str() == "'\\x0'") + else if (tok->str() == "__null" || tok->str() == "'\\0'" || tok->str() == "'\\x0'") { + tok->originalName(tok->str()); tok->str("0"); - else if (tok->isNumber() && - MathLib::isInt(tok->str()) && - MathLib::toLongNumber(tok->str()) == 0) + } else if (tok->isNumber() && + MathLib::isInt(tok->str()) && + MathLib::toLongNumber(tok->str()) == 0) tok->str("0"); } @@ -4394,7 +4395,6 @@ void Tokenizer::simplifyCompoundAssignment() // Remove the whole statement if it says: "+=0;", "-=0;", "*=1;" or "/=1;" if (Token::Match(tok, "+=|-= 0 ;") || - Token::Match(tok, "+=|-= '\\0' ;") || Token::simpleMatch(tok, "|= 0 ;") || Token::Match(tok, "*=|/= 1 ;")) { tok = tok1; diff --git a/test/testother.cpp b/test/testother.cpp index 5d6dab69e..b9c74e8d3 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -165,6 +165,7 @@ private: TEST_CASE(alwaysTrueFalseStringCompare); TEST_CASE(suspiciousStringCompare); + TEST_CASE(suspiciousStringCompare_char); TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfPointer); @@ -5231,6 +5232,33 @@ private: ASSERT_EQUALS("", errout.str()); } + void suspiciousStringCompare_char() { + check("bool foo(char* c) {\n" + " return c == '\\0';\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str()); + + check("bool foo(char* c) {\n" + " return '\\0' != c;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str()); + + check("bool foo(char c) {\n" + " return c == '\\0';\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool foo(char c) {\n" + " return c == 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(char* c) {\n" + " 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()); + } + void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) { // Clear the error buffer.. errout.str("");