diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 96d1d1719..e40660f8e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3208,6 +3208,43 @@ void CheckOther::sizeofCalculationError(const Token *tok, bool inconclusive) "sizeofCalculation", "Found calculation inside sizeof().", inconclusive); } +//----------------------------------------------------------------------------- +// Check for code like sizeof()*sizeof() or sizeof(ptr)/value +//----------------------------------------------------------------------------- +void CheckOther::suspiciousSizeofCalculation() +{ + if (!_settings->isEnabled("style") || !_settings->inconclusive) + return; + + const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::simpleMatch(tok, "sizeof (")) { + const Token* const end = tok->linkAt(1); + const Variable* var = symbolDatabase->getVariableFromVarId(end->previous()->varId()); + if (end->strAt(-1) == "*" || (var && var->isPointer() && !var->isArray())) { + if (end->strAt(1) == "/") + divideSizeofError(tok); + } else if (Token::simpleMatch(end, ") * sizeof")) + multiplySizeofError(tok); + } + } +} + +void CheckOther::multiplySizeofError(const Token *tok) +{ + reportError(tok, Severity::warning, + "multiplySizeof", "Multiplying sizeof() with sizeof() indicates a logic error.", true); +} + +void CheckOther::divideSizeofError(const Token *tok) +{ + reportError(tok, Severity::warning, + "divideSizeof", "Division of result of sizeof() on pointer type.\n" + "Division of result of sizeof() on pointer type. sizeof() returns the size of the pointer, " + "not the size of the memory area it points to.", true); +} + //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- void CheckOther::checkAssignBoolToPointer() diff --git a/lib/checkother.h b/lib/checkother.h index 575d43c2d..251a8d09c 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -59,6 +59,7 @@ public: checkOther.strPlusChar(); checkOther.sizeofsizeof(); checkOther.sizeofCalculation(); + checkOther.suspiciousSizeofCalculation(); checkOther.checkRedundantAssignment(); checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkAssignmentInAssert(); @@ -176,6 +177,9 @@ public: /** @brief %Check for calculations inside sizeof */ void sizeofCalculation(); + /** @brief %Check for suspicious calculations with sizeof results */ + void suspiciousSizeofCalculation(); + /** @brief copying to memory or assigning to a variablen twice */ void checkRedundantAssignment(); @@ -285,6 +289,8 @@ private: void clarifyStatementError(const Token* tok); void sizeofsizeofError(const Token *tok); void sizeofCalculationError(const Token *tok, bool inconclusive); + void multiplySizeofError(const Token *tok); + void divideSizeofError(const Token *tok); void cstyleCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive); void dangerousUsageStrtolError(const Token *tok, const std::string& funcname); @@ -381,6 +387,8 @@ private: c.strPlusCharError(0); c.sizeofsizeofError(0); c.sizeofCalculationError(0, false); + c.multiplySizeofError(0); + c.divideSizeofError(0); c.redundantAssignmentInSwitchError(0, 0, "var"); c.redundantCopyInSwitchError(0, 0, "var"); c.switchCaseFallThrough(0); @@ -459,6 +467,7 @@ private: "* redundant strcpy in a switch statement\n" "* look for 'sizeof sizeof ..'\n" "* look for calculations inside sizeof()\n" + "* look for suspicious calculations with sizeof()\n" "* assignment of a variable to itself\n" "* mutual exclusion over || always evaluating to true\n" "* Clarify calculation with parentheses\n" diff --git a/test/testother.cpp b/test/testother.cpp index acd5c8a11..ec96bb819 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -119,6 +119,8 @@ private: TEST_CASE(sizeofForArrayParameter); TEST_CASE(sizeofForNumericParameter); + TEST_CASE(suspiciousSizeofCalculation); + TEST_CASE(clarifyCalculation); TEST_CASE(clarifyStatement); @@ -4209,8 +4211,37 @@ private: NULL, true ); ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); + } + + void suspiciousSizeofCalculation() { + check("int* p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n", errout.str()); + + check("unknown p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("return sizeof(unknown)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("int p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("int* p[5];\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + check("return sizeof(foo)*sizeof(bar);"); + ASSERT_EQUALS("[test.cpp:1]: (warning, inconclusive) Multiplying sizeof() with sizeof() indicates a logic error.\n", errout.str()); + + check("return (foo)*sizeof(bar);"); + ASSERT_EQUALS("", errout.str()); + + check("return sizeof(foo)*bar;"); + ASSERT_EQUALS("", errout.str()); } void clarifyCalculation() {