diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 0a0f7a3bb..340c939be 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -689,87 +689,43 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string // ClassCheck: Check that memset is not used on classes //--------------------------------------------------------------------------- -void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Token *typeTok) +void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type) { - // Warn if type is a class or struct that contains any std::* variables - const Scope *scope = symbolDatabase->findVariableType(start, typeTok); - if (!scope) - return; - - const Token *tstruct = scope->classDef; - const std::string &typeName = tstruct->str(); - - if (tstruct->tokAt(2)->str() == ":") + // recursively check all parent classes + for (size_t i = 0; i < type->derivedFrom.size(); i++) { - tstruct = tstruct->tokAt(3); - for (; tstruct; tstruct = tstruct->next()) - { - while (Token::Match(tstruct, "public|private|protected|virtual")) - { - tstruct = tstruct->next(); - } - - // recursively check all parent classes - checkMemsetType(start, tok, tstruct); - - tstruct = tstruct->next(); - if (tstruct->str() != ",") - break; - } + if (type->derivedFrom[i].scope) + checkMemsetType(start, tok, type->derivedFrom[i].scope); } - for (; tstruct; tstruct = tstruct->next()) + // Warn if type is a class that contains any virtual functions + std::list::const_iterator func; + + for (func = type->functionList.begin(); func != type->functionList.end(); ++func) { - if (tstruct->str() == "}") - break; + if (func->isVirtual) + memsetError(tok, tok->str(), "virtual method", type->classDef->str()); + } - // struct with function? skip function body.. - if (Token::simpleMatch(tstruct, ") {")) + // Warn if type is a class or struct that contains any std::* variables + std::list::const_iterator var; + + for (var = type->varlist.begin(); var != type->varlist.end(); ++var) + { + // don't warn if variable static or const + if (!var->isStatic() && !var->isConst()) { - tstruct = tstruct->next()->link(); - if (!tstruct) - break; - } + const Token *tok1 = var->typeStartToken(); - // before a statement there must be either: - // * private:|protected:|public: - // * { } ; - if (Token::Match(tstruct, "[;{}]") || - tstruct->str().find(":") != std::string::npos) - { - if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) - memsetError(tok, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeName); + // skip mutable token + if (var->isMutable()) + tok1 = tok1->next(); - else if (Token::Match(tstruct->next(), "std :: %type% <")) - { - // backup the type - const std::string typestr(tstruct->strAt(3)); + // check for std:: type that is not a pointer or reference + if (Token::simpleMatch(tok1, "std ::") && !Token::Match(var->nameToken()->previous(), "*|&")) + memsetError(tok, tok->str(), "'std::" + tok1->strAt(2) + "'", type->classDef->str()); - // check if it's a pointer variable.. - unsigned int level = 0; - while (0 != (tstruct = tstruct->next())) - { - if (tstruct->str() == "<") - ++level; - else if (tstruct->str() == ">") - { - if (level <= 1) - break; - --level; - } - else if (tstruct->str() == "(") - tstruct = tstruct->link(); - } - - if (!tstruct) - break; - - // found error => report - if (Token::Match(tstruct, "> %var% ;")) - memsetError(tok, tok->str(), "'std::" + typestr + "'", typeName); - } - else if (Token::simpleMatch(tstruct->next(), "virtual")) - memsetError(tok, tok->str(), "virtual method", typeName); + /** @todo warn if type is class/struct that doesn't require initialization */ } } } @@ -823,7 +779,10 @@ void CheckClass::noMemset() if (!typeTok) continue; - checkMemsetType(&(*scope), tok, typeTok); + const Scope *type = symbolDatabase->findVariableType(&(*scope), typeTok); + + if (type) + checkMemsetType(&(*scope), tok, type); } } } diff --git a/lib/checkclass.h b/lib/checkclass.h index a07d02e63..25f2eddae 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -84,7 +84,7 @@ public: * Important: The checking doesn't work on simplified tokens list. */ void noMemset(); - void checkMemsetType(const Scope *start, const Token *tok, const Token *typeTok); + void checkMemsetType(const Scope *start, const Token *tok, const Scope *type); /** @brief 'operator=' should return something and it should not be const. */ void operatorEq(); diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index cdd7d4a4e..4418797ea 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -104,7 +104,7 @@ bool CheckMemoryLeak::isclass(const Tokenizer *_tokenizer, const Token *tok) con } //--------------------------------------------------------------------------- -CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2, unsigned int varid) const +CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2, unsigned int varid, std::list *callstack) const { // What we may have... // * var = (char *)malloc(10); @@ -190,7 +190,19 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2, // User function const Token *ftok = tokenizer->getFunctionTokenByName(tok2->str().c_str()); - return functionReturnType(ftok); + if (ftok == NULL) + return No; + + // Prevent recursion + if (callstack && std::find(callstack->begin(), callstack->end(), ftok) != callstack->end()) + return No; + + std::list cs; + if (!callstack) + callstack = &cs; + + callstack->push_back(ftok); + return functionReturnType(ftok, callstack); } @@ -386,13 +398,11 @@ void CheckMemoryLeak::mismatchAllocDealloc(const std::list &calls reportErr(callstack, Severity::error, "mismatchAllocDealloc", "Mismatching allocation and deallocation: " + varname); } -CheckMemoryLeak::AllocType CheckMemoryLeak::functionReturnType(const Token *tok) const +CheckMemoryLeak::AllocType CheckMemoryLeak::functionReturnType(const Token *tok, std::list *callstack) const { if (!tok) return No; - const std::string functionName = tok->str(); - // Locate start of function while (tok) { @@ -437,11 +447,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::functionReturnType(const Token *tok) } else if (tok2->str() == "return") { - // recursion => bail out - if (tok2->strAt(1) == functionName) - return No; - - AllocType allocType = getAllocationType(tok2->next(), 0); + AllocType allocType = getAllocationType(tok2->next(), 0, callstack); if (allocType != No) return allocType; } @@ -457,11 +463,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::functionReturnType(const Token *tok) { if (Token::Match(tok, "%varid% =", varid)) { - // recursion => bail out - if (tok->strAt(2) == functionName) - return No; - - allocType = getAllocationType(tok->tokAt(2), varid); + allocType = getAllocationType(tok->tokAt(2), varid, callstack); } if (Token::Match(tok, "= %varid% ;", varid)) { diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index 7ba6a53b4..da254afff 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -114,7 +114,7 @@ public: /** * @brief Get type of allocation at given position */ - AllocType getAllocationType(const Token *tok2, unsigned int varid) const; + AllocType getAllocationType(const Token *tok2, unsigned int varid, std::list *callstack = NULL) const; /** * @brief Get type of reallocation at given position @@ -144,7 +144,7 @@ public: void memleakUponReallocFailureError(const Token *tok, const std::string &varname); /** What type of allocated memory does the given function return? */ - AllocType functionReturnType(const Token *tok) const; + AllocType functionReturnType(const Token *tok, std::list *callstack = NULL) const; /** Function allocates pointed-to argument (a la asprintf)? */ const char *functionArgAlloc(const Token *tok, unsigned int targetpar, AllocType &allocType) const; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 48955d39c..01ee0c1ac 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -5715,6 +5715,17 @@ void Tokenizer::simplifyVarDecl() } } + else if (Token::Match(tok2, "%type% :: %type% %var% ,|=")) + { + if (tok2->tokAt(3)->str() != "operator") + { + tok2 = tok2->tokAt(4); // The ',' token + typelen = 3; + } + else + tok2 = NULL; + } + else if (Token::Match(tok2, "%type% %var% [ %num% ] ,|=|[") || Token::Match(tok2, "%type% %var% [ %var% ] ,|=|[")) { diff --git a/test/testclass.cpp b/test/testclass.cpp index e4944addc..86822b058 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2933,6 +2933,28 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + checkNoMemset("class Fred\n" + "{\n" + " static std::string b;\n" + "};\n" + "void f()\n" + "{\n" + " Fred fred;\n" + " memset(&fred, 0, sizeof(Fred));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " std::string * b; \n" + "};\n" + "void f()\n" + "{\n" + " Fred fred;\n" + " memset(&fred, 0, sizeof(Fred));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + checkNoMemset("class Fred\n" "{\n" " std::string b; \n" @@ -2944,6 +2966,17 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + checkNoMemset("class Fred\n" + "{\n" + " mutable std::string b; \n" + "};\n" + "void f()\n" + "{\n" + " Fred fred;\n" + " memset(&fred, 0, sizeof(Fred));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + checkNoMemset("class Fred\n" "{\n" "};\n" diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index bf0c0376f..e05398a18 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -3631,6 +3631,18 @@ private: " char *s = foo();\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("int shl()\n" + "{\n" + " int a = shr();\n" + " return a;\n" + "}\n" + "\n" + "int shr()\n" + "{\n" + " return shl();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } }; diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 19bf4fb03..ea1167a50 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -4115,8 +4115,7 @@ private: void vardecl_stl_2() { const char code1[] = "{ std::string x = \"abc\"; }"; - TODO_ASSERT_EQUALS("{ std :: string x ; x = \"abc\" ; }", - "{ std :: string x = \"abc\" ; }", tokenizeAndStringify(code1)); + ASSERT_EQUALS("{ std :: string x ; x = \"abc\" ; }", tokenizeAndStringify(code1)); const char code2[] = "{ std::vector x = y; }"; TODO_ASSERT_EQUALS("{ std :: vector < int > x ; x = y ; }",