From 29063098bfd144f1f3ffd32872fd29ac6d05545e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 18 Mar 2012 07:49:22 +0100 Subject: [PATCH] Fixed #3670 (false positive: Allocation with open never assigned) --- lib/checkmemoryleak.cpp | 60 +++++++++++++++++++++++++---------------- lib/checkmemoryleak.h | 24 +++++++++-------- test/testmemleak.cpp | 18 ++++++++----- 3 files changed, 62 insertions(+), 40 deletions(-) diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 6f3bfc4c8..8cb49fcbc 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -39,6 +39,24 @@ namespace { CheckMemoryLeakNoVar instance4; } +/** + * Count function parameters + * \param tok Function name token before the '(' + */ +static unsigned int countParameters(const Token *tok) +{ + tok = tok->tokAt(2); + if (tok->str() == ")") + return 0; + + unsigned int numpar = 1; + while (NULL != (tok = tok->nextArgument())) + numpar++; + + return numpar; +} + + /** List of functions that can be ignored when searching for memory leaks. * These functions don't take the address of the given pointer * This list needs to be alphabetically sorted so we can run bsearch on it. @@ -168,19 +186,27 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2, if (Token::Match(tok2, "fopen|tmpfile|g_fopen (")) return File; - if (Token::Match(tok2, "open|openat|creat|mkstemp|mkostemp (")) { - // is there a user function with this name? - if (tokenizer && Token::findmatch(tokenizer->tokens(), ("%type% *|&| " + tok2->str()).c_str())) - return No; - return Fd; + if (standards.posix) { + if (Token::Match(tok2, "open|openat|creat|mkstemp|mkostemp (")) { + // simple sanity check of function parameters.. + // TODO: Make such check for all these functions + unsigned int num = countParameters(tok2); + if (tok2->str() == "open" && num != 2 && num != 3) + return No; + + // is there a user function with this name? + if (tokenizer && Token::findmatch(tokenizer->tokens(), ("%type% *|&| " + tok2->str()).c_str())) + return No; + return Fd; + } + + if (Token::simpleMatch(tok2, "popen (")) + return Pipe; + + if (Token::Match(tok2, "opendir|fdopendir (")) + return Dir; } - if (Token::simpleMatch(tok2, "popen (")) - return Pipe; - - if (Token::Match(tok2, "opendir|fdopendir (")) - return Dir; - // User function const Token *ftok = tokenizer->getFunctionTokenByName(tok2->str().c_str()); if (ftok == NULL) @@ -603,18 +629,6 @@ bool CheckMemoryLeakInFunction::notvar(const Token *tok, unsigned int varid, boo } -static unsigned int countParameters(const Token *tok) -{ - tok = tok->tokAt(2); - if (tok->str() == ")") - return 0; - - unsigned int numpar = 1; - while (NULL != (tok = tok->nextArgument())) - numpar++; - - return numpar; -} bool CheckMemoryLeakInFunction::test_white_list(const std::string &funcname) { diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index 717b03705..9c263f5ee 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -54,6 +54,9 @@ private: /** ErrorLogger used to report errors */ ErrorLogger * const errorLogger; + /** Enabled standards */ + const Standards & standards; + /** Disable the default constructors */ CheckMemoryLeak(); @@ -82,9 +85,8 @@ private: void reportErr(const std::list &callstack, Severity::SeverityType severity, const std::string &id, const std::string &msg) const; public: - CheckMemoryLeak(const Tokenizer *t, ErrorLogger *e) - : tokenizer(t), errorLogger(e) { - + CheckMemoryLeak(const Tokenizer *t, ErrorLogger *e, const Standards &s) + : tokenizer(t), errorLogger(e), standards(s) { } /** @brief What type of allocation are used.. the "Many" means that several types of allocation and deallocation are used */ @@ -180,12 +182,12 @@ public: class CheckMemoryLeakInFunction : private Check, public CheckMemoryLeak { public: /** @brief This constructor is used when registering this class */ - CheckMemoryLeakInFunction() : Check(myName()), CheckMemoryLeak(0, 0), symbolDatabase(NULL) + CheckMemoryLeakInFunction() : Check(myName()), CheckMemoryLeak(0, 0, Standards()), symbolDatabase(NULL) { } /** @brief This constructor is used when running checks */ CheckMemoryLeakInFunction(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) - : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog) { + : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog, settings->standards) { // get the symbol database if (tokenizr) symbolDatabase = tokenizr->getSymbolDatabase(); @@ -357,11 +359,11 @@ private: class CheckMemoryLeakInClass : private Check, private CheckMemoryLeak { public: - CheckMemoryLeakInClass() : Check(myName()), CheckMemoryLeak(0, 0) + CheckMemoryLeakInClass() : Check(myName()), CheckMemoryLeak(0, 0, Standards()) { } CheckMemoryLeakInClass(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) - : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog) + : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog, settings->standards) { } void runSimplifiedChecks(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) { @@ -400,11 +402,11 @@ private: class CheckMemoryLeakStructMember : private Check, private CheckMemoryLeak { public: - CheckMemoryLeakStructMember() : Check(myName()), CheckMemoryLeak(0, 0) + CheckMemoryLeakStructMember() : Check(myName()), CheckMemoryLeak(0, 0, Standards()) { } CheckMemoryLeakStructMember(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) - : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog) + : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog, settings->standards) { } void runSimplifiedChecks(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) { @@ -439,11 +441,11 @@ private: class CheckMemoryLeakNoVar : private Check, private CheckMemoryLeak { public: - CheckMemoryLeakNoVar() : Check(myName()), CheckMemoryLeak(0, 0) + CheckMemoryLeakNoVar() : Check(myName()), CheckMemoryLeak(0, 0, Standards()) { } CheckMemoryLeakNoVar(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) - : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog) + : Check(myName(), tokenizr, settings, errLog), CheckMemoryLeak(tokenizr, errLog, settings->standards) { } void runSimplifiedChecks(const Tokenizer *tokenizr, const Settings *settings, ErrorLogger *errLog) { diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index d2dde8d32..96320198e 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -96,6 +96,7 @@ private: errout.str(""); Settings settings; + settings.standards.posix = true; Tokenizer tokenizer(&settings, this); std::istringstream istr(code); @@ -103,7 +104,7 @@ private: // there is no allocation const Token *tok = Token::findsimplematch(tokenizer.tokens(), "ret ="); - CheckMemoryLeak check(&tokenizer, 0); + CheckMemoryLeak check(&tokenizer, 0, settings.standards); ASSERT_EQUALS(CheckMemoryLeak::No, check.getAllocationType(tok->tokAt(2), 1)); } }; @@ -126,6 +127,7 @@ private: Settings settings; settings.experimental = experimental; + settings.standards.posix = true; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -353,6 +355,7 @@ private: errout.str(""); Settings settings; + settings.standards.posix = true; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -392,6 +395,8 @@ private: ASSERT_EQUALS(";;alloc;", getcode("int * const a = new int[10];", "a")); ASSERT_EQUALS(";;alloc;", getcode("const int * const a = new int[10];", "a")); ASSERT_EQUALS(";;alloc;", getcode("char *a = g_strdup_printf(\"%i\", f());", "a")); + ASSERT_EQUALS(";;alloc;", getcode("int i = open(a,b);", "i")); + ASSERT_EQUALS(";;assign;", getcode("int i = open();", "i")); // alloc; return use; ASSERT_EQUALS(";;alloc;returnuse;", getcode("int *a = new int[10]; return a;", "a")); @@ -456,8 +461,8 @@ private: ASSERT_EQUALS(";;while1{}", getcode("char *s; for(;;) { }", "s")); ASSERT_EQUALS(";;while(var){}", getcode("char *s; while (s) { }", "s")); ASSERT_EQUALS(";;while(!var){}", getcode("char *s; while (!s) { }", "s")); - ASSERT_EQUALS(";;alloc;while(var){}", getcode("int fd = open(); while (fd >= 0) { }", "fd")); - ASSERT_EQUALS(";;alloc;while(!var){}", getcode("int fd = open(); while (fd < 0) { }", "fd")); + ASSERT_EQUALS(";;alloc;while(var){}", getcode("int fd = open(a,b); while (fd >= 0) { }", "fd")); + ASSERT_EQUALS(";;alloc;while(!var){}", getcode("int fd = open(a,b); while (fd < 0) { }", "fd")); // asprintf.. ASSERT_EQUALS(";;alloc;", getcode("char *s; asprintf(&s, \"xyz\");", "s")); @@ -514,8 +519,8 @@ private: ASSERT_EQUALS(";;exit;{}", getcode("char *s; list_for_each(x,y,z) { }", "s")); // open/close - ASSERT_EQUALS(";;alloc;if(var){dealloc;}", getcode("int f; f=open(); if(f>=0)close(f);", "f")); - ASSERT_EQUALS(";;alloc;ifv{;}", getcode("int f; f=open(); if(f!=-1 || x);", "f")); + ASSERT_EQUALS(";;alloc;if(var){dealloc;}", getcode("int f; f=open(a,b); if(f>=0)close(f);", "f")); + ASSERT_EQUALS(";;alloc;ifv{;}", getcode("int f; f=open(a,b); if(f!=-1 || x);", "f")); ASSERT_EQUALS(";;;dealloc;loop{}}", getcode(";int f; while (close(f) == -1) { } }", "f")); ASSERT_EQUALS(";;;dealloc;assign;;}", getcode(";int res; res = close(res); }", "res")); @@ -3447,7 +3452,7 @@ private: "{\n" " int handle;\n" " \n" - " handle = open(\"myfile\");\n" + " handle = open(\"myfile\", O_RDONLY);\n" " if (handle < 0) return 1;\n" " \n" " while (some_condition()) \n" @@ -5183,6 +5188,7 @@ private: errout.str(""); Settings settings; + settings.standards.posix = true; // Tokenize.. Tokenizer tokenizer(&settings, this);