diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index d218543af..80f635ca5 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -580,7 +580,7 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const std::string& b /** * Execute a shell command and read the output from it. Returns true if command terminated successfully. */ -// cppcheck-suppress passedByValue - used as callback so we need to preserve the signature +// cppcheck-suppress passedByValueCallback - used as callback so we need to preserve the signature // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature int CppCheckExecutor::executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_) { diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index 988a2ab4b..a70bc39e6 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -56,7 +56,7 @@ #endif // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature -static int executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue +static int executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) // cppcheck-suppress passedByValueCallback { output.clear(); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 418c9f06b..e8c904976 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1366,7 +1366,7 @@ void CheckOther::checkPassByReference() const bool isConst = var->isConst(); if (isConst) { - passedByValueError(var->nameToken(), var->name(), inconclusive); + passedByValueError(var, inconclusive); continue; } @@ -1375,18 +1375,26 @@ void CheckOther::checkPassByReference() continue; if (canBeConst(var, mSettings)) { - passedByValueError(var->nameToken(), var->name(), inconclusive); + passedByValueError(var, inconclusive); } } } -void CheckOther::passedByValueError(const Token *tok, const std::string &parname, bool inconclusive) +void CheckOther::passedByValueError(const Variable* var, bool inconclusive) { - reportError(tok, Severity::performance, "passedByValue", - "$symbol:" + parname + "\n" - "Function parameter '$symbol' should be passed by const reference.\n" - "Parameter '$symbol' is passed by value. It could be passed " - "as a const reference which is usually faster and recommended in C++.", CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); + std::string id = "passedByValue"; + std::string msg = "$symbol:" + (var ? var->name() : "") + "\n" + "Function parameter '$symbol' should be passed by const reference."; + ErrorPath errorPath; + if (var && var->scope() && var->scope()->function && var->scope()->function->functionPointerUsage) { + id += "Callback"; + errorPath.emplace_front(var->scope()->function->functionPointerUsage, "Function pointer used here."); + msg += " However it seems that '" + var->scope()->function->name() + "' is a callback function."; + } + if (var) + errorPath.emplace_back(var->nameToken(), msg); + msg += "\nParameter '$symbol' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++."; + reportError(errorPath, Severity::performance, id.c_str(), msg, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); } static bool isUnusedVariable(const Variable *var) diff --git a/lib/checkother.h b/lib/checkother.h index 701b90090..420c208b5 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -241,7 +241,7 @@ private: void clarifyStatementError(const Token* tok); void cstyleCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive, bool toIsInt); - void passedByValueError(const Token *tok, const std::string &parname, bool inconclusive); + void passedByValueError(const Variable* var, bool inconclusive); void constVariableError(const Variable *var, const Function *function); void constStatementError(const Token *tok, const std::string &type, bool inconclusive); void signedCharArrayIndexError(const Token *tok); @@ -314,7 +314,7 @@ private: c.checkComparisonFunctionIsAlwaysTrueOrFalseError(nullptr, "isless","varName",false); c.checkCastIntToCharAndBackError(nullptr, "func_name"); c.cstyleCastError(nullptr); - c.passedByValueError(nullptr, "parametername", false); + c.passedByValueError(nullptr, false); c.constVariableError(nullptr, nullptr); c.constStatementError(nullptr, "type", false); c.signedCharArrayIndexError(nullptr); diff --git a/test/testother.cpp b/test/testother.cpp index 4440cf517..a5ac9521f 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -9916,6 +9916,16 @@ private: check("std::map m;\n" // #10817 "void f(const decltype(m)::const_iterator i) {}"); ASSERT_EQUALS("", errout.str()); + + check("int (*pf) (std::vector) = nullptr;\n" // #12118 + "int f(std::vector v) {\n" + " return v.size();\n" + "}\n" + "void g() {\n" + " pf = f;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (performance) Function parameter 'v' should be passed by const reference. However it seems that 'f' is a callback function.\n", + errout.str()); } void checkComparisonFunctionIsAlwaysTrueOrFalse() {