m_spanningtree Cleanup TreeServer and TreeSocket construction/destruction

5 files changed, 28 insertions, 87 deletions

diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp
index c9956c778..393485cce 100644
--- a/src/modules/m_spanningtree/treeserver.cpp
+++ b/src/modules/m_spanningtree/treeserver.cpp
@@ -34,42 +34,28 @@
  * represents our own server. Therefore, it has no route, no parent, and
  * no socket associated with it. Its version string is our own local version.
  */
-TreeServer::TreeServer(SpanningTreeUtilities* Util, std::string Name, std::string Desc, const std::string &id)
-	: ServerName(Name.c_str()), ServerDesc(Desc), Utils(Util), ServerUser(ServerInstance->FakeClient)
+TreeServer::TreeServer(SpanningTreeUtilities* Util)
+	: Parent(NULL), Route(NULL), ServerName(ServerInstance->Config->ServerName.c_str()), ServerDesc(ServerInstance->Config->ServerDesc)
+	, VersionString(ServerInstance->GetVersionString()), Socket(NULL), Utils(Util), sid(ServerInstance->Config->GetSID()), ServerUser(ServerInstance->FakeClient)
+	, age(ServerInstance->Time()), Warned(false), bursting(false), UserCount(0), OperCount(0), rtt(0), StartBurst(0), Hidden(false)
 {
-	age = ServerInstance->Time();
-	bursting = false;
-	Parent = NULL;
-	VersionString.clear();
-	UserCount = OperCount = 0;
-	VersionString = ServerInstance->GetVersionString();
-	Route = NULL;
-	Socket = NULL; /* Fix by brain */
-	StartBurst = rtt = 0;
-	Warned = Hidden = false;
 	AddHashEntry();
-	SetID(id);
 }
 
 /** When we create a new server, we call this constructor to initialize it.
  * This constructor initializes the server's Route and Parent, and sets up
  * its ping counters so that it will be pinged one minute from now.
  */
-TreeServer::TreeServer(SpanningTreeUtilities* Util, std::string Name, std::string Desc, const std::string &id, TreeServer* Above, TreeSocket* Sock, bool Hide)
-	: Parent(Above), ServerName(Name.c_str()), ServerDesc(Desc), Socket(Sock), Utils(Util), ServerUser(new FakeUser(id, Name)), Hidden(Hide)
+TreeServer::TreeServer(SpanningTreeUtilities* Util, const std::string& Name, const std::string& Desc, const std::string& id, TreeServer* Above, TreeSocket* Sock, bool Hide)
+	: Parent(Above), ServerName(Name.c_str()), ServerDesc(Desc), Socket(Sock), Utils(Util), sid(id), ServerUser(new FakeUser(id, Name))
+	, age(ServerInstance->Time()), Warned(false), bursting(true), UserCount(0), OperCount(0), rtt(0), Hidden(Hide)
 {
-	age = ServerInstance->Time();
-	bursting = true;
-	VersionString.clear();
-	UserCount = OperCount = 0;
 	SetNextPingTime(ServerInstance->Time() + Utils->PingFreq);
 	SetPingFlag();
-	Warned = false;
-	rtt = 0;
 
 	long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000);
 	this->StartBurst = ts;
-	ServerInstance->Logs->Log("m_spanningtree", LOG_DEBUG, "Started bursting at time %lu", ts);
+	ServerInstance->Logs->Log("m_spanningtree", LOG_DEBUG, "Server %s started bursting at time %lu", sid.c_str(), ts);
 
 	/* find the 'route' for this server (e.g. the one directly connected
 	 * to the local server, which we can use to reach it)
@@ -123,8 +109,6 @@ TreeServer::TreeServer(SpanningTreeUtilities* Util, std::string Name, std::strin
 	 */
 
 	this->AddHashEntry();
-
-	SetID(id);
 }
 
 const std::string& TreeServer::GetID()
@@ -155,13 +139,6 @@ void TreeServer::FinishBurst()
 	AddServerEvent(Utils->Creator, ServerName.c_str());
 }
 
