From f28dec1f5a1eab27515d5e1cdd0ac65c6199395b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 21 Jun 2009 13:48:39 +0200 Subject: [PATCH 1/5] detect memory leak when all is given when calling an unknown function --- src/checkmemoryleak.cpp | 16 ++++++++++++++++ test/testmemleak.cpp | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/checkmemoryleak.cpp b/src/checkmemoryleak.cpp index fd47650e4..6a792b56a 100644 --- a/src/checkmemoryleak.cpp +++ b/src/checkmemoryleak.cpp @@ -988,6 +988,22 @@ void CheckMemoryLeakInFunction::simplifycode(Token *tok, bool &all) tok2 = tokEnd; } + // If "--all" is given, remove all "callfunc".. + if (_settings->_showAll) + { + Token *tok2 = tok; + while (tok2) + { + if (tok2->str() == "callfunc") + { + tok2->deleteThis(); + all = true; + } + else + tok2 = tok2->next(); + } + } + // reduce the code.. bool done = false; while (! done) diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 7ce92577a..c81dbb358 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -254,6 +254,7 @@ private: TEST_CASE(unknownFunction1); TEST_CASE(unknownFunction2); TEST_CASE(unknownFunction3); + TEST_CASE(unknownFunction4); // VCL.. TEST_CASE(vcl1); @@ -1951,6 +1952,18 @@ private: ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: p\n", errout.str()); } + void unknownFunction4() + { + check("void foo()\n" + "{\n" + " int *p = new int[100];\n" + " a();\n" + " if (b) return;\n" + " delete [] p;\n" + "}\n", true); + ASSERT_EQUALS("[test.cpp:5]: (all) Memory leak: p\n", errout.str()); + } + void checkvcl(const char code[], const char _autoDealloc[]) From 8715ba14586e19ad2a5401d9e3d7202fc073fa67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 21 Jun 2009 14:12:59 +0200 Subject: [PATCH 2/5] CheckMemoryLeakInFunction: More sensitive checking when the code calls an unknown function --- src/checkmemoryleak.cpp | 8 ++++++++ test/testmemleak.cpp | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/checkmemoryleak.cpp b/src/checkmemoryleak.cpp index 6a792b56a..1d113ea3e 100644 --- a/src/checkmemoryleak.cpp +++ b/src/checkmemoryleak.cpp @@ -1353,6 +1353,14 @@ void CheckMemoryLeakInFunction::simplifycode(Token *tok, bool &all) done = false; } + // Delete "callfunc ;" that is followed by "use|if|callfunc" + // If the function doesn't throw exception or exit the application, then the "callfunc" is not needed + if (Token::Match(tok2, "callfunc ; use|if|callfunc")) + { + tok2->deleteThis(); + done = false; + } + // Delete second case in "case ; case ;" while (Token::simpleMatch(tok2, "case ; case ;")) { diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index c81dbb358..7f0855283 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -1960,8 +1960,8 @@ private: " a();\n" " if (b) return;\n" " delete [] p;\n" - "}\n", true); - ASSERT_EQUALS("[test.cpp:5]: (all) Memory leak: p\n", errout.str()); + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Memory leak: p\n", errout.str()); } From 13e805f332d5b846b0c1c56d774f3933a487086c Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Sun, 21 Jun 2009 22:01:43 +0700 Subject: [PATCH 3/5] Fixed ticket #399 (Add detection for resource leaks after open() usage) http://sourceforge.net/apps/trac/cppcheck/ticket/399 --- src/checkmemoryleak.cpp | 26 ++++++++++- src/checkmemoryleak.h | 2 +- test/testmemleak.cpp | 95 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) diff --git a/src/checkmemoryleak.cpp b/src/checkmemoryleak.cpp index 1d113ea3e..f1f7868ec 100644 --- a/src/checkmemoryleak.cpp +++ b/src/checkmemoryleak.cpp @@ -117,6 +117,9 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::GetAllocationType(const Token *tok2) if (Token::Match(tok2, "fopen|tmpfile (")) return File; + if (Token::Match(tok2, "open|openat|creat|mkstemp|mkostemp (")) + return Fd; + if (Token::simpleMatch(tok2, "popen (")) return Pipe; @@ -188,6 +191,9 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::GetDeallocationType(const Token *tok Token::simpleMatch(tok, "fcloseall ( )")) return File; + if (Token::simpleMatch(tok, std::string("close ( " + names + " )").c_str())) + return Fd; + if (Token::simpleMatch(tok, std::string("pclose ( " + names + " )").c_str())) return Pipe; @@ -205,6 +211,7 @@ void CheckMemoryLeak::MemoryLeak(const Token *tok, const char varname[], AllocTy { if (alloctype == CheckMemoryLeak::File || alloctype == CheckMemoryLeak::Pipe || + alloctype == CheckMemoryLeak::Fd || alloctype == CheckMemoryLeak::Dir) resourceLeakError(tok, varname); else if (all) @@ -400,6 +407,12 @@ const char * CheckMemoryLeakInFunction::call_func(const Token *tok, std::liststr() != ")") tok = tok->next(); } + else if (Token::simpleMatch(tok, std::string("if ( " + varnameStr + " == -1 )").c_str()) || + Token::simpleMatch(tok, std::string("if ( " + varnameStr + " < 0 )").c_str())) + { + // FIXME: ensure then this variable has int type and uses as file descriptor + addtoken("if(!var)"); + } else if (Token::simpleMatch(tok, "if (") && notvar(tok->tokAt(2), varnames, true)) { addtoken("if(!var)"); @@ -714,7 +733,7 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::listtokAt(4)->str() != "const" ? 4 : 5); checkScope(tok->next(), tok->strAt(varname_tok), classmember, sz); } + + else if (Token::Match(tok, "[{};] int %var% [;=]")) + { + checkScope(tok->next(), tok->strAt(2), classmember, sz); + } } } } diff --git a/src/checkmemoryleak.h b/src/checkmemoryleak.h index c07a1a043..bac1b4e94 100644 --- a/src/checkmemoryleak.h +++ b/src/checkmemoryleak.h @@ -47,7 +47,7 @@ public: CheckMemoryLeak() { } /** What type of allocation are used.. the "Many" means that several types of allocation and deallocation are used */ - enum AllocType { No, Malloc, gMalloc, New, NewArray, File, Pipe, Dir, Many }; + enum AllocType { No, Malloc, gMalloc, New, NewArray, File, Fd, Pipe, Dir, Many }; void MemoryLeak(const Token *tok, const char varname[], AllocType alloctype, bool all); void MismatchError(const Token *Tok1, const std::list &callstack, const char varname[]); diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 7f0855283..a0a04c95e 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -274,6 +274,11 @@ private: TEST_CASE(fcloseall_function); TEST_CASE(file_functions); + TEST_CASE(open_function); + TEST_CASE(creat_function); + TEST_CASE(close_function); + TEST_CASE(fd_functions); + TEST_CASE(opendir_function); TEST_CASE(fdopendir_function); TEST_CASE(closedir_function); @@ -2177,6 +2182,96 @@ private: ASSERT_EQUALS("", errout.str()); } + void open_function() + { + check("void f(const char *path)\n" + "{\n" + " int fd = open(path, O_RDONLY);\n" + "}\n", true); + ASSERT_EQUALS("[test.cpp:4]: (error) Resource leak: fd\n", errout.str()); + + check("void f(const char *path)\n" + "{\n" + " int fd = open(path, O_RDONLY);\n" + " if (fd == -1)\n" + " return;\n" + " close(fd);\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + + check("void f(const char *path)\n" + "{\n" + " int fd = open(path, O_RDONLY);\n" + " if (fd < 0)\n" + " return;\n" + " close(fd);\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + } + + void creat_function() + { + check("void f(const char *path)\n" + "{\n" + " int fd = creat(path, S_IRWXU);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Resource leak: fd\n", errout.str()); + } + + void close_function() + { + check("void f(const char *path)\n" + "{\n" + " int fd = open(path, O_RDONLY);\n" + " close(fd);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(const char *path)\n" + "{\n" + " int fd = creat(path, S_IRWXU);\n" + " close(fd);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(const char *path)\n" + "{\n" + " int fd = creat(path, S_IRWXU);\n" + " if (close(fd) < 0) {\n" + " perror(\"close\");\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void fd_functions() + { + check("void f(const char *path)\n" + "{\n" + " int fd = open(path, O_RDONLY);\n" + " read(fd, buf, count);\n" + " readv(fd, iov, iovcnt);\n" + " readahead(fd, offset, count);\n" + " pread(fd, buf, count, offset);\n" + " write(fd, buf, count);\n" + " writev(fd, iov, iovcnt);\n" + " pwrite(fd, buf, count, offset);\n" + " ioctl(fd, request);\n" + " posix_fallocate(fd, offset, len);\n" + " posix_fadvise(fd, offset, len, advise);\n" + " fsync(fd);\n" + " fdatasync(fd);\n" + " sync_file_range(fd, offset, nbytes, flags);\n" + " lseek(fd, offset, whence);\n" + " fcntl(fd, cmd);\n" + " flock(fd, op);\n" + " lockf(fd, cmd, len);\n" + " ftruncate(fd, len);\n" + " fstat(fd, buf);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:23]: (error) Resource leak: fd\n", errout.str()); + } + void opendir_function() { check("void f()\n" From 5b5352226c4615e9fd09a4af75befd97939fd9a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 21 Jun 2009 21:03:21 +0200 Subject: [PATCH 4/5] doc: simplified the documentation for autovariables a bit --- src/checkautovariables.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/checkautovariables.h b/src/checkautovariables.h index ba29fbf3d..a96f04878 100644 --- a/src/checkautovariables.h +++ b/src/checkautovariables.h @@ -78,11 +78,10 @@ private: std::string classInfo() const { - return "Auto variables are deallocated when they go out of scope. " - "A pointer to an auto variable is therefore only valid as long as the auto variable is in scope.\n" + return "A pointer to a variable is only valid as long as the variable is in scope.\n" "Check:\n" - " * returning a pointer to auto variable\n" - " * assignment of an auto-variable to an effective parameter of a function\n"; + " * returning a pointer to variable\n" + " * assigning address of an variable to an effective parameter of a function\n"; } }; From 5c36d339436d67ae580c5aaeced219fa0f058757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 21 Jun 2009 21:03:58 +0200 Subject: [PATCH 5/5] astyle formatting --- src/checkmemoryleak.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/checkmemoryleak.cpp b/src/checkmemoryleak.cpp index f1f7868ec..ba431014b 100644 --- a/src/checkmemoryleak.cpp +++ b/src/checkmemoryleak.cpp @@ -1372,13 +1372,13 @@ void CheckMemoryLeakInFunction::simplifycode(Token *tok, bool &all) done = false; } - // Delete "callfunc ;" that is followed by "use|if|callfunc" - // If the function doesn't throw exception or exit the application, then the "callfunc" is not needed - if (Token::Match(tok2, "callfunc ; use|if|callfunc")) - { - tok2->deleteThis(); - done = false; - } + // Delete "callfunc ;" that is followed by "use|if|callfunc" + // If the function doesn't throw exception or exit the application, then the "callfunc" is not needed + if (Token::Match(tok2, "callfunc ; use|if|callfunc")) + { + tok2->deleteThis(); + done = false; + } // Delete second case in "case ; case ;" while (Token::simpleMatch(tok2, "case ; case ;"))