walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

CheckClass: Fix the noDestructor warning

This commit is contained in:
Daniel Marjamäki 2018-05-01 15:31:13 +02:00
parent 3ef1627d11
commit 40b6f6b3dd
3 changed files with 58 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
lib/checkclass.cpp
View File

@ -341,12 +341,27 @@ void CheckClass::copyconstructors()
if (!hasCopyCtor) {
bool unknown = false;
if (!isNonCopyable(scope, &unknown))
noCopyConstructorError(scope, unknown);
noCopyConstructorError(scope, allocatedVars.begin()->second, unknown);
}
if (!hasOperatorEq)
noOperatorEqError(scope);
noOperatorEqError(scope, allocatedVars.begin()->second);
if (!hasDestructor) {
// TODO: how are pointers allocated? noDestructorError(scope);
const Token * mustDealloc = nullptr;
for (std::map<unsigned int, const Token*>::const_iterator it = allocatedVars.begin(); it != allocatedVars.end(); ++it) {
if (!Token::Match(it->second, "%var% [(=] new %type%")) {
mustDealloc = it->second;
break;
}
if (it->second->valueType() && it->second->valueType()->isIntegral()) {
mustDealloc = it->second;
break;
}
const Variable *var = it->second->variable();
if (var && var->typeScope() && var->typeScope()->functionList.empty() && var->type()->derivedFrom.empty())
mustDealloc = it->second;
}
if (mustDealloc)
noDestructorError(scope, mustDealloc);
}
}
@ -412,34 +427,32 @@ void CheckClass::copyConstructorShallowCopyError(const Token *tok, const std::st
"$symbol:" + varname + "\nValue of pointer '$symbol', which points to allocated memory, is copied in copy constructor instead of allocating new memory.", CWE398, false);
}
void CheckClass::noCopyConstructorError(const Scope *scope, bool inconclusive)
void CheckClass::noCopyConstructorError(const Scope *scope, const Token *alloc, bool inconclusive)
{
const Token *tok = scope ? scope->classDef : nullptr;
const std::string &classname = scope ? scope->className : "class";
bool isStruct = scope ? (scope->type == Scope::eStruct) : false;
reportError(tok, Severity::style, "noCopyConstructor",
reportError(alloc, Severity::style, "noCopyConstructor",
"$symbol:" + classname + "\n" +
std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.", CWE398, inconclusive);
std::string(isStruct ? "Struct" : "Class") + " '$symbol' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.", CWE398, inconclusive);
}
void CheckClass::noOperatorEqError(const Scope *scope)
void CheckClass::noOperatorEqError(const Scope *scope, const Token *alloc)
{
const Token *tok = scope ? scope->classDef : nullptr;
const std::string &classname = scope ? scope->className : "class";
bool isStruct = scope ? (scope->type == Scope::eStruct) : false;
reportError(tok, Severity::style, "noOperatorEq",
reportError(alloc, Severity::style, "noOperatorEq",
"$symbol:" + classname + "\n" +
std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a assignment operator which is recommended since the class contains a pointer to allocated memory.", CWE398, false);
std::string(isStruct ? "Struct" : "Class") + " '$symbol' does not have a assignment operator but it has dynamic memory/resource allocation. It is recommended to delete or define the assignment operator.", CWE398, false);
}
void CheckClass::noDestructorError(const Scope *scope)
void CheckClass::noDestructorError(const Scope *scope, const Token *alloc)
{
const Token *tok = scope ? scope->classDef : nullptr;
const std::string &classname = scope ? scope->className : "class";
bool isStruct = scope ? (scope->type == Scope::eStruct) : false;
reportError(tok, Severity::style, "noDestructor",
reportError(alloc, Severity::style, "noDestructor",
"$symbol:" + classname + "\n" +
std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a destructor which is recommended since the class contains a pointer to allocated memory.", CWE398, false);
std::string(isStruct ? "Struct" : "Class") + " '$symbol' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.", CWE398, false);
}
bool CheckClass::canNotCopy(const Scope *scope)

12
lib/checkclass.h
View File

@ -168,9 +168,9 @@ private:
void noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct);
//void copyConstructorMallocError(const Token *cctor, const Token *alloc, const std::string& var_name);
void copyConstructorShallowCopyError(const Token *tok, const std::string& varname);
void noCopyConstructorError(const Scope *scope, bool inconclusive);
void noOperatorEqError(const Scope *scope);
void noDestructorError(const Scope *scope);
void noCopyConstructorError(const Scope *scope, const Token *alloc, bool inconclusive);
void noOperatorEqError(const Scope *scope, const Token *alloc);
void noDestructorError(const Scope *scope, const Token *alloc);
void uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool inconclusive);
void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive);
void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname);
@ -204,9 +204,9 @@ private:
c.noExplicitConstructorError(nullptr, "classname", false);
//c.copyConstructorMallocError(nullptr, 0, "var");
c.copyConstructorShallowCopyError(nullptr, "var");
c.noCopyConstructorError(nullptr, false);
c.noOperatorEqError(nullptr);
c.noDestructorError(nullptr);
c.noCopyConstructorError(nullptr, nullptr, false);
c.noOperatorEqError(nullptr, nullptr);
c.noDestructorError(nullptr, nullptr);
c.uninitVarError(nullptr, false, "classname", "varname", false);
c.operatorEqVarError(nullptr, "classname", emptyString, false);
c.unusedPrivateFunctionError(nullptr, "classname", "funcname");

30
test/testclass.cpp
View File

@ -700,7 +700,7 @@ private:
" ~F();\n"
" F& operator=(const F&f);\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (style) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:8]: (style) Class 'F' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.\n", errout.str());
checkCopyConstructor("class F\n"
"{\n"
@ -735,7 +735,7 @@ private:
" ~F();\n"
" F& operator=(const F&f);\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (style, inconclusive) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (style, inconclusive) Class 'F' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.\n", errout.str());
checkCopyConstructor("class E { E(E&); };\n" // non-copyable
"class F : E\n"
@ -758,7 +758,7 @@ private:
" ~F();\n"
" F& operator=(const F&f);\n"
"};");
ASSERT_EQUALS("[test.cpp:2]: (style) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (style) Class 'F' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.\n", errout.str());
checkCopyConstructor("class F {\n"
" char *p;\n"
@ -777,7 +777,7 @@ private:
" ~F();\n"
" F& operator=(const F&f);\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (style) class 'F' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Class 'F' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.\n", errout.str());
// #7198
checkCopyConstructor("struct F {\n"
@ -814,7 +814,7 @@ private:
" F(const F &f);\n"
" ~F();\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (style) struct 'F' does not have a assignment operator which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a assignment operator but it has dynamic memory/resource allocation. It is recommended to delete or define the assignment operator.\n", errout.str());
}
void noDestructor() {
@ -824,7 +824,25 @@ private:
" F(const F &f);\n"
" F&operator=(const F&);"
"};");
// TODO ASSERT_EQUALS("[test.cpp:1]: (style) struct 'F' does not have a destructor which is recommended since the class contains a pointer to allocated memory.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.\n", errout.str());
checkCopyConstructor("struct F {\n"
" C* c;\n"
" F() { c = new C; }\n"
" F(const F &f);\n"
" F&operator=(const F&);"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyConstructor("struct Data { int x; int y; };\n"
"struct F {\n"
" Data* c;\n"
" F() { c = new Data; }\n"
" F(const F &f);\n"
" F&operator=(const F&);"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) Struct 'F' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.\n", errout.str());
}
// Check the operator Equal