From dd6c1ce4b0413738e243ec6ba934051df8d3cbc4 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 27 Apr 2022 17:27:02 +0200 Subject: [PATCH 1/3] Fix #10991 FN: Redundant pointer operation --- lib/checkother.cpp | 19 ++++++++++++------- lib/checkother.h | 4 ++-- test/testother.cpp | 10 ++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 487a18e5688..16c74f5e244 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2941,7 +2941,12 @@ void CheckOther::checkRedundantPointerOp() if (tok->isExpandedMacro() && tok->str() == "(") tok = tok->link(); - if (!tok->isUnaryOp("&") || !tok->astOperand1()->isUnaryOp("*")) + bool addressOfDeref{}; + if (tok->isUnaryOp("&") && tok->astOperand1()->isUnaryOp("*")) + addressOfDeref = true; + else if (tok->isUnaryOp("*") && tok->astOperand1()->isUnaryOp("&")) + addressOfDeref = false; + else continue; // variable @@ -2950,18 +2955,18 @@ void CheckOther::checkRedundantPointerOp() continue; const Variable *var = varTok->variable(); - if (!var || !var->isPointer()) + if (!var || (addressOfDeref && !var->isPointer())) continue; - redundantPointerOpError(tok, var->name(), false); + redundantPointerOpError(tok, var->name(), false, addressOfDeref); } } -void CheckOther::redundantPointerOpError(const Token* tok, const std::string &varname, bool inconclusive) +void CheckOther::redundantPointerOpError(const Token* tok, const std::string &varname, bool inconclusive, bool addressOfDeref) { - reportError(tok, Severity::style, "redundantPointerOp", - "$symbol:" + varname + "\n" - "Redundant pointer operation on '$symbol' - it's already a pointer.", CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); + std::string msg = "$symbol:" + varname + "\nRedundant pointer operation on '$symbol' - it's already a "; + msg += addressOfDeref ? "pointer." : "variable."; + reportError(tok, Severity::style, "redundantPointerOp", msg, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); } void CheckOther::checkInterlockedDecrement() diff --git a/lib/checkother.h b/lib/checkother.h index 86da1db089a..8f8403526c8 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -275,7 +275,7 @@ class CPPCHECKLIB CheckOther : public Check { void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean); void varFuncNullUBError(const Token *tok); void commaSeparatedReturnError(const Token *tok); - void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive); + void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive, bool addressOfDeref); void raceAfterInterlockedDecrementError(const Token* tok); void unusedLabelError(const Token* tok, bool inSwitch, bool hasIfdef); void unknownEvaluationOrder(const Token* tok); @@ -343,7 +343,7 @@ class CPPCHECKLIB CheckOther : public Check { c.varFuncNullUBError(nullptr); c.nanInArithmeticExpressionError(nullptr); c.commaSeparatedReturnError(nullptr); - c.redundantPointerOpError(nullptr, "varname", false); + c.redundantPointerOpError(nullptr, "varname", false, /*addressOfDeref*/ true); c.unusedLabelError(nullptr, false, false); c.unusedLabelError(nullptr, false, true); c.unusedLabelError(nullptr, true, false); diff --git a/test/testother.cpp b/test/testother.cpp index 23c9873330b..06e13e0b2dc 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -8764,6 +8764,16 @@ class TestOther : public TestFixture { "}\n", nullptr, false, true); ASSERT_EQUALS("[test.cpp:2]: (style) Redundant pointer operation on 'y' - it's already a pointer.\n", errout.str()); + check("int f() {\n" // #10991 + " int value = 4;\n" + " int result1 = *(&value);\n" + " int result2 = *&value;\n" + " return result1 + result2;\n" + "}\n", nullptr, false, true); + ASSERT_EQUALS("[test.cpp:3]: (style) Redundant pointer operation on 'value' - it's already a variable.\n" + "[test.cpp:4]: (style) Redundant pointer operation on 'value' - it's already a variable.\n", + errout.str()); + // no warning for bitwise AND check("void f(const int *b) {\n" " int x = 0x20 & *b;\n" From 76f0650a32b85bad03cc0ee798191ba4ffe55842 Mon Sep 17 00:00:00 2001 From: chrchr Date: Thu, 28 Apr 2022 10:41:29 +0200 Subject: [PATCH 2/3] Fix FP redundantPointerOp --- lib/checkother.cpp | 2 +- test/testother.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 16c74f5e244..ae1d309f565 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2951,7 +2951,7 @@ void CheckOther::checkRedundantPointerOp() // variable const Token *varTok = tok->astOperand1()->astOperand1(); - if (!varTok || varTok->isExpandedMacro()) + if (!varTok || varTok->isExpandedMacro() || (!addressOfDeref && varTok->valueType() && varTok->valueType()->pointer && varTok->valueType()->reference != Reference::None)) continue; const Variable *var = varTok->variable(); diff --git a/test/testother.cpp b/test/testother.cpp index 06e13e0b2dc..d753c70325f 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -8774,6 +8774,9 @@ class TestOther : public TestFixture { "[test.cpp:4]: (style) Redundant pointer operation on 'value' - it's already a variable.\n", errout.str()); + check("void f(int**& p) {}\n", nullptr, false, true); + ASSERT_EQUALS("", errout.str()); + // no warning for bitwise AND check("void f(const int *b) {\n" " int x = 0x20 & *b;\n" From d196e3931565c5caec6f9c8f08ff9820fb67fa0e Mon Sep 17 00:00:00 2001 From: chrchr Date: Thu, 28 Apr 2022 10:45:33 +0200 Subject: [PATCH 3/3] Check for LValue --- lib/checkother.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index ae1d309f565..a737b328abc 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2951,7 +2951,7 @@ void CheckOther::checkRedundantPointerOp() // variable const Token *varTok = tok->astOperand1()->astOperand1(); - if (!varTok || varTok->isExpandedMacro() || (!addressOfDeref && varTok->valueType() && varTok->valueType()->pointer && varTok->valueType()->reference != Reference::None)) + if (!varTok || varTok->isExpandedMacro() || (!addressOfDeref && varTok->valueType() && varTok->valueType()->pointer && varTok->valueType()->reference == Reference::LValue)) continue; const Variable *var = varTok->variable();