diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c97c6bccf..b6ef2d584 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -419,6 +419,37 @@ void CheckOther::sizeofForArrayParameterError(const Token *tok) ); } +void CheckOther::checkSizeofForStrncmpSize() +{ + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + const char pattern1[] = "strncmp ( %any , %any% , sizeof ( %var% ) )"; + const char pattern2[] = "strncmp ( %any , %any% , sizeof %var% )"; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::Match(tok, pattern1) || Token::Match(tok, pattern2)) { + int tokIdx = 7; + if (tok->tokAt(tokIdx)->str() == "(") + ++tokIdx; + const Token *tokVar = tok->tokAt(tokIdx); + if (tokVar->varId() > 0) { + const Variable *var = symbolDatabase->getVariableFromVarId(tokVar->varId()); + if (var && var->nameToken()->strAt(-1) == "*") { + sizeofForStrncmpError(tokVar); + } + } + } + } +} + +void CheckOther::sizeofForStrncmpError(const Token *tok) +{ + reportError(tok, Severity::warning, "strncmpLen", + "Passing sizeof(pointer) as the last argument to strncmp.\n" + "Passing a pointer to sizeof returns the size of the pointer, not " + "the number of characters pointed to by that pointer. This " + "means that only 4 or 8 (on 64-bit systems) characters are " + "compared, which is probably not what was expected."); +} + //--------------------------------------------------------------------------- // switch (x) // { @@ -2253,8 +2284,9 @@ void CheckOther::checkAlwaysTrueOrFalseStringCompare() if (!_settings->isEnabled("style") && !_settings->isEnabled("performance")) return; - const char pattern1[] = "strcmp|stricmp|strcmpi|strcasecmp|wcscmp ( %str% , %str% )"; + const char pattern1[] = "strncmp|strcmp|stricmp|strcmpi|strcasecmp|wcscmp ( %str% , %str% "; const char pattern2[] = "QString :: compare ( %str% , %str% )"; + const char pattern3[] = "strncmp|strcmp|stricmp|strcmpi|strcasecmp|wcscmp ( %var% , %var% "; const Token *tok = _tokenizer->tokens(); while (tok && (tok = Token::findmatch(tok, pattern1)) != NULL) { @@ -2267,6 +2299,20 @@ void CheckOther::checkAlwaysTrueOrFalseStringCompare() alwaysTrueFalseStringCompareError(tok, tok->strAt(4), tok->strAt(6)); tok = tok->tokAt(7); } + + if (!_settings->inconclusive) + return; + + tok = _tokenizer->tokens(); + while (tok && (tok = Token::findmatch(tok, pattern3)) != NULL) { + const Token *var1 = tok->tokAt(2); + const Token *var2 = tok->tokAt(4); + const std::string &str1 = var1->str(); + const std::string &str2 = var2->str(); + if (str1 == str2) + alwaysTrueStringVariableCompareError(tok, str1, str2); + tok = tok->tokAt(5); + } } void CheckOther::alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2) @@ -2289,6 +2335,14 @@ void CheckOther::alwaysTrueFalseStringCompareError(const Token *tok, const std:: } } +void CheckOther::alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2) +{ + reportError(tok, Severity::warning, "stringCompare", + "Comparison of identical string variables.\n" + "The compared strings, '" + str1 + "' and '" + str2 + "', are identical. " + "This could be a logic bug."); +} + //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- void CheckOther::sizeofsizeof() diff --git a/lib/checkother.h b/lib/checkother.h index 8dcd03133..7f44fd07a 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -58,6 +58,7 @@ public: checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkAssignmentInAssert(); checkOther.checkSizeofForArrayParameter(); + checkOther.checkSizeofForStrncmpSize(); checkOther.checkSizeofForNumericParameter(); checkOther.checkSelfAssignment(); checkOther.checkDuplicateIf(); @@ -196,6 +197,9 @@ public: /** @brief %Check for using sizeof with array given as function argument */ void checkSizeofForArrayParameter(); + /** @brief %Check for using sizeof with a char pointer */ + void checkSizeofForStrncmpSize(); + /** @brief %Check for using sizeof with numeric given as function argument */ void checkSizeofForNumericParameter(); @@ -264,6 +268,7 @@ public: void catchExceptionByValueError(const Token *tok); void memsetZeroBytesError(const Token *tok, const std::string &varname); void sizeofForArrayParameterError(const Token *tok); + void sizeofForStrncmpError(const Token *tok); void sizeofForNumericParameterError(const Token *tok); void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len); void incorrectStringBooleanError(const Token *tok, const std::string& string); @@ -273,6 +278,7 @@ public: void duplicateBranchError(const Token *tok1, const Token *tok2); 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 duplicateBreakError(const Token *tok); void assignBoolToPointerError(const Token *tok); void unsignedLessThanZeroError(const Token *tok, const std::string &varname); @@ -293,6 +299,7 @@ public: c.fflushOnInputStreamError(0, "stdin"); c.misusedScopeObjectError(NULL, "varname"); c.sizeofForArrayParameterError(0); + c.sizeofForStrncmpError(0); c.sizeofForNumericParameterError(0); c.coutCerrMisusageError(0, "cout"); @@ -326,6 +333,7 @@ public: c.duplicateBranchError(0, 0); c.duplicateExpressionError(0, 0, "&&"); c.alwaysTrueFalseStringCompareError(0, "str1", "str2"); + c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2"); c.duplicateBreakError(0); c.unsignedLessThanZeroError(0, "varname"); c.unsignedPositiveError(0, "varname"); diff --git a/test/testother.cpp b/test/testother.cpp index 8655852cb..10f8f0274 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -139,6 +139,7 @@ private: TEST_CASE(duplicateExpression2); // ticket #2730 TEST_CASE(alwaysTrueFalseStringCompare); + TEST_CASE(checkStrncmpSizeof); TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkForSuspiciousSemicolon1); @@ -3418,6 +3419,36 @@ private: " }" "}"); ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "int main()\n" + "{\n" + " if (strncmp(\"hotdog\",\"hotdog\", 6) == 0)" + " {" + " std::cout << \"Equal\n\"" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of always identical static strings.\n", errout.str()); + + check_preprocess_suppress( + "int foo(const char *buf)\n" + "{\n" + " if (strcmp(buf, buf) == 0)" + " {" + " std::cout << \"Equal\n\"" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of identical string variables.\n", errout.str()); + } + + void checkStrncmpSizeof() { + check( + "int fun(const char *buf1)\n" + "{\n" + " const char *buf1_ex = \"foobarbaz\";\n" + " return strncmp(buf1, buf1_ex, sizeof(buf1_ex)) == 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Passing sizeof(pointer) as the last argument to strncmp.\n", errout.str()); } void check_signOfUnsignedVariable(const char code[]) {