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

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 <deque>
 #include <string>
 
-/** 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<type> 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<unsigned char*, std::string> 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<ExtensionItem*>& 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<int api>
-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<ExtensionItem*>& 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 << "<metadata>";
-		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<SQLrequest*> 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)