diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a5ebea7ee..cc98c1377 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -421,6 +421,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) // { diff --git a/lib/checkother.h b/lib/checkother.h index 60ec133e3..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); @@ -294,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"); diff --git a/test/testother.cpp b/test/testother.cpp index c27c2f003..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); @@ -3440,6 +3441,16 @@ private: 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[]) { // Clear the error buffer.. errout.str("");