const variables/parameters; Improved check to handle pointers also (misra 8.13)

lib/checkother.cpp

@@ -1452,6 +1452,54 @@ void CheckOther::checkConstVariable()
     }
 }
 
+void CheckOther::checkConstPointer()
+{
+    if (!mSettings->severity.isEnabled(Severity::style))
+        return;
+
+    std::set<const Variable *> pointers;
+    std::set<const Variable *> nonConstPointers;
+    for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
+        if (!tok->variable())
+            continue;
+        if (tok == tok->variable()->nameToken())
+            continue;
+        if (!tok->valueType())
+            continue;
+        if (tok->valueType()->pointer == 0 || tok->valueType()->constness > 0)
+            continue;
+        if (nonConstPointers.find(tok->variable()) != nonConstPointers.end())
+            continue;
+        pointers.insert(tok->variable());
+        bool nonConst = true;
+        const Token *parent = tok->astParent();
+        bool deref = false;
+        if (parent && parent->isUnaryOp("*"))
+            deref = true;
+        else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok)
+            deref = true;
+        if (deref) {
+            while (Token::Match(parent->astParent(), "%cop%") && parent->astParent()->isBinaryOp())
+                parent = parent->astParent();
+            if (Token::simpleMatch(parent->astParent(), "return"))
+                nonConst = false;
+            else if (Token::Match(parent->astParent(), "%assign%") && parent == parent->astParent()->astOperand2()) {
+                bool takingRef = false;
+                const Token *lhs = parent->astParent()->astOperand1();
+                if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs)
+                    takingRef = true;
+                nonConst = takingRef;
+            } else if (Token::simpleMatch(parent->astParent(), "[") && parent->astParent()->astOperand2() == parent)
+                nonConst = false;
+        }
+        if (nonConst)
+            nonConstPointers.insert(tok->variable());
+    }
+    for (const Variable *p: pointers) {
+        if (nonConstPointers.find(p) == nonConstPointers.end())
+            constVariableError(p, nullptr);
+    }
+}
 void CheckOther::constVariableError(const Variable *var, const Function *function)
 {
     const std::string vartype((var && var->isArgument()) ? "Parameter" : "Variable");

lib/checkother.h

@@ -95,6 +95,7 @@ public:
         checkOther.clarifyCalculation();
         checkOther.checkPassByReference();
         checkOther.checkConstVariable();
+        checkOther.checkConstPointer();
         checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
         checkOther.checkInvalidFree();
         checkOther.clarifyStatement();
@@ -135,6 +136,7 @@ public:
     void checkPassByReference();
 
     void checkConstVariable();
+    void checkConstPointer();
 
     /** @brief Using char variable as array index / as operand in bit operation */
     void checkCharVariable();

test/testother.cpp

@@ -99,6 +99,7 @@ private:
 
         TEST_CASE(constVariable);
         TEST_CASE(constParameterCallback);
+        TEST_CASE(constPointer);
 
         TEST_CASE(switchRedundantAssignmentTest);
         TEST_CASE(switchRedundantOperationTest);
@@ -2631,6 +2632,29 @@ private:
         ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:13]: (style) Parameter 'signal' can be declared with const. However it seems that 'signalEvent' is a callback function, if 'signal' is declared with const you might also need to cast function pointer(s).\n", errout.str());
     }
 
+    void constPointer() {
+        check("void foo(int *p) { return *p; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
+
+        check("void foo(int *p) { x = *p; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
+
+        check("void foo(int *p) { int &ref = *p; ref = 12; }");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(int *p) { x = *p + 10; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
+
+        check("void foo(int *p) { return p[10]; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
+
+        check("void foo(int *p) { int &ref = p[0]; ref = 12; }");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(int *p) { x[*p] = 12; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'p' can be declared with const\n", errout.str());
+    }
+
     void switchRedundantAssignmentTest() {
         check("void foo()\n"
               "{\n"
@@ -5667,7 +5691,7 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
-        check("void test(int * p, int * q) {\n"
+        check("void test(const int * p, const int * q) {\n"
               "    int i = *p;\n"
               "    int j = *p;\n"
               "}");
@@ -5795,7 +5819,7 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
 
-        check("void test(int * p) {\n"
+        check("void test(const int * p) {\n"
               "    int i = *p;\n"
               "    int j = *p;\n"
               "}");
@@ -8160,7 +8184,7 @@ private:
         ASSERT_EQUALS("[test.cpp:2]: (style) Redundant pointer operation on 'y' - it's already a pointer.\n", errout.str());
 
         // no warning for bitwise AND
-        check("void f(int *b) {\n"
+        check("void f(const int *b) {\n"
               "    int x = 0x20 & *b;\n"
               "}\n", nullptr, false, true);
         ASSERT_EQUALS("", errout.str());