New checks (inconclusive): Suspicious calculation with sizeof()

- Check for sizeof(ptr)/something: This indicates that programmer was trying to calculate array size, but sizeof(ptr) doesn't return the length of the memory area, but size of a pointer.
- Check for sizeof()*sizeof(): This indicates that programmer misunderstood what sizeof() does: It does return the length in bytes of the given variable, not e.g. the number of elements in an array.

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()

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"

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() {