diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a0130aa9c..957812646 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2834,6 +2834,44 @@ void CheckOther::alwaysTrueStringVariableCompareError(const Token *tok, const st "This could be a logic bug."); } + +//----------------------------------------------------------------------------- +//----------------------------------------------------------------------------- +void CheckOther::checkSuspiciousStringCompare() +{ + if (!_settings->isEnabled("style")) + return; + + const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); + + for (const Token* tok = _tokenizer->list.front(); tok && tok->tokAt(3); tok = tok->next()) { + if (tok->next()->type() != Token::eComparisonOp) + continue; + + const Token* varTok = tok; + const Token* litTok = tok->tokAt(2); + + if (varTok->strAt(-1) == "+" || litTok->strAt(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); + + const Variable* var = symbolDatabase->getVariableFromVarId(varTok->varId()); + if (var && var->isPointer()) + suspiciousStringCompareError(tok, var->name()); + } + } +} + +void CheckOther::suspiciousStringCompareError(const Token* tok, const std::string& var) +{ + reportError(tok, Severity::warning, "literalWithCharPtrCompare", + "String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?"); +} + + //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- void CheckOther::checkModuloAlwaysTrueFalse() diff --git a/lib/checkother.h b/lib/checkother.h index 005a83d5a..e8789923e 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -75,6 +75,7 @@ public: checkOther.checkComparisonOfBoolExpressionWithInt(); checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574) checkOther.checkIncompleteArrayFill(); + checkOther.checkSuspiciousStringCompare(); } /** @brief Run checks against the simplified token list */ @@ -200,6 +201,9 @@ public: /** @brief %Check for using bad usage of strncmp and substr */ void checkIncorrectStringCompare(); + /** @brief %Check for comparison of a string literal with a char* variable */ + void checkSuspiciousStringCompare(); + /** @brief %Check for using postfix increment on bool */ void checkIncrementBoolean(); @@ -297,6 +301,7 @@ private: void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op); 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 duplicateBreakError(const Token *tok, bool inconclusive); void unreachableCodeError(const Token* tok, bool inconclusive); void assignBoolToPointerError(const Token *tok); @@ -356,6 +361,7 @@ private: c.clarifyConditionError(0, true, false); c.clarifyStatementError(0); c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12"); + c.suspiciousStringCompareError(0, "foo"); c.incorrectStringBooleanError(0, "\"Hello World\""); c.incrementBooleanError(0); c.comparisonOfBoolWithIntError(0, "varname", true); @@ -427,6 +433,7 @@ private: "* suspicious condition (assignment+comparison)\n" "* 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" "* duplicate break statement\n" "* unreachable code\n" "* testing if unsigned variable is negative\n" diff --git a/lib/token.h b/lib/token.h index 40cebd596..673ec7a51 100644 --- a/lib/token.h +++ b/lib/token.h @@ -301,6 +301,8 @@ public: } void varId(unsigned int id) { _varId = id; + if (id != 0) + _type = eVariable; } /** diff --git a/test/testother.cpp b/test/testother.cpp index 67bafa0a9..72c6e2951 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -153,6 +153,7 @@ private: TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values) TEST_CASE(alwaysTrueFalseStringCompare); + TEST_CASE(suspiciousStringCompare); TEST_CASE(checkPointerSizeof); TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfPointer); @@ -4824,6 +4825,33 @@ private: ASSERT_EQUALS("", errout.str()); } + void suspiciousStringCompare() { + check("bool foo(char* c) {\n" + " return c == \"x\";\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str()); + + check("bool foo(const char* c) {\n" + " return \"x\" == c;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str()); + + check("bool foo(char* c) {\n" + " return foo+\"x\" == c;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool foo(char* c) {\n" + " return \"x\" == c+foo;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool foo(const std::string& c) {\n" + " return \"x\" == c;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void checkPointerSizeof() { check( "char *x = malloc(10);\n"