walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix #11041 FN constVariable with array of pointers [regression] (#4080) 

* Fix #11041 FN constVariable with array of pointers [regression]

* Use std::vector for deterministic order of results, use helper variables
This commit is contained in:
chrchr-github 2022-05-05 06:54:03 +02:00 committed by GitHub
parent 3b30cd7ea8
commit aebc080c0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
lib/checkother.cpp
View File

@ -1540,45 +1540,47 @@ void CheckOther::checkConstPointer()
if (!mSettings->severity.isEnabled(Severity::style)) if (!mSettings->severity.isEnabled(Severity::style))
return; return;
std::set<const Variable *> pointers; std::vector<const Variable*> pointers, nonConstPointers;
std::set<const Variable *> nonConstPointers;
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
if (!tok->variable()) const Variable* const var = tok->variable();
if (!var)
continue; continue;
if (!tok->variable()->isLocal() && !tok->variable()->isArgument()) if (!var->isLocal() && !var->isArgument())
continue; continue;
const Token* const nameTok = tok->variable()->nameToken(); const Token* const nameTok = var->nameToken();
// declarations of (static) pointers are (not) split up, array declarations are never split up // declarations of (static) pointers are (not) split up, array declarations are never split up
if (tok == nameTok && (!tok->variable()->isStatic() || Token::simpleMatch(nameTok->next(), "[")) && if (tok == nameTok && (!var->isStatic() || Token::simpleMatch(nameTok->next(), "[")) &&
// range-based for loop // range-based for loop
!(Token::simpleMatch(nameTok->astParent(), ":") && Token::simpleMatch(nameTok->astParent()->astParent(), "("))) !(Token::simpleMatch(nameTok->astParent(), ":") && Token::simpleMatch(nameTok->astParent()->astParent(), "(")))
continue; continue;
if (!tok->valueType()) const ValueType* const vt = tok->valueType();
if (!vt)
continue; continue;
if (tok->valueType()->pointer != 1 || (tok->valueType()->constness & 1) || tok->valueType()->reference != Reference::None) if (vt->pointer != 1 && !(vt->pointer == 2 && var->isArray()) || (vt->constness & 1) || vt->reference != Reference::None)
continue; continue;
if (nonConstPointers.find(tok->variable()) != nonConstPointers.end()) if (std::find(nonConstPointers.begin(), nonConstPointers.end(), var) != nonConstPointers.end())
continue; continue;
pointers.insert(tok->variable()); pointers.emplace_back(var);
const Token *parent = tok->astParent(); const Token* const parent = tok->astParent();
bool deref = false; bool deref = false;
if (parent && parent->isUnaryOp("*")) if (parent && parent->isUnaryOp("*"))
deref = true; deref = true;
else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && parent->astOperand1() != nameTok) else if (Token::simpleMatch(parent, "[") && parent->astOperand1() == tok && tok != nameTok)
deref = true; deref = true;
if (deref) { if (deref) {
if (Token::Match(parent->astParent(), "%cop%") && !parent->astParent()->isUnaryOp("&") && !parent->astParent()->isUnaryOp("*")) const Token* const gparent = parent->astParent();
if (Token::Match(gparent, "%cop%") && !gparent->isUnaryOp("&") && !gparent->isUnaryOp("*"))
continue; continue;
if (Token::simpleMatch(parent->astParent(), "return")) if (Token::simpleMatch(gparent, "return"))
continue; continue;
else if (Token::Match(parent->astParent(), "%assign%") && parent == parent->astParent()->astOperand2()) { else if (Token::Match(gparent, "%assign%") && parent == gparent->astOperand2()) {
bool takingRef = false; bool takingRef = false;
const Token *lhs = parent->astParent()->astOperand1(); const Token *lhs = gparent->astOperand1();
if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs) if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs->variable()->nameToken() == lhs)
takingRef = true; takingRef = true;
if (!takingRef) if (!takingRef)
continue; continue;
} else if (Token::simpleMatch(parent->astParent(), "[") && parent->astParent()->astOperand2() == parent) } else if (Token::simpleMatch(gparent, "[") && gparent->astOperand2() == parent)
continue; continue;
} else { } else {
if (Token::Match(parent, "%oror%|%comp%|&&|?|!|-")) if (Token::Match(parent, "%oror%|%comp%|&&|?|!|-"))
@ -1586,14 +1588,14 @@ void CheckOther::checkConstPointer()
else if (Token::simpleMatch(parent, "(") && Token::Match(parent->astOperand1(), "if|while")) else if (Token::simpleMatch(parent, "(") && Token::Match(parent->astOperand1(), "if|while"))
continue; continue;
} }
nonConstPointers.insert(tok->variable()); nonConstPointers.emplace_back(var);
} }
for (const Variable *p: pointers) { for (const Variable *p: pointers) {
if (p->isArgument()) { if (p->isArgument()) {
if (!p->scope() || !p->scope()->function || p->scope()->function->isImplicitlyVirtual(true) || p->scope()->function->hasVirtualSpecifier()) if (!p->scope() || !p->scope()->function || p->scope()->function->isImplicitlyVirtual(true) || p->scope()->function->hasVirtualSpecifier())
continue; continue;
} }
if (nonConstPointers.find(p) == nonConstPointers.end()) { if (std::find(nonConstPointers.begin(), nonConstPointers.end(), p) == nonConstPointers.end()) {
const Token *start = (p->isArgument()) ? p->scope()->bodyStart : p->nameToken()->next(); const Token *start = (p->isArgument()) ? p->scope()->bodyStart : p->nameToken()->next();
const int indirect = p->isArray() ? p->dimensions().size() : 1; const int indirect = p->isArray() ? p->dimensions().size() : 1;
if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, mSettings, mTokenizer->isCPP())) if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, mSettings, mTokenizer->isCPP()))

15
test/testother.cpp
View File

@ -3059,6 +3059,19 @@ private:
"};\n" "};\n"
"S<int*> s;\n"); "S<int*> s;\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f(int i) {\n"
" const char *tmp;\n"
" char* a[] = { \"a\", \"aa\" };\n"
" static char* b[] = { \"b\", \"bb\" };\n"
" tmp = a[i];\n"
" printf(\"%s\", tmp);\n"
" tmp = b[i];\n"
" printf(\"%s\", tmp);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'a' can be declared with const\n"
"[test.cpp:4]: (style) Variable 'b' can be declared with const\n",
errout.str());
} }
void switchRedundantAssignmentTest() { void switchRedundantAssignmentTest() {
@ -9213,7 +9226,7 @@ private:
" int local_argc = 0;\n" " int local_argc = 0;\n"
" local_argv[local_argc++] = argv[0];\n" " local_argv[local_argc++] = argv[0];\n"
"}\n", "test.c"); "}\n", "test.c");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.c:1]: (style) Parameter 'argv' can be declared with const\n", errout.str());
check("void f() {\n" check("void f() {\n"
" int x = 0;\n" " int x = 0;\n"