Skip to content

Commit e9a0d79

Browse files
chrchr-githubchrchr-github
authored
Fix #11014 FN redundantPointerOp / remove simplifyMulAndParens() (#4062)
1 parent e0ccfea commit e9a0d79

6 files changed

Lines changed: 65 additions & 114 deletions

File tree

‎lib/checkleakautovar.cpp‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,11 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
331331
if (Token::Match(tok, "const %type%"))
332332
tok = tok->tokAt(2);
333333

334+
while (tok->str() == "(")
335+
tok = tok->next();
336+
while (tok->isUnaryOp("*") && tok->astOperand1()->isUnaryOp("&"))
337+
tok = tok->astOperand1()->astOperand1();
338+
334339
// parse statement, skip to last member
335340
const Token *varTok = tok;
336341
while (Token::Match(varTok, "%name% ::|. %name% !!("))
@@ -342,9 +347,22 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
342347
while (Token::Match(ftok, "%name% :: %name%"))
343348
ftok = ftok->tokAt(2);
344349

350+
auto isAssignment = [](const Token* varTok) -> const Token* {
351+
if (varTok->varId()) {
352+
const Token* top = varTok;
353+
while (top->astParent()) {
354+
top = top->astParent();
355+
if (!Token::Match(top, "(|*|&|."))
356+
break;
357+
}
358+
if (top->str() == "=" && succeeds(top, varTok))
359+
return top;
360+
}
361+
return nullptr;
362+
};
363+
345364
// assignment..
346-
if (Token::Match(varTok, "%var% =")) {
347-
const Token* const tokAssignOp = varTok->next();
365+
if (const Token* const tokAssignOp = isAssignment(varTok)) {
348366

349367
// taking address of another variable..
350368
if (Token::Match(tokAssignOp, "= %var% [+;]")) {

‎lib/tokenize.cpp‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,69 +2709,6 @@ bool Tokenizer::simplifyUsing()
27092709

27102710
return substitute;
27112711
}
2712-
void Tokenizer::simplifyMulAndParens()
2713-
{
2714-
if (!list.front())
2715-
return;
2716-
for (Token *tok = list.front()->tokAt(3); tok; tok = tok->next()) {
2717-
if (!tok->isName())
2718-
continue;
2719-
//fix ticket #2784 - improved by ticket #3184
2720-
int closedPars = 0;
2721-
Token *tokend = tok->next();
2722-
Token *tokbegin = tok->previous();
2723-
while (tokend && tokend->str() == ")") {
2724-
++closedPars;
2725-
tokend = tokend->next();
2726-
}
2727-
if (!tokend || !(tokend->isAssignmentOp()))
2728-
continue;
2729-
while (Token::Match(tokbegin, "&|(")) {
2730-
if (tokbegin->str() == "&") {
2731-
if (Token::Match(tokbegin->tokAt(-2), "[;{}&(] *")) {
2732-
//remove '* &'
2733-
tokbegin = tokbegin->tokAt(-2);
2734-
tokbegin->deleteNext(2);
2735-
} else if (Token::Match(tokbegin->tokAt(-3), "[;{}&(] * (")) {
2736-
if (closedPars == 0)
2737-
break;
2738-
--closedPars;
2739-
//remove ')'
2740-
tok->deleteNext();
2741-
//remove '* ( &'
2742-
tokbegin = tokbegin->tokAt(-3);
2743-
tokbegin->deleteNext(3);
2744-
} else
2745-
break;
2746-
} else if (tokbegin->str() == "(") {
2747-
if (closedPars == 0)
2748-
break;
2749-
2750-
//find consecutive opening parentheses
2751-
int openPars = 0;
2752-
while (tokbegin && tokbegin->str() == "(" && openPars <= closedPars) {
2753-
++openPars;
2754-
tokbegin = tokbegin->previous();
2755-
}
2756-
if (!tokbegin || openPars > closedPars)
2757-
break;
2758-
2759-
if ((openPars == closedPars && Token::Match(tokbegin, "[;{}]")) ||
2760-
Token::Match(tokbegin->tokAt(-2), "[;{}&(] * &") ||
2761-
Token::Match(tokbegin->tokAt(-3), "[;{}&(] * ( &")) {
2762-
//remove the excessive parentheses around the variable
2763-
while (openPars > 0) {
2764-
tok->deleteNext();
2765-
tokbegin->deleteNext();
2766-
--closedPars;
2767-
--openPars;
2768-
}
2769-
} else
2770-
break;
2771-
}
2772-
}
2773-
}
2774-
}
27752712

27762713
bool Tokenizer::createTokens(std::istream &code,
27772714
const std::string& FileName)
@@ -5039,9 +4976,6 @@ bool Tokenizer::simplifyTokenList1(const char FileName[])
50394976
// simplify labels and 'case|default'-like syntaxes
50404977
simplifyLabelsCaseDefault();
50414978

5042-
// simplify '[;{}] * & ( %any% ) =' to '%any% ='
5043-
simplifyMulAndParens();
5044-
50454979
if (!isC() && !mSettings->library.markupFile(FileName)) {
50464980
findComplicatedSyntaxErrorsInTemplates();
50474981
}

‎lib/tokenize.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,6 @@ class CPPCHECKLIB Tokenizer {
202202
*/
203203
static void eraseDeadCode(Token *begin, const Token *end);
204204

205-
/**
206-
* Simplify '* & ( %name% ) =' or any combination of '* &' and '()'
207-
* parentheses around '%name%' to '%name% ='
208-
*/
209-
void simplifyMulAndParens();
210-
211205
/**
212206
* Calculates sizeof value for given type.
213207
* @param type Token which will contain e.g. "int", "*", or string.

‎test/testleakautovar.cpp‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ class TestLeakAutoVar : public TestFixture {
9292
TEST_CASE(assign20); // #9187
9393
TEST_CASE(assign21); // #10186
9494
TEST_CASE(assign22); // #9139
95+
TEST_CASE(assign23);
9596

9697
TEST_CASE(isAutoDealloc);
9798

@@ -463,6 +464,44 @@ class TestLeakAutoVar : public TestFixture {
463464
ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: p\n", errout.str());
464465
}
465466

467+
void assign23() {
468+
Settings s = settings;
469+
LOAD_LIB_2(settings.library, "posix.cfg");
470+
check("void f() {\n"
471+
" int n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14;\n"
472+
" *&n1 = open(\"xx.log\", O_RDONLY);\n"
473+
" *&(n2) = open(\"xx.log\", O_RDONLY);\n"
474+
" *(&n3) = open(\"xx.log\", O_RDONLY);\n"
475+
" *&*&n4 = open(\"xx.log\", O_RDONLY);\n"
476+
" *&*&*&(n5) = open(\"xx.log\", O_RDONLY);\n"
477+
" *&*&(*&n6) = open(\"xx.log\", O_RDONLY);\n"
478+
" *&*(&*&n7) = open(\"xx.log\", O_RDONLY);\n"
479+
" *(&*&n8) = open(\"xx.log\", O_RDONLY);\n"
480+
" *&(*&*&(*&n9)) = open(\"xx.log\", O_RDONLY);\n"
481+
" (n10) = open(\"xx.log\", O_RDONLY);\n"
482+
" ((n11)) = open(\"xx.log\", O_RDONLY);\n"
483+
" ((*&n12)) = open(\"xx.log\", O_RDONLY);\n"
484+
" *(&(*&n13)) = open(\"xx.log\", O_RDONLY);\n"
485+
" ((*&(*&n14))) = open(\"xx.log\", O_RDONLY);\n"
486+
"}\n", true);
487+
ASSERT_EQUALS("[test.cpp:17]: (error) Resource leak: n1\n"
488+
"[test.cpp:17]: (error) Resource leak: n2\n"
489+
"[test.cpp:17]: (error) Resource leak: n3\n"
490+
"[test.cpp:17]: (error) Resource leak: n4\n"
491+
"[test.cpp:17]: (error) Resource leak: n5\n"
492+
"[test.cpp:17]: (error) Resource leak: n6\n"
493+
"[test.cpp:17]: (error) Resource leak: n7\n"
494+
"[test.cpp:17]: (error) Resource leak: n8\n"
495+
"[test.cpp:17]: (error) Resource leak: n9\n"
496+
"[test.cpp:17]: (error) Resource leak: n10\n"
497+
"[test.cpp:17]: (error) Resource leak: n11\n"
498+
"[test.cpp:17]: (error) Resource leak: n12\n"
499+
"[test.cpp:17]: (error) Resource leak: n13\n"
500+
"[test.cpp:17]: (error) Resource leak: n14\n",
501+
errout.str());
502+
settings = s;
503+
}
504+
466505
void isAutoDealloc() {
467506
check("void f() {\n"
468507
" char *p = new char[100];"

‎test/testother.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8774,6 +8774,12 @@ class TestOther : public TestFixture {
87748774
"[test.cpp:4]: (style) Redundant pointer operation on 'value' - it's already a variable.\n",
87758775
errout.str());
87768776

8777+
check("void f(int& a, int b) {\n"
8778+
" *(&a) = b;\n"
8779+
"}\n", nullptr, false, true);
8780+
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant pointer operation on 'a' - it's already a variable.\n",
8781+
errout.str());
8782+
87778783
check("void f(int**& p) {}\n", nullptr, false, true);
87788784
ASSERT_EQUALS("", errout.str());
87798785

‎test/testtokenize.cpp‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@ class TestTokenizer : public TestFixture {
187187

188188
TEST_CASE(tokenize_double);
189189
TEST_CASE(tokenize_strings);
190-
TEST_CASE(simplifyMulAndParens); // Ticket #2784 + #3184
191190

192191
TEST_CASE(simplifyStructDecl);
193192

@@ -1949,45 +1948,6 @@ class TestTokenizer : public TestFixture {
19491948
"}", tokenizeAndStringify(code));
19501949
}
19511950

1952-
void simplifyMulAndParens() {
1953-
// (error) Resource leak
1954-
const char code[] = "void f() {"
1955-
" *&n1=open();"
1956-
" *&(n2)=open();"
1957-
" *(&n3)=open();"
1958-
" *&*&n4=open();"
1959-
" *&*&*&(n5)=open();"
1960-
" *&*&(*&n6)=open();"
1961-
" *&*(&*&n7)=open();"
1962-
" *(&*&n8)=open();"
1963-
" *&(*&*&(*&n9))=open();"
1964-
" (n10) = open();"
1965-
" ((n11)) = open();"
1966-
" ((*&n12))=open();"
1967-
" *(&(*&n13))=open();"
1968-
" ((*&(*&n14)))=open();"
1969-
" ((*&(*&n15)))+=10;"
1970-
"}";
1971-
const char expected[] = "void f ( ) {"
1972-
" n1 = open ( ) ;"
1973-
" n2 = open ( ) ;"
1974-
" n3 = open ( ) ;"
1975-
" n4 = open ( ) ;"
1976-
" n5 = open ( ) ;"
1977-
" n6 = open ( ) ;"
1978-
" n7 = open ( ) ;"
1979-
" n8 = open ( ) ;"
1980-
" n9 = open ( ) ;"
1981-
" n10 = open ( ) ;"
1982-
" n11 = open ( ) ;"
1983-
" n12 = open ( ) ;"
1984-
" n13 = open ( ) ;"
1985-
" n14 = open ( ) ;"
1986-
" n15 += 10 ; "
1987-
"}";
1988-
ASSERT_EQUALS(expected, tokenizeAndStringify(code));
1989-
}
1990-
19911951
void simplifyStructDecl() {
19921952
const char code[] = "const struct A { int a; int b; } a;";
19931953
ASSERT_EQUALS("struct A { int a ; int b ; } ; const struct A a ;", tokenizeAndStringify(code));

0 commit comments

Comments
 (0)