From 9db7af579c46a9f0379fdf71fb773a0a76a94846 Mon Sep 17 00:00:00 2001 From: danieldg Date: Sat, 17 Oct 2009 18:52:39 +0000 Subject: [PATCH] Make classbase and refcountbase uncopyable; expand comments on their indended uses git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@11888 e03df62e-2008-0410-955e-edbf42e46eb7 --- include/base.h | 42 ++++++++++++++++------ include/channels.h | 2 +- include/dns.h | 6 ++-- include/extensible.h | 1 + include/hashcomp.h | 10 +++--- include/inspsocket.h | 2 +- include/mode.h | 2 +- include/modules.h | 6 ++-- include/users.h | 2 +- src/base.cpp | 16 +++++---- src/cull_list.cpp | 4 +-- src/inspsocket.cpp | 4 +-- src/mode.cpp | 9 +++-- src/modules.cpp | 4 +-- src/modules/m_alias.cpp | 2 +- src/modules/m_banredirect.cpp | 2 +- src/modules/m_cgiirc.cpp | 2 +- src/modules/m_dccallow.cpp | 4 +-- src/modules/m_filter.cpp | 2 +- src/modules/m_httpd_acl.cpp | 3 +- src/modules/m_httpd_stats.cpp | 2 +- src/modules/m_spanningtree/main.cpp | 2 +- src/modules/m_spanningtree/main.h | 2 +- src/modules/m_spanningtree/treesocket.h | 2 +- src/modules/m_spanningtree/treesocket1.cpp | 2 +- src/modules/m_spanningtree/utils.cpp | 8 ++--- src/modules/m_spanningtree/utils.h | 2 +- src/modules/m_sqlv2.h | 11 ++++-- src/modules/m_timedbans.cpp | 2 +- src/modules/u_listmode.h | 4 +-- src/users.cpp | 6 ++-- 31 files changed, 101 insertions(+), 67 deletions(-) diff --git a/include/base.h b/include/base.h index ed03eeac5..624e2174f 100644 --- a/include/base.h +++ b/include/base.h @@ -18,11 +18,17 @@ #include #include -/** The base class for all inspircd classes. - * Wherever possible, all classes you create should inherit from this, - * giving them the ability to be passed to various core functions - * as 'anonymous' classes. -*/ +/** Dummy class to help enforce culls being parent-called up to classbase */ +class CullResult +{ + CullResult(); + friend class classbase; +}; + +/** The base class for all inspircd classes with a well-defined lifetime. + * Classes that inherit from this may be destroyed through GlobalCulls, + * and may rely on cull() being called prior to their deletion. + */ class CoreExport classbase { public: @@ -33,23 +39,39 @@ class CoreExport classbase * * @return true to allow the delete, or false to halt the delete */ - virtual bool cull(); + virtual CullResult cull(); virtual ~classbase(); + private: + // uncopyable + classbase(const classbase&); + void operator=(const classbase&); }; /** The base class for inspircd classes that support reference counting. * Any objects that do not have a well-defined lifetime should inherit from - * this + * this, and should be assigned to a reference object to establish their + * lifetime. + * + * Reference objects should not hold circular references back to themselves, + * even indirectly; this will cause a memory leak because the count will never + * drop to zero. + * + * Using a normal pointer for the object is recommended if you can assure that + * at least one reference<> will remain as long as that pointer is used; this + * will avoid the slight overhead of changing the reference count. */ -class CoreExport refcountbase : public classbase +class CoreExport refcountbase { unsigned int refcount; public: refcountbase(); - virtual bool cull(); virtual ~refcountbase(); inline unsigned int GetReferenceCount() const { return refcount; } friend class reference_base; + private: + // uncopyable + refcountbase(const refcountbase&); + void operator=(const refcountbase&); }; class CoreExport reference_base @@ -81,7 +103,7 @@ class reference : public reference_base if (value) { int rc = dec(value); - if (rc == 0 && value->cull()) + if (rc == 0) delete value; } } diff --git a/include/channels.h b/include/channels.h index 1ded25a73..7728bb55b 100644 --- a/include/channels.h +++ b/include/channels.h @@ -36,7 +36,7 @@ struct ModResult; /** Holds an entry for a ban list, exemption list, or invite list. * This class contains a single element in a channel list, such as a banlist. */ -class HostItem : public classbase +class HostItem { public: /** Time the item was added diff --git a/include/dns.h b/include/dns.h index 76a69b6e4..1b9ce59a4 100644 --- a/include/dns.h +++ b/include/dns.h @@ -40,7 +40,7 @@ class Module; /** * Result status, used internally */ -class CoreExport DNSResult : public classbase +class CoreExport DNSResult { public: /** Result ID @@ -72,7 +72,7 @@ typedef std::pair DNSInfo; /** Cached item stored in the query cache. */ -class CoreExport CachedQuery : public classbase +class CoreExport CachedQuery { public: /** The cached result data, an IP or hostname @@ -181,7 +181,7 @@ enum ForceProtocol * can occur by calling virtual methods, one is a success situation, and the other * an error situation. */ -class CoreExport Resolver : public Extensible +class CoreExport Resolver { protected: /** diff --git a/include/extensible.h b/include/extensible.h index ff7bce477..ca9c44546 100644 --- a/include/extensible.h +++ b/include/extensible.h @@ -72,6 +72,7 @@ class CoreExport Extensible : public classbase */ inline const ExtensibleStore& GetExtList() const { return extensions; } + virtual CullResult cull(); virtual ~Extensible(); void doUnhookExtensions(const std::vector& toRemove); }; diff --git a/include/hashcomp.h b/include/hashcomp.h index 6cbc14850..5392c6ae5 100644 --- a/include/hashcomp.h +++ b/include/hashcomp.h @@ -204,7 +204,7 @@ namespace irc * std::string, or a const char* const* array, using overloaded * constructors. */ - class CoreExport stringjoiner : public classbase + class CoreExport stringjoiner { private: @@ -248,7 +248,7 @@ namespace irc * It can then reproduce this list, clamped to a maximum of MAXMODES * values per line. */ - class CoreExport modestacker : public classbase + class CoreExport modestacker { private: /** The mode sequence and its parameters @@ -335,7 +335,7 @@ namespace irc * list will be ":item". This is to allow for parsing 'source' fields * from data. */ - class CoreExport tokenstream : public classbase + class CoreExport tokenstream { private: @@ -394,7 +394,7 @@ namespace irc * the next token, until none remain, at which point the method returns * an empty string. */ - class CoreExport sepstream : public classbase + class CoreExport sepstream { private: /** Original string. @@ -467,7 +467,7 @@ namespace irc * start or end < 0) then GetToken() will return the first element * of the pair of numbers. */ - class CoreExport portparser : public classbase + class CoreExport portparser { private: diff --git a/include/inspsocket.h b/include/inspsocket.h index 84716eae6..3e5c75235 100644 --- a/include/inspsocket.h +++ b/include/inspsocket.h @@ -146,7 +146,7 @@ class CoreExport StreamSocket : public EventHandler */ virtual void Close(); /** This ensures that close is called prior to destructor */ - virtual bool cull(); + virtual CullResult cull(); }; /** * BufferedSocket is an extendable socket class which modules diff --git a/include/mode.h b/include/mode.h index 81858a823..b63e5e6a9 100644 --- a/include/mode.h +++ b/include/mode.h @@ -172,7 +172,7 @@ class CoreExport ModeHandler : public classbase * @param type Type of the mode (MODETYPE_USER or MODETYPE_CHANNEL) */ ModeHandler(Module* me, const std::string& name, char modeletter, ParamSpec params, ModeType type); - virtual bool cull(); + virtual CullResult cull(); virtual ~ModeHandler(); /** * Returns true if the mode is a list mode diff --git a/include/modules.h b/include/modules.h index ef6f5c251..96506f598 100644 --- a/include/modules.h +++ b/include/modules.h @@ -230,7 +230,7 @@ do { \ * error when attempting to load a module compiled against a different API_VERSION. */ template -class CoreExport VersionBase : public classbase +class CoreExport VersionBase { public: /** Module description @@ -349,7 +349,7 @@ class ConfigReader; * its methods will be called when irc server events occur. class inherited from module must be * instantiated by the ModuleFactory class (see relevent section) for the module to be initialised. */ -class CoreExport Module : public Extensible +class CoreExport Module : public classbase { public: /** File that this module was loaded from @@ -369,7 +369,7 @@ class CoreExport Module : public Extensible /** Clean up prior to destruction * If you override, you must call this AFTER your module's cleanup */ - virtual bool cull(); + virtual CullResult cull(); /** Default destructor. * destroys a module class diff --git a/include/users.h b/include/users.h index ed672450e..0ce9f59ab 100644 --- a/include/users.h +++ b/include/users.h @@ -848,7 +848,7 @@ class CoreExport User : public StreamSocket /** Default destructor */ virtual ~User(); - virtual bool cull(); + virtual CullResult cull(); }; /** Derived from Resolver, and performs user forward/reverse lookups. diff --git a/src/base.cpp b/src/base.cpp index 1b01da707..2e2dbfb28 100644 --- a/src/base.cpp +++ b/src/base.cpp @@ -22,22 +22,21 @@ classbase::classbase() { } -bool classbase::cull() +CullResult classbase::cull() { - return true; + return CullResult(); } classbase::~classbase() { } -refcountbase::refcountbase() : refcount(0) +CullResult::CullResult() { } -bool refcountbase::cull() +refcountbase::refcountbase() : refcount(0) { - return (refcount == 0); } refcountbase::~refcountbase() @@ -129,12 +128,17 @@ void Extensible::doUnhookExtensions(const std::vector& toRemove) } } -Extensible::~Extensible() +CullResult Extensible::cull() { for(ExtensibleStore::iterator i = extensions.begin(); i != extensions.end(); ++i) { i->first->free(i->second); } + return classbase::cull(); +} + +Extensible::~Extensible() +{ } LocalExtItem::LocalExtItem(const std::string& Key, Module* mod) : ExtensionItem(Key, mod) diff --git a/src/cull_list.cpp b/src/cull_list.cpp index 6033ec695..1ce6dfae1 100644 --- a/src/cull_list.cpp +++ b/src/cull_list.cpp @@ -26,8 +26,8 @@ void CullList::Apply() { ServerInstance->Logs->Log("CULLLIST", DEBUG, "Deleting %s @%p", typeid(*c).name(), (void*)c); - if (c->cull()) - queue.push_back(c); + c->cull(); + queue.push_back(c); } else { diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp index 37335c855..fc8bc142f 100644 --- a/src/inspsocket.cpp +++ b/src/inspsocket.cpp @@ -145,10 +145,10 @@ void StreamSocket::Close() errno = save; } -bool StreamSocket::cull() +CullResult StreamSocket::cull() { Close(); - return true; + return EventHandler::cull(); } bool StreamSocket::GetNextLine(std::string& line, char delim) diff --git a/src/mode.cpp b/src/mode.cpp index 144ddee72..47553f238 100644 --- a/src/mode.cpp +++ b/src/mode.cpp @@ -53,10 +53,10 @@ ModeHandler::ModeHandler(Module* Creator, const std::string& Name, char modelett { } -bool ModeHandler::cull() +CullResult ModeHandler::cull() { ServerInstance->Modes->DelMode(this); - return true; + return classbase::cull(); } ModeHandler::~ModeHandler() @@ -1027,6 +1027,9 @@ ModeParser::ModeParser() ModeParser::~ModeParser() { ModeHandler* mh = ServerInstance->Modes->FindMode('h', MODETYPE_CHANNEL); - if (mh && mh->cull()) + if (mh) + { + mh->cull(); delete mh; + } } diff --git a/src/modules.cpp b/src/modules.cpp index 913293e13..2dafc8864 100644 --- a/src/modules.cpp +++ b/src/modules.cpp @@ -52,9 +52,9 @@ void Event::Send() // These declarations define the behavours of the base class Module (which does nothing at all) Module::Module() { } -bool Module::cull() +CullResult Module::cull() { - return true; + return classbase::cull(); } Module::~Module() { } diff --git a/src/modules/m_alias.cpp b/src/modules/m_alias.cpp index 6aa1f6c36..62c02396a 100644 --- a/src/modules/m_alias.cpp +++ b/src/modules/m_alias.cpp @@ -17,7 +17,7 @@ /** An alias definition */ -class Alias : public classbase +class Alias { public: /** The text of the alias command */ diff --git a/src/modules/m_banredirect.cpp b/src/modules/m_banredirect.cpp index fdc5b1220..cf6c27828 100644 --- a/src/modules/m_banredirect.cpp +++ b/src/modules/m_banredirect.cpp @@ -20,7 +20,7 @@ /* Originally written by Om, January 2009 */ -class BanRedirectEntry : public classbase +class BanRedirectEntry { public: std::string targetchan; diff --git a/src/modules/m_cgiirc.cpp b/src/modules/m_cgiirc.cpp index 04e35adcb..53c761b3e 100644 --- a/src/modules/m_cgiirc.cpp +++ b/src/modules/m_cgiirc.cpp @@ -26,7 +26,7 @@ enum CGItype { INVALID, PASS, IDENT, PASSFIRST, IDENTFIRST, WEBIRC }; /** Holds a CGI site's details */ -class CGIhost : public classbase +class CGIhost { public: std::string hostmask; diff --git a/src/modules/m_dccallow.cpp b/src/modules/m_dccallow.cpp index c202a97b4..d4be35ceb 100644 --- a/src/modules/m_dccallow.cpp +++ b/src/modules/m_dccallow.cpp @@ -17,14 +17,14 @@ static ConfigReader *Conf; -class BannedFileList : public classbase +class BannedFileList { public: std::string filemask; std::string action; }; -class DCCAllow : public classbase +class DCCAllow { public: std::string nickname; diff --git a/src/modules/m_filter.cpp b/src/modules/m_filter.cpp index a9187098b..0220a3a44 100644 --- a/src/modules/m_filter.cpp +++ b/src/modules/m_filter.cpp @@ -28,7 +28,7 @@ enum FilterFlags FLAG_NOTICE = 16 }; -class FilterResult : public classbase +class FilterResult { public: std::string freeform; diff --git a/src/modules/m_httpd_acl.cpp b/src/modules/m_httpd_acl.cpp index dd6f5a8bb..85ec4b6cd 100644 --- a/src/modules/m_httpd_acl.cpp +++ b/src/modules/m_httpd_acl.cpp @@ -16,9 +16,8 @@ #include "protocol.h" /* $ModDesc: Provides access control lists (passwording of resources, ip restrictions etc) to m_httpd.so dependent modules */ -/* $ModDep: httpd.h */ -class HTTPACL : public Extensible +class HTTPACL { public: std::string path; diff --git a/src/modules/m_httpd_stats.cpp b/src/modules/m_httpd_stats.cpp index a075bc59c..a451f6af9 100644 --- a/src/modules/m_httpd_stats.cpp +++ b/src/modules/m_httpd_stats.cpp @@ -69,7 +69,7 @@ class ModuleHttpStats : public Module void DumpMeta(std::stringstream& data, Extensible* ext) { data << ""; - for(ExtensibleStore::const_iterator i = ext->GetExtList().begin(); i != ext->GetExtList().end(); i++) + for(Extensible::ExtensibleStore::const_iterator i = ext->GetExtList().begin(); i != ext->GetExtList().end(); i++) { ExtensionItem* item = i->first; std::string value = item->serialize(FORMAT_USER, ext, i->second); diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp index afd40e0c9..e46c2d320 100644 --- a/src/modules/m_spanningtree/main.cpp +++ b/src/modules/m_spanningtree/main.cpp @@ -944,7 +944,7 @@ void ModuleSpanningTree::ProtoSendMetaData(void* opaque, Extensible* target, con s->WriteLine(std::string(":")+ServerInstance->Config->GetSID()+" METADATA * "+extname+" :"+extdata); } -bool ModuleSpanningTree::cull() +CullResult ModuleSpanningTree::cull() { Utils->cull(); ServerInstance->Timers->DelTimer(RefreshTimer); diff --git a/src/modules/m_spanningtree/main.h b/src/modules/m_spanningtree/main.h index be9c460d9..692442598 100644 --- a/src/modules/m_spanningtree/main.h +++ b/src/modules/m_spanningtree/main.h @@ -190,7 +190,7 @@ class ModuleSpanningTree : public Module void ProtoSendMetaData(void* opaque, Extensible* target, const std::string &extname, const std::string &extdata); void OnLoadModule(Module* mod); void OnUnloadModule(Module* mod); - bool cull(); + CullResult cull(); ~ModuleSpanningTree(); Version GetVersion(); void Prioritize(); diff --git a/src/modules/m_spanningtree/treesocket.h b/src/modules/m_spanningtree/treesocket.h index a0f0e5d88..b5f97c30f 100644 --- a/src/modules/m_spanningtree/treesocket.h +++ b/src/modules/m_spanningtree/treesocket.h @@ -133,7 +133,7 @@ class TreeSocket : public BufferedSocket */ void CleanNegotiationInfo(); - bool cull(); + CullResult cull(); /** Destructor */ ~TreeSocket(); diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp index d99d8d6c9..f6f237529 100644 --- a/src/modules/m_spanningtree/treesocket1.cpp +++ b/src/modules/m_spanningtree/treesocket1.cpp @@ -103,7 +103,7 @@ void TreeSocket::CleanNegotiationInfo() OutboundPass.clear(); } -bool TreeSocket::cull() +CullResult TreeSocket::cull() { Utils->timeoutlist.erase(this); if (myautoconnect) diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp index 09333fdd2..024605a79 100644 --- a/src/modules/m_spanningtree/utils.cpp +++ b/src/modules/m_spanningtree/utils.cpp @@ -151,7 +151,7 @@ SpanningTreeUtilities::SpanningTreeUtilities(ModuleSpanningTree* C) : Creator(C) this->ReadConfiguration(true); } -bool SpanningTreeUtilities::cull() +CullResult SpanningTreeUtilities::cull() { for (unsigned int i = 0; i < ServerInstance->ports.size(); i++) { @@ -171,9 +171,9 @@ bool SpanningTreeUtilities::cull() } ServerUser->uuid = TreeRoot->GetID(); - if (ServerUser->cull()) - delete ServerUser; - return true; + ServerUser->cull(); + delete ServerUser; + return classbase::cull(); } SpanningTreeUtilities::~SpanningTreeUtilities() diff --git a/src/modules/m_spanningtree/utils.h b/src/modules/m_spanningtree/utils.h index 48677e57d..2fc7304af 100644 --- a/src/modules/m_spanningtree/utils.h +++ b/src/modules/m_spanningtree/utils.h @@ -133,7 +133,7 @@ class SpanningTreeUtilities : public classbase /** Prepare for class destruction */ - bool cull(); + CullResult cull(); /** Destroy class and free listeners etc */ diff --git a/src/modules/m_sqlv2.h b/src/modules/m_sqlv2.h index bcdcb6546..05079a8e6 100644 --- a/src/modules/m_sqlv2.h +++ b/src/modules/m_sqlv2.h @@ -61,7 +61,7 @@ public: * The error string varies from database software to database software * and should be used to display informational error messages to users. */ -class SQLerror : public classbase +class SQLerror { /** The error id */ @@ -149,7 +149,7 @@ public: * * SQLrequest foo = SQLrequest(this, target, "databaseid", (SQLquery("SELECT.. ?"), parameter, parameter)); */ -class SQLquery : public classbase +class SQLquery { public: /** The query 'format string' @@ -242,6 +242,11 @@ public: { } + // Copy constructor - XXX probably shouldn't be needed + SQLrequest(const SQLrequest& o) + : Request(o.source, o.dest, SQLREQID), query(o.query), dbid(o.dbid), pri(o.pri), cancel(o.cancel), + id(o.id), error(o.error) {} + /** Set the priority of a request. */ void Priority(bool p = true) @@ -489,7 +494,7 @@ bool operator!= (const SQLhost& l, const SQLhost& r) * until pop() is called. */ -class QueryQueue : public classbase +class QueryQueue { private: typedef std::deque ReqDeque; diff --git a/src/modules/m_timedbans.cpp b/src/modules/m_timedbans.cpp index 02e687b2d..94301bb7b 100644 --- a/src/modules/m_timedbans.cpp +++ b/src/modules/m_timedbans.cpp @@ -17,7 +17,7 @@ /** Holds a timed ban */ -class TimedBan : public classbase +class TimedBan { public: std::string channel; diff --git a/src/modules/u_listmode.h b/src/modules/u_listmode.h index 1516b724c..0ca44547f 100644 --- a/src/modules/u_listmode.h +++ b/src/modules/u_listmode.h @@ -25,7 +25,7 @@ inline std::string stringtime() /** An item in a listmode's list */ -class ListItem : public classbase +class ListItem { public: std::string nick; @@ -35,7 +35,7 @@ public: /** The number of items a listmode's list may contain */ -class ListLimit : public classbase +class ListLimit { public: std::string mask; diff --git a/src/users.cpp b/src/users.cpp index 657225069..542a6e565 100644 --- a/src/users.cpp +++ b/src/users.cpp @@ -583,14 +583,14 @@ void User::OnError(BufferedSocketError) ServerInstance->Users->QuitUser(this, getError()); } -bool User::cull() +CullResult User::cull() { if (!quitting) ServerInstance->Users->QuitUser(this, "Culled without QuitUser"); if (uuid.empty()) { ServerInstance->Logs->Log("USERS", DEBUG, "User culled twice? UUID empty"); - return true; + return Extensible::cull(); } PurgeEmptyChannels(); if (IS_LOCAL(this)) @@ -625,7 +625,7 @@ bool User::cull() ServerInstance->Users->uuidlist->erase(uuid); uuid.clear(); - return true; + return Extensible::cull(); } void User::Oper(const std::string &opertype, const std::string &opername) -- 2.39.2