diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 934e45e10..b3f9012a9 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2322,6 +2322,9 @@ void CheckMemoryLeakInClass::parseClass(const Token *tok1, std::vectornext(); tok1 = tok1 ? tok1->next() : 0; + // are we parsing the private scope of the class? + bool privateScope = true; + unsigned int indentlevel = 0; for (const Token *tok = tok1; tok; tok = tok->next()) { @@ -2335,6 +2338,9 @@ void CheckMemoryLeakInClass::parseClass(const Token *tok1, std::vectorisName() && tok->str().find(":") != std::string::npos) + privateScope = bool(tok->str() == "private:"); + // Only parse this particular class.. not subclasses if (indentlevel > 0) continue; @@ -2356,6 +2362,10 @@ void CheckMemoryLeakInClass::parseClass(const Token *tok1, std::vectorprevious(), ";|{|}|private:|protected:|public: %type% * %var% ;")) { + // allocation but no deallocation of private variables in public function.. + if (privateScope && tok->isStandardType()) + checkPublicFunctions(tok1, tok->tokAt(2)->varId()); + // No false positives for auto deallocated classes.. if (_settings->isAutoDealloc(tok->str().c_str())) continue; @@ -2494,9 +2504,51 @@ void CheckMemoryLeakInClass::variable(const std::string &classname, const Token } +void CheckMemoryLeakInClass::checkPublicFunctions(const Token *classtok, const unsigned int varid) +{ + // Check that public functions deallocate the pointers that they allocate. + // There is no checking how these functions are used and therefore it + // isn't established if there is real leaks or not. + if (!_settings->_checkCodingStyle) + return; + // Parse public functions.. + // If they allocate member variables, they should also deallocate + bool publicScope = false; + for (const Token *tok = classtok; tok; tok = tok->next()) + { + if (tok->str() == "{") + tok = tok->link(); + else if (tok->str() == "}") + break; + else if (tok->isName() && tok->str().find(":") != std::string::npos) + publicScope = bool(tok->str() == "public:"); + // scope of public function.. + // TODO: parse into any function scope that is not a constructor + else if (publicScope && (Token::Match(tok, "void %type% (") || Token::simpleMatch(tok, "operator = ("))) + { + tok = tok->tokAt(2)->link(); + if (Token::Match(tok, ") const| {")) + { + const Token *tok2 = tok; + while (tok2->str() != "{") + tok2 = tok2->next(); + if (Token::Match(tok2, "{ %varid% =", varid)) + { + const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(3), varid); + if (alloc != CheckMemoryLeak::No) + publicAllocationError(tok2, tok2->strAt(1)); + } + } + } + } +} +void CheckMemoryLeakInClass::publicAllocationError(const Token *tok, const std::string &varname) +{ + reportError(tok, Severity::style, "publicAllocationError", "Possible leak in public function. The pointer '" + varname + "' is not deallocated before it is allocated."); +} diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index 17e345537..e36714a00 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -356,6 +356,10 @@ private: void parseClass(const Token *tok1, std::vector &classname); void variable(const std::string &classname, const Token *tokVarname); + /** Public functions: possible double-allocation */ + void checkPublicFunctions(const Token *classtok, const unsigned int varid); + void publicAllocationError(const Token *tok, const std::string &varname); + void getErrorMessages() { } diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 55592e236..4863bad19 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -2786,6 +2786,7 @@ private: // Check for memory leaks.. Settings settings; settings.inconclusive = true; + settings._checkCodingStyle = true; tokenizer.fillFunctionList(); CheckMemoryLeakInClass checkMemoryLeak(&tokenizer, &settings, this); checkMemoryLeak.check(); @@ -2815,6 +2816,10 @@ private: TEST_CASE(free_member_in_sub_func); TEST_CASE(mismatch1); + + // allocating member variable in public function + TEST_CASE(func1); + TEST_CASE(func2); } @@ -3223,7 +3228,48 @@ private: ASSERT_EQUALS("[test.cpp:14]: (possible error) Mismatching allocation and deallocation: A::pkt_buffer\n", errout.str()); } + void func1() + { + check("class Fred\n" + "{\n" + "private:\n" + " char *s;\n" + "public:\n" + " Fred() { s = 0; }\n" + " ~Fred() { free(s); }\n" + " void xy()\n" + " { s = malloc(100); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:9]: (style) Possible leak in public function. The pointer 's' is not deallocated before it is allocated.\n", errout.str()); + check("class Fred\n" + "{\n" + "public:\n" + " Fred() { s = 0; }\n" + " ~Fred() { free(s); }\n" + " void xy()\n" + " { s = malloc(100); }\n" + "private:\n" + " char *s;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + TODO_ASSERT_EQUALS("publicAllocation", errout.str()); + } + + void func2() + { + check("class Fred\n" + "{\n" + "private:\n" + " char *s;\n" + "public:\n" + " Fred() { s = 0; }\n" + " ~Fred() { free(s); }\n" + " const Fred & operator = (const Fred &f)\n" + " { s = malloc(100); }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:9]: (style) Possible leak in public function. The pointer 's' is not deallocated before it is allocated.\n", errout.str()); + } }; static TestMemleakInClass testMemleakInClass;