Clarify inconclusive uninitMemberVar warning in copy constructors, it's in general a good idea to copy all the data.

This commit is contained in:
Daniel Marjamäki 2020-08-05 13:57:40 +02:00
parent 7a85b8e452
commit d5b64d9c3a
3 changed files with 18 additions and 12 deletions

View File

@ -239,9 +239,9 @@ void CheckClass::constructors()
func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) { func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) {
// don't warn about user defined default constructor when there are other constructors // don't warn about user defined default constructor when there are other constructors
if (printInconclusive) if (printInconclusive)
uninitVarError(func.token, func.access == AccessControl::Private, scope->className, var.name(), true); uninitVarError(func.token, func.access == AccessControl::Private, func.type, scope->className, var.name(), true);
} else } else
uninitVarError(func.token, func.access == AccessControl::Private, scope->className, var.name(), inconclusive); uninitVarError(func.token, func.access == AccessControl::Private, func.type, scope->className, var.name(), inconclusive);
} }
} }
} }
@ -921,9 +921,14 @@ void CheckClass::noExplicitConstructorError(const Token *tok, const std::string
reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, false); reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, false);
} }
void CheckClass::uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool inconclusive) void CheckClass::uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, const std::string &classname, const std::string &varname, bool inconclusive)
{ {
reportError(tok, Severity::warning, isprivate ? "uninitMemberVarPrivate" : "uninitMemberVar", "$symbol:" + classname + "::" + varname + "\nMember variable '$symbol' is not initialized in the constructor.", CWE398, inconclusive); std::string message;
if ((functionType == Function::eCopyConstructor || functionType == Function::eMoveConstructor) && inconclusive)
message = "Member variable '$symbol' is not assigned in the copy constructor. Should it be copied?";
else
message = "Member variable '$symbol' is not initialized in the constructor.";
reportError(tok, Severity::warning, isprivate ? "uninitMemberVarPrivate" : "uninitMemberVar", "$symbol:" + classname + "::" + varname + "\n" + message, CWE398, inconclusive);
} }
void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive)

View File

@ -24,6 +24,7 @@
#include "check.h" #include "check.h"
#include "config.h" #include "config.h"
#include "tokenize.h" #include "tokenize.h"
#include "symboldatabase.h"
#include <list> #include <list>
#include <map> #include <map>
@ -158,7 +159,7 @@ private:
void noCopyConstructorError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive); void noCopyConstructorError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive);
void noOperatorEqError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive); void noOperatorEqError(const Scope *scope, bool isdefault, const Token *alloc, bool inconclusive);
void noDestructorError(const Scope *scope, bool isdefault, const Token *alloc); void noDestructorError(const Scope *scope, bool isdefault, const Token *alloc);
void uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool inconclusive); void uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, 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 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); 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 &memfunc, const std::string &classname, const std::string &type);
@ -194,8 +195,8 @@ private:
c.noCopyConstructorError(nullptr, false, nullptr, false); c.noCopyConstructorError(nullptr, false, nullptr, false);
c.noOperatorEqError(nullptr, false, nullptr, false); c.noOperatorEqError(nullptr, false, nullptr, false);
c.noDestructorError(nullptr, false, nullptr); c.noDestructorError(nullptr, false, nullptr);
c.uninitVarError(nullptr, false, "classname", "varname", false); c.uninitVarError(nullptr, false, Function::eConstructor, "classname", "varname", false);
c.uninitVarError(nullptr, true, "classname", "varnamepriv", false); c.uninitVarError(nullptr, true, Function::eConstructor, "classname", "varnamepriv", false);
c.operatorEqVarError(nullptr, "classname", emptyString, false); c.operatorEqVarError(nullptr, "classname", emptyString, false);
c.unusedPrivateFunctionError(nullptr, "classname", "funcname"); c.unusedPrivateFunctionError(nullptr, "classname", "funcname");
c.memsetError(nullptr, "memfunc", "classname", "class"); c.memsetError(nullptr, "memfunc", "classname", "class");

View File

@ -1223,7 +1223,7 @@ private:
" Fred() { };\n" " Fred() { };\n"
" Fred(const Fred &) { };\n" " Fred(const Fred &) { };\n"
"};", true); "};", true);
ASSERT_EQUALS("[test.cpp:7]: (warning, inconclusive) Member variable 'Fred::var' is not initialized in the constructor.\n", errout.str()); ASSERT_EQUALS("[test.cpp:7]: (warning, inconclusive) Member variable 'Fred::var' is not assigned in the copy constructor. Should it be copied?\n", errout.str());
check("class Fred\n" check("class Fred\n"
"{\n" "{\n"
@ -1235,7 +1235,7 @@ private:
"};\n" "};\n"
"Fred::Fred() { };\n" "Fred::Fred() { };\n"
"Fred::Fred(const Fred &) { };\n", true); "Fred::Fred(const Fred &) { };\n", true);
ASSERT_EQUALS("[test.cpp:10]: (warning, inconclusive) Member variable 'Fred::var' is not initialized in the constructor.\n", errout.str()); ASSERT_EQUALS("[test.cpp:10]: (warning, inconclusive) Member variable 'Fred::var' is not assigned in the copy constructor. Should it be copied?\n", errout.str());
} }
void initvar_nested_constructor() { // ticket #1375 void initvar_nested_constructor() { // ticket #1375
@ -1409,8 +1409,8 @@ private:
" B(B &&){}\n" " B(B &&){}\n"
" const B& operator=(const B&){return *this;}\n" " const B& operator=(const B&){return *this;}\n"
"};", true); "};", true);
ASSERT_EQUALS("[test.cpp:11]: (warning, inconclusive) Member variable 'B::a' is not initialized in the constructor.\n" ASSERT_EQUALS("[test.cpp:11]: (warning, inconclusive) Member variable 'B::a' is not assigned in the copy constructor. Should it be copied?\n"
"[test.cpp:12]: (warning, inconclusive) Member variable 'B::a' is not initialized in the constructor.\n" "[test.cpp:12]: (warning, inconclusive) Member variable 'B::a' is not assigned in the copy constructor. Should it be copied?\n"
"[test.cpp:13]: (warning, inconclusive) Member variable 'B::a' is not assigned a value in 'B::operator='.\n", "[test.cpp:13]: (warning, inconclusive) Member variable 'B::a' is not assigned a value in 'B::operator='.\n",
errout.str()); errout.str());
@ -1510,7 +1510,7 @@ private:
" A() {}\n" " A() {}\n"
" A(const A& rhs) {}\n" " A(const A& rhs) {}\n"
"};", true); "};", true);
ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Member variable 'A::b' is not initialized in the constructor.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Member variable 'A::b' is not assigned in the copy constructor. Should it be copied?\n", errout.str());
} }
void initvar_with_member_function_this() { void initvar_with_member_function_this() {