-void TreeServer::SetID(const std::string &id)
-{
-	ServerInstance->Logs->Log("m_spanningtree", LOG_DEBUG, "Setting SID to " + id);
-	sid = id;
-	Utils->sidlist[sid] = this;
-}
-
 int TreeServer::QuitUsers(const std::string &reason)
 {
 	const char* reason_s = reason.c_str();
@@ -196,20 +173,8 @@ int TreeServer::QuitUsers(const std::string &reason)
  */
 void TreeServer::AddHashEntry()
 {
-	server_hash::iterator iter = Utils->serverlist.find(this->ServerName.c_str());
-	if (iter == Utils->serverlist.end())
-		Utils->serverlist[this->ServerName.c_str()] = this;
-}
-
-/** This method removes the reference to this object
- * from the hash_map which is used for linear searches.
- * It is only called by the default destructor.
- */
-void TreeServer::DelHashEntry()
-{
-	server_hash::iterator iter = Utils->serverlist.find(this->ServerName.c_str());
-	if (iter != Utils->serverlist.end())
-		Utils->serverlist.erase(iter);
+	Utils->serverlist[ServerName.c_str()] = this;
+	Utils->sidlist[sid] = this;
 }
 
 /** These accessors etc should be pretty self-
@@ -338,11 +303,9 @@ CullResult TreeServer::cull()
 TreeServer::~TreeServer()
 {
 	/* We'd better tidy up after ourselves, eh? */
-	this->DelHashEntry();
 	if (ServerUser != ServerInstance->FakeClient)
 		delete ServerUser;
 
-	server_hash::iterator iter = Utils->sidlist.find(GetID());
-	if (iter != Utils->sidlist.end())
-		Utils->sidlist.erase(iter);
+	Utils->sidlist.erase(sid);
+	Utils->serverlist.erase(ServerName.c_str());
 }
diff --git a/src/modules/m_spanningtree/treeserver.h b/src/modules/m_spanningtree/treeserver.h
index 460231502..26cc0e424 100644
--- a/src/modules/m_spanningtree/treeserver.h
+++ b/src/modules/m_spanningtree/treeserver.h
@@ -51,15 +51,14 @@ class TreeServer : public classbase
 	SpanningTreeUtilities* Utils;		/* Utility class */
 	std::string sid;			/* Server ID */
 
-	/** Set server ID
-	 * @param id Server ID
-	 * @throws CoreException on duplicate ID
+	/** This method is used to add this TreeServer to the
+	 * hash maps. It is only called by the constructors.
 	 */
-	void SetID(const std::string &id);
+	void AddHashEntry();
 
  public:
 	FakeUser* const ServerUser;		/* User representing this server */
-	time_t age;
+	const time_t age;
 
 	bool Warned;				/* True if we've warned opers about high latency on this server */
 	bool bursting;				/* whether or not this server is bursting */
@@ -71,28 +70,16 @@ class TreeServer : public classbase
 	 * represents our own server. Therefore, it has no route, no parent, and
 	 * no socket associated with it. Its version string is our own local version.
 	 */
-	TreeServer(SpanningTreeUtilities* Util, std::string Name, std::string Desc, const std::string &id);
+	TreeServer(SpanningTreeUtilities* Util);
 
 	/** When we create a new server, we call this constructor to initialize it.
 	 * This constructor initializes the server's Route and Parent, and sets up
 	 * its ping counters so that it will be pinged one minute from now.
 	 */
-	TreeServer(SpanningTreeUtilities* Util, std::string Name, std::string Desc, const std::string &id, TreeServer* Above, TreeSocket* Sock, bool Hide);
+	TreeServer(SpanningTreeUtilities* Util, const std::string& Name, const std::string& Desc, const std::string& id, TreeServer* Above, TreeSocket* Sock, bool Hide);
 
 	int QuitUsers(const std::string &reason);
 
-	/** This method is used to add the structure to the
-	 * hash_map for linear searches. It is only called
-	 * by the constructors.
-	 */
-	void AddHashEntry();
-
-	/** This method removes the reference to this object
-	 * from the hash_map which is used for linear searches.
-	 * It is only called by the default destructor.
-	 */
-	void DelHashEntry();
-
 	/** Get route.
 	 * The 'route' is defined as the locally-
 	 * connected server which can be used to reach this server.
@@ -190,7 +177,10 @@ class TreeServer : public classbase
 	void FinishBurstInternal();
 
 	CullResult cull();
+
 	/** Destructor
+	 * Removes the reference to this object from the
+	 * hash maps.
 	 */
 	~TreeServer();
 };
