diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 142705528..3c3a06c8a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1452,6 +1452,54 @@ void CheckOther::checkConstVariable() } } +void CheckOther::checkConstPointer() +{ + if (!mSettings->severity.isEnabled(Severity::style)) + return; + + std::set pointers; + std::set 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"); diff --git a/lib/checkother.h b/lib/checkother.h index 4c8dc9cd1..16a63d3eb 100644 --- a/lib/checkother.h +++ b/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(); diff --git a/test/testother.cpp b/test/testother.cpp index 74da029e2..0577ac986 100644 --- a/test/testother.cpp +++ b/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());