From 4d9a1427b29df127ebe2d3c035b3c3b58330268e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 2 Feb 2022 20:44:10 +0100 Subject: [PATCH] CheckClass: Write separate errorid for missing member copy instead of uninitMember --- lib/checkclass.cpp | 39 +++++++++++++++++++++++++-------------- lib/checkclass.h | 12 +++++++----- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 942f6c020..d5e13004e 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -239,14 +239,17 @@ void CheckClass::constructors() } } - bool inconclusive = false; + // Is there missing member copy in copy/move constructor or assignment operator? + bool missingCopy = false; + // Don't warn about unknown types in copy constructors since we // don't know if they can be copied or not.. - if ((func.type == Function::eCopyConstructor || func.type == Function::eMoveConstructor || func.type == Function::eOperatorEqual) && !isVariableCopyNeeded(var)) - inconclusive = true; + if ((func.type == Function::eCopyConstructor || func.type == Function::eMoveConstructor || func.type == Function::eOperatorEqual) && !isVariableCopyNeeded(var)) { + if (!printInconclusive) + continue; - if (!printInconclusive && inconclusive) - continue; + missingCopy = true; + } // It's non-static and it's not initialized => error if (func.type == Function::eOperatorEqual) { @@ -261,7 +264,7 @@ void CheckClass::constructors() } if (classNameUsed) - operatorEqVarError(func.token, scope->className, var.name(), inconclusive); + operatorEqVarError(func.token, scope->className, var.name(), missingCopy); } else if (func.access != AccessControl::Private || mSettings->standards.cpp >= Standards::CPP11) { // If constructor is not in scope then we maybe using a constructor from a different template specialization if (!precedes(scope->bodyStart, func.tokenDef)) @@ -276,9 +279,11 @@ void CheckClass::constructors() func.functionScope->bodyStart->link() == func.functionScope->bodyStart->next()) { // don't warn about user defined default constructor when there are other constructors if (printInconclusive) - uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, true); - } else - uninitVarError(func.token, func.access == AccessControl::Private, func.type, var.scope()->className, var.name(), derived, inconclusive); + uninitVarError(func.token, func.access == AccessControl::Private, var.scope()->className, var.name(), derived, true); + } else if (missingCopy) + missingMemberCopyError(func.token, var.scope()->className, var.name()); + else + uninitVarError(func.token, func.access == AccessControl::Private, var.scope()->className, var.name(), derived, false); } } } @@ -980,19 +985,25 @@ void CheckClass::noExplicitConstructorError(const Token *tok, const std::string reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, Certainty::normal); } -void CheckClass::uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive) +void CheckClass::uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool 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."; + message = "Member variable '$symbol' is not initialized in the constructor."; if (derived) message += " Maybe it should be initialized directly in the class " + classname + "?"; std::string id = std::string("uninit") + (derived ? "Derived" : "") + "MemberVar" + (isprivate ? "Private" : ""); reportError(tok, Severity::warning, id, "$symbol:" + classname + "::" + varname + "\n" + message, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); } +void CheckClass::missingMemberCopyError(const Token *tok, const std::string& classname, const std::string& varname) +{ + const std::string message = + "$symbol:" + classname + "::" + varname + "\n" + + "Member variable '$symbol' is not assigned in the copy constructor. Should it be copied?"; + const char id[] = "missingMemberCopy"; + reportError(tok, Severity::warning, id, message, CWE398, Certainty::inconclusive); +} + void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { reportError(tok, Severity::warning, "operatorEqVarError", "$symbol:" + classname + "::" + varname + "\nMember variable '$symbol' is not assigned a value in '" + classname + "::operator='.", CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); diff --git a/lib/checkclass.h b/lib/checkclass.h index 8c4b01131..f98881f70 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -199,7 +199,8 @@ private: 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 noDestructorError(const Scope *scope, bool isdefault, const Token *alloc); - void uninitVarError(const Token *tok, bool isprivate, Function::Type functionType, const std::string &classname, const std::string &varname, bool derived, bool inconclusive); + void uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool derived, bool inconclusive); + void missingMemberCopyError(const Token *tok, const std::string& classname, const std::string& varname); 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 memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type); @@ -236,10 +237,11 @@ private: c.noCopyConstructorError(nullptr, false, nullptr, false); c.noOperatorEqError(nullptr, false, nullptr, false); c.noDestructorError(nullptr, false, nullptr); - c.uninitVarError(nullptr, false, Function::eConstructor, "classname", "varname", false, false); - c.uninitVarError(nullptr, true, Function::eConstructor, "classname", "varnamepriv", false, false); - c.uninitVarError(nullptr, false, Function::eConstructor, "classname", "varname", true, false); - c.uninitVarError(nullptr, true, Function::eConstructor, "classname", "varnamepriv", true, false); + c.uninitVarError(nullptr, false, "classname", "varname", false, false); + c.uninitVarError(nullptr, true, "classname", "varnamepriv", false, false); + c.uninitVarError(nullptr, false, "classname", "varname", true, false); + c.uninitVarError(nullptr, true, "classname", "varnamepriv", true, false); + c.missingMemberCopyError(nullptr, "classname", "varnamepriv"); c.operatorEqVarError(nullptr, "classname", emptyString, false); c.unusedPrivateFunctionError(nullptr, "classname", "funcname"); c.memsetError(nullptr, "memfunc", "classname", "class");