diff --git a/src/modules/m_spanningtree/treesocket.h b/src/modules/m_spanningtree/treesocket.h
index 0dcdc830e..ded03d6ca 100644
--- a/src/modules/m_spanningtree/treesocket.h
+++ b/src/modules/m_spanningtree/treesocket.h
@@ -104,7 +104,7 @@ class TreeSocket : public BufferedSocket
 	bool CheckDuplicate(const std::string& servername, const std::string& sid);
 
  public:
-	time_t age;
+	const time_t age;
 
 	/** Because most of the I/O gubbins are encapsulated within
 	 * BufferedSocket, we just call the superclass constructor for
diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp
index b7fadf342..719c31bce 100644
--- a/src/modules/m_spanningtree/treesocket1.cpp
+++ b/src/modules/m_spanningtree/treesocket1.cpp
@@ -36,18 +36,13 @@
  * to it.
  */
 TreeSocket::TreeSocket(SpanningTreeUtilities* Util, Link* link, Autoconnect* myac, const std::string& ipaddr)
-	: Utils(Util)
+	: Utils(Util), linkID(assign(link->Name)), LinkState(CONNECTING), MyRoot(NULL), proto_version(0), ConnectionFailureShown(false)
+	, age(ServerInstance->Time())
 {
-	age = ServerInstance->Time();
-	linkID = assign(link->Name);
 	capab = new CapabData;
 	capab->link = link;
 	capab->ac = myac;
 	capab->capab_phase = 0;
-	MyRoot = NULL;
-	proto_version = 0;
-	ConnectionFailureShown = false;
-	LinkState = CONNECTING;
 	if (!link->Hook.empty())
 	{
 		ServiceProvider* prov = ServerInstance->Modules->FindService(SERVICE_IOHOOK, link->Hook);
@@ -68,16 +63,11 @@ TreeSocket::TreeSocket(SpanningTreeUtilities* Util, Link* link, Autoconnect* mya
  * connection. This constructor is used for this purpose.
  */
 TreeSocket::TreeSocket(SpanningTreeUtilities* Util, int newfd, ListenSocket* via, irc::sockets::sockaddrs* client, irc::sockets::sockaddrs* server)
-	: BufferedSocket(newfd), Utils(Util)
+	: BufferedSocket(newfd), Utils(Util), linkID("inbound from " + client->addr()), LinkState(WAIT_AUTH_1), MyRoot(NULL), proto_version(0)
+	, ConnectionFailureShown(false), age(ServerInstance->Time())
 {
 	capab = new CapabData;
 	capab->capab_phase = 0;
-	MyRoot = NULL;
-	age = ServerInstance->Time();
-	LinkState = WAIT_AUTH_1;
-	proto_version = 0;
-	ConnectionFailureShown = false;
-	linkID = "inbound from " + client->addr();
 
 	FOREACH_MOD(I_OnHookIO, OnHookIO(this, via));
 	if (GetIOHook())
@@ -111,8 +101,7 @@ CullResult TreeSocket::cull()
 
 TreeSocket::~TreeSocket()
 {
-	if (capab)
-		delete capab;
+	delete capab;
 }
 
 /** When an outbound connection finishes connecting, we receive
diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp
index c469f41a2..f527de56e 100644
--- a/src/modules/m_spanningtree/utils.cpp
+++ b/src/modules/m_spanningtree/utils.cpp
@@ -130,9 +130,8 @@ SpanningTreeUtilities::SpanningTreeUtilities(ModuleSpanningTree* C)
 	: RefreshTimer(this), Creator(C)
 {
 	ServerInstance->Timers->AddTimer(&RefreshTimer);
-	ServerInstance->Logs->Log("m_spanningtree", LOG_DEBUG, "***** Using SID for hash: %s *****", ServerInstance->Config->GetSID().c_str());
 
-	this->TreeRoot = new TreeServer(this, ServerInstance->Config->ServerName, ServerInstance->Config->ServerDesc, ServerInstance->Config->GetSID());
+	this->TreeRoot = new TreeServer(this);
 	this->ReadConfiguration();
 }