diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 569f4ed9d..a43c39ea5 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -693,6 +693,103 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string // ClassCheck: Check that memset is not used on classes //--------------------------------------------------------------------------- +void CheckClass::checkMemsetType(const Token *tok, const std::string &type) +{ + // check for cached message for this type + std::map::const_iterator msg = _memsetClassMessages.find(type); + if (msg != _memsetClassMessages.end()) + { + memsetError(tok, type, msg->second); + return; + } + + // Warn if type is a class or struct that contains any std::* variables + const std::string pattern2(std::string("struct|class ") + type + " :|{"); + const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); + + if (!tstruct) + return; + + // typeKind is either 'struct' or 'class' + const std::string &typeKind = tstruct->str(); + + if (tstruct->tokAt(2)->str() == ":") + { + 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(tok, tstruct->str()); + + tstruct = tstruct->next(); + if (tstruct->str() != ",") + break; + } + } + + for (; tstruct; tstruct = tstruct->next()) + { + if (tstruct->str() == "}") + break; + + // struct with function? skip function body.. + if (Token::simpleMatch(tstruct, ") {")) + { + tstruct = tstruct->next()->link(); + if (!tstruct) + break; + } + + // 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, type, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeKind); + + else if (Token::Match(tstruct->next(), "std :: %type% <")) + { + // backup the type + const std::string typestr(tstruct->strAt(3)); + + // 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, type, tok->str(), "'std::" + typestr + "'", typeKind); + } + else if (Token::simpleMatch(tstruct->next(), "virtual")) + memsetError(tok, type, tok->str(), "virtual method", typeKind); + else if (!Token::Match(tstruct->next(), "static|}")) + checkMemsetType(tok, tstruct->next()->str()); + } + } +} + void CheckClass::noMemset() { createSymbolDatabase(); @@ -726,73 +823,20 @@ void CheckClass::noMemset() if (type.empty()) continue; - // Warn if type is a class or struct that contains any std::* variables - const std::string pattern2(std::string("struct|class ") + type + " {"); - const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); - - if (!tstruct) - continue; - - const std::string &typeName = tstruct->str(); - - for (; tstruct; tstruct = tstruct->next()) - { - if (tstruct->str() == "}") - break; - - // struct with function? skip function body.. - if (Token::simpleMatch(tstruct, ") {")) - { - tstruct = tstruct->next()->link(); - if (!tstruct) - break; - } - - // 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(), tstruct->strAt(3), typeName); - - else if (Token::Match(tstruct->next(), "std :: %type% <")) - { - // backup the type - const std::string typestr(tstruct->strAt(3)); - - // 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(), typestr, typeName); - } - } - } + checkMemsetType(tok, type); } } -void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type) +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &message) { - reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a 'std::" + classname + "'"); + reportError(tok, Severity::error, "memsetClass", message); + // cache the message for this type so we don't have to look it up again + _memsetClassMessages[type] = message; +} + +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind) +{ + memsetError(tok, type, "Using '" + memfunc + "' on " + typekind + " that contains a " + classname); } //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index 2dd238f83..9cd12f499 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -117,7 +117,8 @@ private: void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname); void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname); void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname); - void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type); + void memsetError(const Token *tok, const std::string &type, const std::string &message); + void memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind); void operatorEqReturnError(const Token *tok); void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived); void thisSubtractionError(const Token *tok); @@ -133,7 +134,7 @@ private: c.uninitVarError(0, "classname", "varname"); c.operatorEqVarError(0, "classname", ""); c.unusedPrivateFunctionError(0, "classname", "funcname"); - c.memsetError(0, "memfunc", "classname", "class"); + c.memsetError(0, "type", "memfunc", "classname", "class"); c.operatorEqReturnError(0); //c.virtualDestructorError(0, "Base", "Derived"); c.thisSubtractionError(0); @@ -227,6 +228,10 @@ private: void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); bool canNotCopy(const Scope *scope) const; + + // noMemset helpers + void checkMemsetType(const Token *tok, const std::string &type); + std::map _memsetClassMessages; }; /// @} //--------------------------------------------------------------------------- diff --git a/test/testclass.cpp b/test/testclass.cpp index d49d5abe3..afa2748a2 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2949,6 +2949,56 @@ private: " 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" + " std::string s;\n" + "};\n" + "class Pebbles: public Fred {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + + checkNoMemset("struct Stringy {\n" + " std::string inner;\n" + "};\n" + "struct Foo {\n" + " Stringy s;\n" + "};\n" + "int main() {\n" + " Foo foo;\n" + " memset(&foo, 0, sizeof(Foo));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on struct that contains a 'std::string'\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " virtual ~Fred();\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 virtual method\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + "};\n" + "class Wilma\n" + "{\n" + " virtual ~Wilma();\n" + "};\n" + "class Pebbles: public Fred, Wilma {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:12]: (error) Using 'memset' on class that contains a virtual method\n", errout.str()); } void memsetOnStruct()