From ac59485e7ebc41951e193e13909c7c82dc8e23a1 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Mon, 4 Aug 2014 11:45:24 +0200 Subject: [PATCH] Refactorized CheckAutoVariables::assignFunctionArg(): - Splitted message into style message (assigning non-pointers) and warning message (assigning pointers) - Support operator++/-- (#4793) --- lib/checkautovariables.cpp | 22 ++++++++++++++++++---- lib/checkautovariables.h | 5 ++++- test/testautovariables.cpp | 18 ++++++++++++------ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index a6791315b..92c288f4b 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -119,17 +119,23 @@ static bool variableIsUsedInScope(const Token* start, unsigned int varId, const void CheckAutoVariables::assignFunctionArg() { - if (!_settings->isEnabled("warning")) + bool style = _settings->isEnabled("style"); + bool warning = _settings->isEnabled("warning"); + if (!style && !warning) return; + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++i) { const Scope * scope = symbolDatabase->functionScopes[i]; for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { - if (Token::Match(tok, "[;{}] %var% =") && + if (Token::Match(tok, "[;{}] %var% =|++|--") && isNonReferenceArg(tok->next()) && !variableIsUsedInScope(Token::findsimplematch(tok->tokAt(2), ";"), tok->next()->varId(), scope)) { - errorUselessAssignmentPtrArg(tok->next()); + if (tok->next()->variable()->isPointer() && warning) + errorUselessAssignmentPtrArg(tok->next()); + else if (style) + errorUselessAssignmentArg(tok->next()); } } } @@ -274,12 +280,20 @@ void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok, "value is invalid."); } +void CheckAutoVariables::errorUselessAssignmentArg(const Token *tok) +{ + reportError(tok, + Severity::style, + "uselessAssignmentArg", + "Assignment of function parameter has no effect outside the function."); +} + void CheckAutoVariables::errorUselessAssignmentPtrArg(const Token *tok) { reportError(tok, Severity::warning, "uselessAssignmentPtrArg", - "Assignment of function parameter has no effect outside the function."); + "Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?"); } //--------------------------------------------------------------------------- diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 087d992a9..eed95a8c2 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -87,6 +87,7 @@ private: void errorReturnTempReference(const Token *tok); void errorInvalidDeallocation(const Token *tok); void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname); + void errorUselessAssignmentArg(const Token *tok); void errorUselessAssignmentPtrArg(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { @@ -98,6 +99,7 @@ private: c.errorReturnTempReference(0); c.errorInvalidDeallocation(0); c.errorReturnAddressOfFunctionParameter(0, "parameter"); + c.errorUselessAssignmentArg(0); c.errorUselessAssignmentPtrArg(0); } @@ -112,7 +114,8 @@ private: "* assigning address of an variable to an effective parameter of a function\n" "* returning reference to local/temporary variable\n" "* returning address of function parameter\n" - "* useless assignment of pointer parameter\n"; + "* suspicious assignment of pointer argument\n" + "* useless assignment of function argument\n"; } }; /// @} diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 02680967c..4b945793f 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -41,6 +41,7 @@ private: Settings settings; settings.inconclusive = inconclusive; settings.addEnabled("warning"); + settings.addEnabled("style"); // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -132,7 +133,7 @@ private: " int num = 2;\n" " res = #\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?\n", errout.str()); check("void func1(int **res)\n" "{\n" @@ -161,7 +162,7 @@ private: " int num = 2;\n" " res = #\n" "}"); - ASSERT_EQUALS("[test.cpp:7]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?\n", errout.str()); check("class Fred {\n" " void func1(int **res);\n" @@ -264,12 +265,12 @@ private: check("void foo(char* p) {\n" " p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?\n", errout.str()); check("void foo(int b) {\n" " b = foo(b);\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Assignment of function parameter has no effect outside the function.\n", errout.str()); check("void foo(char* p) {\n" " if (!p) p = buf;\n" @@ -297,13 +298,13 @@ private: check("void foo(Foo* p) {\n" " p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?\n", errout.str()); check("class Foo {};\n" "void foo(Foo p) {\n" " p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Assignment of function parameter has no effect outside the function.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Assignment of function parameter has no effect outside the function.\n", errout.str()); check("void foo(Foo p) {\n" " p = 0;\n" @@ -321,6 +322,11 @@ private: " return d;" "}",false,false); ASSERT_EQUALS("", errout.str()); + + check("void foo(int* ptr) {\n" // #4793 + " ptr++;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?\n", errout.str()); } void testautovar11() { // #4641 - fp, assign local struct member address to function parameter