Fix FN constVariablePointer (#5076)

* Fix FN constVariablePointer

* Fix FP

* Add const

* Fix tests

* Add const

cli/cmdlineparser.cpp

@@ -791,26 +791,26 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                     for (; node && strcmp(node->Value(), "rule") == 0; node = node->NextSiblingElement()) {
                         Settings::Rule rule;
 
-                        tinyxml2::XMLElement *tokenlist = node->FirstChildElement("tokenlist");
+                        const tinyxml2::XMLElement *tokenlist = node->FirstChildElement("tokenlist");
                         if (tokenlist)
                             rule.tokenlist = tokenlist->GetText();
 
-                        tinyxml2::XMLElement *pattern = node->FirstChildElement("pattern");
+                        const tinyxml2::XMLElement *pattern = node->FirstChildElement("pattern");
                         if (pattern) {
                             rule.pattern = pattern->GetText();
                         }
 
                         tinyxml2::XMLElement *message = node->FirstChildElement("message");
                         if (message) {
-                            tinyxml2::XMLElement *severity = message->FirstChildElement("severity");
+                            const tinyxml2::XMLElement *severity = message->FirstChildElement("severity");
                             if (severity)
                                 rule.severity = Severity::fromString(severity->GetText());
 
-                            tinyxml2::XMLElement *id = message->FirstChildElement("id");
+                            const tinyxml2::XMLElement *id = message->FirstChildElement("id");
                             if (id)
                                 rule.id = id->GetText();
 
-                            tinyxml2::XMLElement *summary = message->FirstChildElement("summary");
+                            const tinyxml2::XMLElement *summary = message->FirstChildElement("summary");
                             if (summary)
                                 rule.summary = summary->GetText() ? summary->GetText() : "";
                         }

gui/librarydialog.cpp

@@ -275,7 +275,7 @@ void LibraryDialog::sortFunctions(bool sort)
         mUi->functions->sortItems();
     } else {
         mIgnoreChanges = true;
-        CppcheckLibraryData::Function *selfunction = currentFunction();
+        const CppcheckLibraryData::Function* selfunction = currentFunction();
         mUi->functions->clear();
         for (CppcheckLibraryData::Function &function : mData.functions) {
             mUi->functions->addItem(new FunctionListItem(mUi->functions,

lib/astutils.cpp

@@ -2632,7 +2632,7 @@ bool isVariableChanged(const Token *start, const Token *end, int indirect, const
 
 const Token* findExpression(const Token* start, const nonneg int exprid)
 {
-    Function * f = Scope::nestedInFunction(start->scope());
+    const Function* f = Scope::nestedInFunction(start->scope());
     if (!f)
         return nullptr;
     const Scope* scope = f->functionScope;

lib/checkautovariables.cpp

@@ -475,7 +475,7 @@ static bool isDanglingSubFunction(const Token* tokvalue, const Token* tok)
     const Variable* var = tokvalue->variable();
     if (!var->isLocal())
         return false;
-    Function* f = Scope::nestedInFunction(tok->scope());
+    const Function* f = Scope::nestedInFunction(tok->scope());
     if (!f)
         return false;
     const Token* parent = tokvalue->astParent();

lib/checkother.cpp

@@ -1537,6 +1537,16 @@ void CheckOther::checkConstVariable()
     }
 }
 
+static const Token* getVariableChangedStart(const Variable* p)
+{
+    if (p->isArgument())
+        return p->scope()->bodyStart;
+    const Token* start = p->nameToken()->next();
+    if (start->isSplittedVarDeclEq())
+        start = start->tokAt(3);
+    return start;
+}
+
 void CheckOther::checkConstPointer()
 {
     if (!mSettings->severity.isEnabled(Severity::style))
@@ -1619,6 +1629,8 @@ void CheckOther::checkConstPointer()
                 continue;
             else if (Token::simpleMatch(parent, "(") && Token::Match(parent->astOperand1(), "if|while"))
                 continue;
+            else if (Token::simpleMatch(parent, "=") && parent->astOperand1() == tok)
+                continue;
             else if (const Token* ftok = getTokenArgumentFunction(tok, argn)) {
                 if (ftok->function() && !parent->isCast()) {
                     const Variable* argVar = ftok->function()->getArgumentVar(argn);
@@ -1638,7 +1650,7 @@ void CheckOther::checkConstPointer()
                 continue;
         }
         if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), p) == nonConstPointers.cend()) {
-            const Token *start = (p->isArgument()) ? p->scope()->bodyStart : p->nameToken()->next();
+            const Token *start = getVariableChangedStart(p);
             const int indirect = p->isArray() ? p->dimensions().size() : 1;
             if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, mSettings, mTokenizer->isCPP()))
                 continue;

lib/checkunusedvar.cpp

@@ -293,7 +293,7 @@ void Variables::read(nonneg int varid, const Token* tok)
 
 void Variables::readAliases(nonneg int varid, const Token* tok)
 {
-    VariableUsage *usage = find(varid);
+    const VariableUsage *usage = find(varid);
 
     if (usage) {
         for (nonneg int const aliases : usage->_aliases) {

lib/clangimport.cpp

@@ -1149,10 +1149,10 @@ Token *clangimport::AstNode::createTokens(TokenList *tokenList)
     if (nodeType == NamespaceDecl) {
         if (children.empty())
             return nullptr;
-        Token *defToken = addtoken(tokenList, "namespace");
+        const Token *defToken = addtoken(tokenList, "namespace");
         const std::string &s = mExtTokens[mExtTokens.size() - 2];
-        Token *nameToken = (s.compare(0,4,"col:")==0 || s.compare(0,5,"line:")==0) ?
+        const Token* nameToken = (s.compare(0, 4, "col:") == 0 || s.compare(0, 5, "line:") == 0) ?
-                           addtoken(tokenList, mExtTokens.back()) : nullptr;
+                                 addtoken(tokenList, mExtTokens.back()) : nullptr;
         Scope *scope = createScope(tokenList, Scope::ScopeType::eNamespace, children, defToken);
         if (nameToken)
             scope->className = nameToken->str();

lib/reverseanalyzer.cpp

@@ -162,7 +162,7 @@ struct ReverseTraversal {
                 parent = parent->astParent();
             if (!Token::Match(parent, "%oror%|&&|?"))
                 continue;
-            Token* condTok = parent->astOperand1();
+            const Token* condTok = parent->astOperand1();
             if (!condTok)
                 continue;
             bool checkThen, checkElse;

lib/templatesimplifier.cpp

@@ -1320,7 +1320,7 @@ void TemplateSimplifier::simplifyTemplateAliases()
 
             // copy template-id from declaration to after instantiation
             Token * dst = aliasUsage.token()->next()->findClosingBracket();
-            Token * end = TokenList::copyTokens(dst, aliasDeclaration.aliasStartToken(), aliasDeclaration.aliasEndToken()->previous(), false)->next();
+            const Token* end = TokenList::copyTokens(dst, aliasDeclaration.aliasStartToken(), aliasDeclaration.aliasEndToken()->previous(), false)->next();
 
             // replace parameters
             for (Token *tok1 = dst->next(); tok1 != end; tok1 = tok1->next()) {
@@ -3740,7 +3740,7 @@ void TemplateSimplifier::simplifyTemplates(
         for (Token *tok = mTokenList.front(); tok; tok = tok->next()) {
             if (!Token::Match(tok, ")|>|>> requires %name%|("))
                 continue;
-            Token *end = skipRequires(tok->next());
+            const Token* end = skipRequires(tok->next());
             if (end)
                 Token::eraseTokens(tok, end);
         }

lib/tokenize.cpp

@@ -892,7 +892,7 @@ namespace {
             if (!after)
                 throw InternalError(tok, "Failed to simplify typedef. Is the code valid?");
 
-            Token* const tok4 = useAfterVarRange ? insertTokens(after->previous(), mRangeAfterVar)->next() : tok3->next();
+            const Token* const tok4 = useAfterVarRange ? insertTokens(after->previous(), mRangeAfterVar)->next() : tok3->next();
 
             tok->deleteThis();
 
@@ -4287,7 +4287,7 @@ void Tokenizer::setVarIdClassDeclaration(Token* const startToken,
                                          std::map<nonneg int, std::map<std::string, nonneg int>>& structMembers)
 {
     // end of scope
-    Token* const endToken = startToken->link();
+    const Token* const endToken = startToken->link();
 
     // determine class name
     std::string className;

lib/valueflow.cpp

@@ -6276,7 +6276,7 @@ struct ConditionHandler {
                 continue;
             }
             if (Token::Match(tok->astParent(), "==|!=")) {
-                Token* sibling = tok->astSibling();
+                const Token* sibling = tok->astSibling();
                 if (sibling->hasKnownIntValue() && (astIsBool(tok) || astIsBool(sibling))) {
                     const bool value = sibling->values().front().intvalue;
                     if (inverted)
@@ -7354,8 +7354,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist,
     });
     if (!r) {
         std::string fname = "<unknown>";
-        Function* f = functionScope->function;
+        if (const Function* f = functionScope->function)
-        if (f)
             fname = f->name();
         if (settings.debugwarnings)
             bailout(tokenlist, errorLogger, functionScope->bodyStart, "Too many argument passed to " + fname);

test/cfg/bsd.c

@@ -77,6 +77,7 @@ void uninitvar_timercmp(struct timeval t)
 
 void nullPointer_timercmp(struct timeval t)
 {
+    // cppcheck-suppress constVariablePointer
     struct timeval *p=0;
     // cppcheck-suppress nullPointer
     (void)timercmp(&t, p, <);

test/cfg/gnu.c

@@ -205,6 +205,7 @@ void uninitvar_timercmp(struct timeval t)
 
 void nullPointer_timercmp(struct timeval t)
 {
+    // cppcheck-suppress constVariablePointer
     struct timeval *p=0;
     // cppcheck-suppress nullPointer
     (void)timercmp(&t, p, <);
@@ -417,11 +418,11 @@ void bufferAccessOutOfBounds()
 
 void leakReturnValNotUsed()
 {
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     char* ptr = (char*)strdupa("test");
     // cppcheck-suppress ignoredReturnValue
     strdupa("test");
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     char* ptr2 = (char*)strndupa("test", 1);
     // cppcheck-suppress ignoredReturnValue
     strndupa("test", 1);

test/cfg/posix.c

@@ -985,8 +985,7 @@ void memleak_getaddrinfo()
 
 void memleak_mmap(int fd)
 {
-    // cppcheck-suppress unusedAllocatedMemory
+    // cppcheck-suppress [unusedAllocatedMemory, unreadVariable, constVariablePointer]
-    // cppcheck-suppress unreadVariable
     void *addr = mmap(NULL, 255, PROT_NONE, MAP_PRIVATE, fd, 0);
     // cppcheck-suppress memleak
 }
@@ -1040,7 +1039,7 @@ int munmap_no_double_free(int tofd, // #11396
 
 void resourceLeak_fdopen(int fd)
 {
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     FILE *f = fdopen(fd, "r");
     // cppcheck-suppress resourceLeak
 }
@@ -1065,14 +1064,14 @@ int no_resourceLeak_mkstemp_02(char *template)
 
 void resourceLeak_fdopendir(int fd)
 {
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     DIR* leak1 = fdopendir(fd);
     // cppcheck-suppress resourceLeak
 }
 
 void resourceLeak_opendir(void)
 {
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     DIR* leak1 = opendir("abc");
     // cppcheck-suppress resourceLeak
 }
@@ -1187,9 +1186,7 @@ void uninitvar(int fd)
     regcomp(&reg, pattern, cflags2);
     regerror(0, &reg, 0, 0);
 #ifndef __CYGWIN__
-    // cppcheck-suppress uninitvar
+    // cppcheck-suppress [uninitvar, unreadVariable, ecvtCalled, constVariablePointer]
-    // cppcheck-suppress unreadVariable
-    // cppcheck-suppress ecvtCalled
     char *buffer = ecvt(d, 11, &decimal, &sign);
 #endif
     // cppcheck-suppress gcvtCalled
@@ -1282,8 +1279,7 @@ void dl(const char* libname, const char* func)
     // cppcheck-suppress resourceLeak
     lib = dlopen(libname, RTLD_LAZY);
     const char* funcname;
-    // cppcheck-suppress uninitvar
+    // cppcheck-suppress [uninitvar, unreadVariable, constVariablePointer]
-    // cppcheck-suppress unreadVariable
     void* sym = dlsym(lib, funcname);
     // cppcheck-suppress ignoredReturnValue
     dlsym(lib, "foo");

test/cfg/std.c

@@ -264,8 +264,7 @@ char* nullPointer_fgets(char *buffer, int n, FILE *stream)
 
 void memleak_aligned_alloc(void)
 {
-    // cppcheck-suppress unusedAllocatedMemory
+    // cppcheck-suppress [unusedAllocatedMemory, unreadVariable, constVariablePointer]
-    // cppcheck-suppress unreadVariable
     char * alignedBuf = aligned_alloc(8, 16);
     // cppcheck-suppress memleak
 }
@@ -358,7 +357,7 @@ void arrayIndexOutOfBounds()
 
 void resourceLeak_tmpfile(void)
 {
-    // cppcheck-suppress unreadVariable
+    // cppcheck-suppress [unreadVariable, constVariablePointer]
     FILE * fp = tmpfile();
     // cppcheck-suppress resourceLeak
 }
@@ -4184,6 +4183,7 @@ void uninitvar_tolower(int character)
 
     // cppcheck-suppress unassignedVariable
     int c2;
+    // cppcheck-suppress constVariablePointer
     int *pc=&c2;
     // cppcheck-suppress uninitvar
     (void)tolower(*pc);
@@ -4191,6 +4191,7 @@ void uninitvar_tolower(int character)
     // No warning is expected
     (void)tolower(character);
 
+    // cppcheck-suppress constVariablePointer
     int *pChar = &character;
     // No warning is expected
     (void)tolower(*pChar);
@@ -4204,6 +4205,7 @@ void uninitvar_toupper(int character)
 
     // cppcheck-suppress unassignedVariable
     int c2;
+    // cppcheck-suppress constVariablePointer
     int *pc=&c2;
     // cppcheck-suppress uninitvar
     (void)toupper(*pc);
@@ -4211,6 +4213,7 @@ void uninitvar_toupper(int character)
     // No warning is expected
     (void)toupper(character);
 
+    // cppcheck-suppress constVariablePointer
     int *pChar = &character;
     // No warning is expected
     (void)toupper(*pChar);

test/cfg/windows.cpp

@@ -525,8 +525,7 @@ void nullPointer()
 
 void memleak_malloca()
 {
-    // cppcheck-suppress unusedAllocatedMemory
+    // cppcheck-suppress [unusedAllocatedMemory, unreadVariable, constVariablePointer]
-    // cppcheck-suppress unreadVariable
     void *pMem = _malloca(10);
     // cppcheck-suppress memleak
 }

test/testother.cpp

@@ -3632,6 +3632,11 @@ private:
                       "[test.cpp:13]: (style) Parameter 's' can be declared as pointer to const\n"
                       "[test.cpp:19]: (style) Parameter 's' can be declared as pointer to const\n",
                       errout.str());
+
+        check("void f(int*& p, int* q) {\n"
+              "    p = q;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void switchRedundantAssignmentTest() {
@@ -7440,7 +7445,7 @@ private:
               "  if (*p < 0) continue;\n"
               "  if ((*p > 0)) {}\n"
               "}\n");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'p' can be declared as pointer to const\n", errout.str());
 
         check("void f() {\n"
               "  int val = 0;\n"
@@ -7449,7 +7454,9 @@ private:
               "  if ((*p > 0)) {}\n"
               "}\n");
         TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The comparison '*p < 0' is always false.\n"
-                           "[test.cpp:2] -> [test.cpp:4]: (style) The comparison '*p > 0' is always false.\n", "", errout.str());
+                           "[test.cpp:2] -> [test.cpp:4]: (style) The comparison '*p > 0' is always false.\n",
+                           "[test.cpp:3]: (style) Variable 'p' can be declared as pointer to const\n",
+                           errout.str());
 
         check("void f() {\n"
               "  int val = 0;\n"
@@ -9747,7 +9754,7 @@ private:
         check("void f(char **ptr) {\n"
               "    int *x = &(*ptr)[10];\n"
               "}\n", nullptr, true);
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'x' can be declared as pointer to const\n", errout.str());
 
         // function calls
         check("void f(Mutex *mut) {\n"
@@ -10887,7 +10894,9 @@ private:
               "    return xp > yp;\n"
               "}");
         ASSERT_EQUALS(
-            "[test.cpp:2] -> [test.cpp:4] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:6]: (error) Comparing pointers that point to different objects\n",
+            "[test.cpp:2] -> [test.cpp:4] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:6]: (error) Comparing pointers that point to different objects\n"
+            "[test.cpp:4]: (style) Variable 'xp' can be declared as pointer to const\n"
+            "[test.cpp:5]: (style) Variable 'yp' can be declared as pointer to const\n",
             errout.str());
 
         check("bool f() {\n"
@@ -10908,7 +10917,9 @@ private:
               "    return xp > yp;\n"
               "}");
         ASSERT_EQUALS(
-            "[test.cpp:1] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n",
+            "[test.cpp:1] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n"
+            "[test.cpp:5]: (style) Variable 'xp' can be declared as pointer to const\n"
+            "[test.cpp:6]: (style) Variable 'yp' can be declared as pointer to const\n",
             errout.str());
 
         check("struct A {int data;};\n"
@@ -10920,7 +10931,9 @@ private:
               "    return xp > yp;\n"
               "}");
         ASSERT_EQUALS(
-            "[test.cpp:2] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n",
+            "[test.cpp:2] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n"
+            "[test.cpp:5]: (style) Variable 'xp' can be declared as pointer to const\n"
+            "[test.cpp:6]: (style) Variable 'yp' can be declared as pointer to const\n",
             errout.str());
 
         check("bool f(int * xp, int* yp) {\n"
@@ -10945,7 +10958,9 @@ private:
               "    int* yp = &x[1];\n"
               "    return xp > yp;\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'xp' can be declared as pointer to const\n"
+                      "[test.cpp:4]: (style) Variable 'yp' can be declared as pointer to const\n",
+                      errout.str());
 
         check("bool f(const int * xp, const int* yp) {\n"
               "    return xp > yp;\n"
@@ -10975,7 +10990,9 @@ private:
               "    int* yp = &y->data;\n"
               "    return xp > yp;\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (style) Variable 'xp' can be declared as pointer to const\n"
+                      "[test.cpp:6]: (style) Variable 'yp' can be declared as pointer to const\n",
+                      errout.str());
 
         check("struct S { int i; };\n" // #11576
               "int f(S s) {\n"

tools/triage/mainwindow.cpp

@@ -412,7 +412,7 @@ void MainWindow::resultsContextMenu(const QPoint& pos)
         return;
     QMenu submenu;
     submenu.addAction("Copy");
-    QAction* menuItem = submenu.exec(ui->results->mapToGlobal(pos));
+    const QAction* menuItem = submenu.exec(ui->results->mapToGlobal(pos));
     if (menuItem && menuItem->text().contains("Copy"))
     {
         QString text;