From d16f35facc5c59ea6ccfe1e55b942e90c2fb2948 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 4 Mar 2015 10:55:32 +0100 Subject: [PATCH 1/4] CModule: use member initialization lists [-Weffc++] (#270) This fixes the problem that CModule::GetType() returned a random uninitialized value in CModule constructor, which was als the reason for #905. CModule constructor signature has been changed so that it optionally takes the type so it can be initialized appropriately. The new type argument has a default value in order to retain source compatibility in case some 3rdparty module would call CModule ctor by hand instead of using the MODCONSTRUCTOR macro. --- include/znc/Modules.h | 49 +++++++++++++++++++++++-------------------- src/Modules.cpp | 34 ++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/include/znc/Modules.h b/include/znc/Modules.h index 2c915367..9eeb9ecc 100644 --- a/include/znc/Modules.h +++ b/include/znc/Modules.h @@ -49,16 +49,6 @@ class CModInfo; #endif #endif -typedef void* ModHandle; - -template void TModInfo(CModInfo& Info) {} - -template CModule* TModLoad(ModHandle p, CUser* pUser, - CIRCNetwork* pNetwork, const CString& sModName, - const CString& sModPath) { - return new M(p, pUser, pNetwork, sModName, sModPath); -} - #if HAVE_VISIBILITY # define MODULE_EXPORT __attribute__((__visibility__("default"))) #else @@ -97,8 +87,8 @@ template CModule* TModLoad(ModHandle p, CUser* pUser, */ #define MODCONSTRUCTOR(CLASS) \ CLASS(ModHandle pDLL, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, \ - const CString& sModPath) \ - : CModule(pDLL, pUser, pNetwork, sModName, sModPath) + const CString& sModPath, CModInfo::EModuleType eType) \ + : CModule(pDLL, pUser, pNetwork, sModName, sModPath, eType) // User Module Macros /** This works exactly like MODULEDEFS, but for user modules. */ @@ -209,25 +199,30 @@ protected: }; #endif +typedef void* ModHandle; + class CModInfo { public: - typedef CModule* (*ModLoader)(ModHandle p, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sModPath); - typedef enum { GlobalModule, UserModule, NetworkModule } EModuleType; - CModInfo() { - m_fLoader = nullptr; - m_bHasArgs = false; + typedef CModule* (*ModLoader)(ModHandle p, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sModPath, EModuleType eType); + + CModInfo() : CModInfo("", "", NetworkModule) { } - CModInfo(const CString& sName, const CString& sPath, EModuleType eType) { - m_sName = sName; - m_sPath = sPath; - m_fLoader = nullptr; - m_bHasArgs = false; + CModInfo(const CString& sName, const CString& sPath, EModuleType eType) + : m_seType(), + m_eDefaultType(eType), + m_sName(sName), + m_sPath(sPath), + m_sDescription(""), + m_sWikiPage(""), + m_sArgsHelpText(""), + m_bHasArgs(false), + m_fLoader(nullptr) { } ~CModInfo() {} @@ -286,6 +281,14 @@ protected: ModLoader m_fLoader; }; +template void TModInfo(CModInfo& Info) {} + +template CModule* TModLoad(ModHandle p, CUser* pUser, + CIRCNetwork* pNetwork, const CString& sModName, + const CString& sModPath, CModInfo::EModuleType eType) { + return new M(p, pUser, pNetwork, sModName, sModPath, eType); +} + /** A helper class for handling commands in modules. */ class CModCommand { public: @@ -356,7 +359,7 @@ private: class CModule { public: CModule(ModHandle pDLL, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, - const CString& sDataDir); + const CString& sDataDir, CModInfo::EModuleType eType = CModInfo::NetworkModule); // TODO: remove default value in ZNC 2.x virtual ~CModule(); CModule(const CModule&) = delete; diff --git a/src/Modules.cpp b/src/Modules.cpp index 0397a67d..8891eccc 100644 --- a/src/Modules.cpp +++ b/src/Modules.cpp @@ -124,15 +124,28 @@ const CString& CTimer::GetDescription() const { return m_sDescription; } /////////////////// !Timer /////////////////// -CModule::CModule(ModHandle pDLL, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataDir) { - m_pDLL = pDLL; - m_pManager = &(CZNC::Get().GetManager());; - m_pUser = pUser; - m_pNetwork = pNetwork; - m_pClient = nullptr; - m_sModName = sModName; - m_sDataDir = sDataDir; - +CModule::CModule(ModHandle pDLL, CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataDir, CModInfo::EModuleType eType) + : m_eType(eType), + m_sDescription(""), + m_sTimers(), + m_sSockets(), +#ifdef HAVE_PTHREAD + m_sJobs(), +#endif + m_pDLL(pDLL), + m_pManager(&(CZNC::Get().GetManager())), + m_pUser(pUser), + m_pNetwork(pNetwork), + m_pClient(nullptr), + m_sModName(sModName), + m_sDataDir(sDataDir), + m_sSavePath(""), + m_sArgs(""), + m_sModPath(""), + m_mssRegistry(), + m_vSubPages(), + m_mCommands() +{ if (m_pNetwork) { m_sSavePath = m_pNetwork->GetNetworkPath() + "/moddata/" + m_sModName; } else if (m_pUser) { @@ -1023,9 +1036,8 @@ bool CModules::LoadModule(const CString& sModule, const CString& sArgs, CModInfo return false; } - CModule* pModule = Info.GetLoader()(p, pUser, pNetwork, sModule, sDataPath); + CModule* pModule = Info.GetLoader()(p, pUser, pNetwork, sModule, sDataPath, eType); pModule->SetDescription(Info.GetDescription()); - pModule->SetType(eType); pModule->SetArgs(sArgs); pModule->SetModPath(CDir::ChangeDir(CZNC::Get().GetCurPath(), sModPath)); push_back(pModule); From e1ada6c643e93ab28bd1d72e3e6f79a471474c5e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 4 Mar 2015 11:00:34 +0100 Subject: [PATCH 2/4] TDNSTask & CDNSJob: use member intialization lists [-Weffc++] (#270) --- include/znc/Socket.h | 4 ++++ src/Socket.cpp | 6 ------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/znc/Socket.h b/include/znc/Socket.h index 1d63bad9..26b683a5 100644 --- a/include/znc/Socket.h +++ b/include/znc/Socket.h @@ -138,6 +138,8 @@ private: friend class CTDNSMonitorFD; #ifdef HAVE_THREADED_DNS struct TDNSTask { + TDNSTask() : sHostname(""), iPort(0), sSockName(""), iTimeout(0), bSSL(false), sBindhost(""), pcSock(nullptr), bDoneTarget(false), bDoneBind(false), aiTarget(nullptr), aiBind(nullptr) {} + CString sHostname; u_short iPort; CString sSockName; @@ -153,6 +155,8 @@ private: }; class CDNSJob : public CJob { public: + CDNSJob() : sHostname(""), task(nullptr), pManager(nullptr), bBind(false), iRes(0), aiResult(nullptr) {} + CString sHostname; TDNSTask* task; CSockManager* pManager; diff --git a/src/Socket.cpp b/src/Socket.cpp index 6bcf1c1a..f3ce66b8 100644 --- a/src/Socket.cpp +++ b/src/Socket.cpp @@ -208,8 +208,6 @@ void CSockManager::StartTDNSThread(TDNSTask* task, bool bBind) { arg->sHostname = sHostname; arg->task = task; arg->bBind = bBind; - arg->iRes = 0; - arg->aiResult = nullptr; arg->pManager = this; CThreadPool::Get().addJob(arg); @@ -363,13 +361,9 @@ void CSockManager::Connect(const CString& sHostname, u_short iPort, const CStrin task->bSSL = bSSL; task->sBindhost = sBindHost; task->pcSock = pcSock; - task->aiTarget = nullptr; - task->aiBind = nullptr; - task->bDoneTarget = false; if (sBindHost.empty()) { task->bDoneBind = true; } else { - task->bDoneBind = false; StartTDNSThread(task, true); } StartTDNSThread(task, false); From 5aa8b0dcef723578c702affbde72bc5a04853144 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 4 Mar 2015 18:13:32 +0100 Subject: [PATCH 3/4] Fix copy ctor/assignment oper warnings of -Weffc++ (#270) --- include/znc/Query.h | 3 +++ include/znc/Socket.h | 6 ++++++ include/znc/Utils.h | 3 +++ src/IRCNetwork.cpp | 6 ++++++ src/IRCSock.cpp | 2 ++ src/User.cpp | 3 +++ src/WebModules.cpp | 3 +++ 7 files changed, 26 insertions(+) diff --git a/include/znc/Query.h b/include/znc/Query.h index fc4422c3..8627f9bd 100644 --- a/include/znc/Query.h +++ b/include/znc/Query.h @@ -31,6 +31,9 @@ public: CQuery(const CString& sName, CIRCNetwork* pNetwork); ~CQuery(); + CQuery(const CQuery&) = delete; + CQuery& operator=(const CQuery&) = delete; + // Buffer const CBuffer& GetBuffer() const { return m_Buffer; } unsigned int GetBufferCount() const { return m_Buffer.GetLineCount(); } diff --git a/include/znc/Socket.h b/include/znc/Socket.h index 26b683a5..ea87a696 100644 --- a/include/znc/Socket.h +++ b/include/znc/Socket.h @@ -140,6 +140,9 @@ private: struct TDNSTask { TDNSTask() : sHostname(""), iPort(0), sSockName(""), iTimeout(0), bSSL(false), sBindhost(""), pcSock(nullptr), bDoneTarget(false), bDoneBind(false), aiTarget(nullptr), aiBind(nullptr) {} + TDNSTask(const TDNSTask&) = delete; + TDNSTask& operator=(const TDNSTask&) = delete; + CString sHostname; u_short iPort; CString sSockName; @@ -157,6 +160,9 @@ private: public: CDNSJob() : sHostname(""), task(nullptr), pManager(nullptr), bBind(false), iRes(0), aiResult(nullptr) {} + CDNSJob(const CDNSJob&) = delete; + CDNSJob& operator=(const CDNSJob&) = delete; + CString sHostname; TDNSTask* task; CSockManager* pManager; diff --git a/include/znc/Utils.h b/include/znc/Utils.h index a0b4cf8b..d18acb9c 100644 --- a/include/znc/Utils.h +++ b/include/znc/Utils.h @@ -222,6 +222,9 @@ public: CBlowfish(const CString & sPassword, int iEncrypt, const CString & sIvec = ""); ~CBlowfish(); + CBlowfish(const CBlowfish&) = default; + CBlowfish& operator=(const CBlowfish&) = default; + //! output must be freed static unsigned char *MD5(const unsigned char *input, u_int ilen); diff --git a/src/IRCNetwork.cpp b/src/IRCNetwork.cpp index 98ee955d..1948f546 100644 --- a/src/IRCNetwork.cpp +++ b/src/IRCNetwork.cpp @@ -37,6 +37,9 @@ public: virtual ~CIRCNetworkPingTimer() {} + CIRCNetworkPingTimer(const CIRCNetworkPingTimer&) = delete; + CIRCNetworkPingTimer& operator=(const CIRCNetworkPingTimer&) = delete; + protected: void RunJob() override { CIRCSock* pIRCSock = m_pNetwork->GetIRCSock(); @@ -66,6 +69,9 @@ public: virtual ~CIRCNetworkJoinTimer() {} + CIRCNetworkJoinTimer(const CIRCNetworkJoinTimer&) = delete; + CIRCNetworkJoinTimer& operator=(const CIRCNetworkJoinTimer&) = delete; + void Delay(unsigned short int uDelay) { m_bDelayed = true; Start(uDelay); diff --git a/src/IRCSock.cpp b/src/IRCSock.cpp index b1603a2e..9c3748ec 100644 --- a/src/IRCSock.cpp +++ b/src/IRCSock.cpp @@ -41,6 +41,8 @@ class CIRCFloodTimer : public CCron { CIRCFloodTimer(CIRCSock* pSock) : m_pSock(pSock) { StartMaxCycles(m_pSock->m_fFloodRate, 0); } + CIRCFloodTimer(const CIRCFloodTimer&) = delete; + CIRCFloodTimer& operator=(const CIRCFloodTimer&) = delete; void RunJob() override { if (m_pSock->m_iSendsAllowed < m_pSock->m_uFloodBurst) { m_pSock->m_iSendsAllowed++; diff --git a/src/User.cpp b/src/User.cpp index ea5daf2b..151ef9d3 100644 --- a/src/User.cpp +++ b/src/User.cpp @@ -34,6 +34,9 @@ public: } virtual ~CUserTimer() {} + CUserTimer(const CUserTimer&) = delete; + CUserTimer& operator=(const CUserTimer&) = delete; + private: protected: void RunJob() override { diff --git a/src/WebModules.cpp b/src/WebModules.cpp index a1fa26fb..a5c79927 100644 --- a/src/WebModules.cpp +++ b/src/WebModules.cpp @@ -53,6 +53,9 @@ public: CWebAuth(CWebSock* pWebSock, const CString& sUsername, const CString& sPassword, bool bBasic); virtual ~CWebAuth() {} + CWebAuth(const CWebAuth&) = delete; + CWebAuth& operator=(const CWebAuth&) = delete; + void SetWebSock(CWebSock* pWebSock) { m_pWebSock = pWebSock; } void AcceptedLogin(CUser& User) override; void RefusedLogin(const CString& sReason) override; From e62ed5f30037cdb485a7b75f21558c1eae73b183 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Sat, 7 Mar 2015 21:07:35 +0100 Subject: [PATCH 4/4] modperl & modpython: fix GetType() at module construction time --- modules/modperl/module.h | 4 ++-- modules/modperl/startup.pl | 3 +-- modules/modpython/module.h | 8 ++++---- modules/modpython/znc.py | 3 +-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/modules/modperl/module.h b/modules/modperl/module.h index 92bb658e..fc797c1c 100644 --- a/modules/modperl/module.h +++ b/modules/modperl/module.h @@ -30,8 +30,8 @@ class CPerlModule : public CModule { VWebSubPages* _GetSubPages(); public: CPerlModule(CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataPath, - SV* perlObj) - : CModule(nullptr, pUser, pNetwork, sModName, sDataPath) { + CModInfo::EModuleType eType, SV* perlObj) + : CModule(nullptr, pUser, pNetwork, sModName, sDataPath, eType) { m_perlObj = newSVsv(perlObj); } SV* GetPerlObj() { diff --git a/modules/modperl/startup.pl b/modules/modperl/startup.pl index 55d095ea..0a5e0f45 100644 --- a/modules/modperl/startup.pl +++ b/modules/modperl/startup.pl @@ -111,7 +111,7 @@ sub LoadModule { $modrefcount{$modname}++; $datapath = $datapath->GetPerlStr; $datapath =~ s/\.pm$//; - my $cmod = ZNC::CPerlModule->new($user, $network, $modname, $datapath, $pmod); + my $cmod = ZNC::CPerlModule->new($user, $network, $modname, $datapath, $type, $pmod); my %nv; tie %nv, 'ZNC::ModuleNV', $cmod; $pmod->{_cmod} = $cmod; @@ -119,7 +119,6 @@ sub LoadModule { $cmod->SetDescription($pmod->description); $cmod->SetArgs($args); $cmod->SetModPath($modpath); - $cmod->SetType($type); push @allmods, $pmod; $container->push_back($cmod); my $x = ''; diff --git a/modules/modpython/module.h b/modules/modpython/module.h index 0a3e2b81..caa40f4f 100644 --- a/modules/modpython/module.h +++ b/modules/modpython/module.h @@ -32,8 +32,8 @@ class CPyModule : public CModule { VWebSubPages* _GetSubPages(); public: CPyModule(CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataPath, - PyObject* pyObj, CModPython* pModPython) - : CModule(nullptr, pUser, pNetwork, sModName, sDataPath) { + CModInfo::EModuleType eType, PyObject* pyObj, CModPython* pModPython) + : CModule(nullptr, pUser, pNetwork, sModName, sDataPath, eType) { m_pyObj = pyObj; Py_INCREF(pyObj); m_pModPython = pModPython; @@ -145,8 +145,8 @@ static inline CPyModule* AsPyModule(CModule* p) { return dynamic_cast(p); } -inline CPyModule* CreatePyModule(CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataPath, PyObject* pyObj, CModPython* pModPython) { - return new CPyModule(pUser, pNetwork, sModName, sDataPath, pyObj, pModPython); +inline CPyModule* CreatePyModule(CUser* pUser, CIRCNetwork* pNetwork, const CString& sModName, const CString& sDataPath, CModInfo::EModuleType eType, PyObject* pyObj, CModPython* pModPython) { + return new CPyModule(pUser, pNetwork, sModName, sDataPath, eType, pyObj, pModPython); } class CPyTimer : public CTimer { diff --git a/modules/modpython/znc.py b/modules/modpython/znc.py index cd4de4b3..0b6a1217 100644 --- a/modules/modpython/znc.py +++ b/modules/modpython/znc.py @@ -538,12 +538,11 @@ def load_module(modname, args, module_type, user, network, retmsg, modpython): return 1 module = cl() - module._cmod = CreatePyModule(user, network, modname, datapath, module, modpython) + module._cmod = CreatePyModule(user, network, modname, datapath, module_type, module, modpython) module.nv = ModuleNV(module._cmod) module.SetDescription(cl.description) module.SetArgs(args) module.SetModPath(pymodule.__file__) - module.SetType(module_type) _py_modules.add(module) if module_type == CModInfo.UserModule: