m_spanningtree Change TreeServer::GetSocket() to always return the socket that can be used to talk to the server

The only exception is the tree root (us) which has no associated socket (NULL)

Add TreeServer::IsRoot() and IsLocal() helpers

11 files changed, 68 insertions, 94 deletions

diff --git a/src/modules/m_spanningtree/fjoin.cpp b/src/modules/m_spanningtree/fjoin.cpp
index 831d52c1d..93320757c 100644
--- a/src/modules/m_spanningtree/fjoin.cpp
+++ b/src/modules/m_spanningtree/fjoin.cpp
@@ -133,7 +133,7 @@ CmdResult CommandFJoin::Handle(User* srcuser, std::vector<std::string>& params)
 	}
 
 	irc::modestacker modestack(true);
-	TreeSocket* src_socket = Utils->FindServer(srcuser->server)->GetRoute()->GetSocket();
+	TreeSocket* src_socket = Utils->FindServer(srcuser->server)->GetSocket();
 
 	/* Now, process every 'modes,uuid' pair */
 	irc::tokenstream users(*params.rbegin());
diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp
index 9c2f3969a..6e4277c08 100644
--- a/src/modules/m_spanningtree/main.cpp
+++ b/src/modules/m_spanningtree/main.cpp
@@ -138,60 +138,53 @@ restart:
 	{
 		TreeServer *s = i->second;
 
-		if (s->GetSocket() && s->GetSocket()->GetLinkState() == DYING)
+		// Skip myself
+		if (s->IsRoot())
+			continue;
+
+		if (s->GetSocket()->GetLinkState() == DYING)
 		{
 			s->GetSocket()->Close();
 			goto restart;
 		}
 
-		// Fix for bug #792, do not ping servers that are not connected yet!
-		// Remote servers have Socket == NULL and local connected servers have
-		// Socket->LinkState == CONNECTED
-		if (s->GetSocket() && s->GetSocket()->GetLinkState() != CONNECTED)
+		// Do not ping servers that are not fully connected yet!
+		// Servers which are connected to us have IsLocal() == true and if they're fully connected
+		// then Socket->LinkState == CONNECTED. Servers that are linked to another server are always fully connected.
+		if (s->IsLocal() && s->GetSocket()->GetLinkState() != CONNECTED)
 			continue;
 
 		// Now do PING checks on all servers
-		TreeServer *mts = Utils->BestRouteTo(s->GetID());
-
-		if (mts)
+		// Only ping if this server needs one
+		if (curtime >= s->NextPingTime())
 		{
-			// Only ping if this server needs one
-			if (curtime >= s->NextPingTime())
+			// And if they answered the last
+			if (s->AnsweredLastPing())
 			{
-				// And if they answered the last
-				if (s->AnsweredLastPing())
-				{
-					// They did, send a ping to them
-					s->SetNextPingTime(curtime + Utils->PingFreq);
-					TreeSocket *tsock = mts->GetSocket();
-
-					// ... if we can find a proper route to them
-					if (tsock)
-					{
-						tsock->WriteLine(":" + ServerInstance->Config->GetSID() + " PING " + s->GetID());
-						s->LastPingMsec = ts;
-					}
-				}
-				else
+				// They did, send a ping to them
+				s->SetNextPingTime(curtime + Utils->PingFreq);
+				s->GetSocket()->WriteLine(":" + ServerInstance->Config->GetSID() + " PING " + s->GetID());
+				s->LastPingMsec = ts;
+			}
+			else
+			{
+				// They didn't answer the last ping, if they are locally connected, get rid of them.
+				if (s->IsLocal())
 				{
-					// They didn't answer the last ping, if they are locally connected, get rid of them.
-					TreeSocket *sock = s->GetSocket();
-					if (sock)
-					{
-						sock->SendError("Ping timeout");
-						sock->Close();
-						goto restart;
-					}
+					TreeSocket* sock = s->GetSocket();
+					sock->SendError("Ping timeout");
+					sock->Close();
+					goto restart;
 				}
 			}
+		}
 
-			// If warn on ping enabled and not warned and the difference is sufficient and they didn't answer the last ping...
-			if ((Utils->PingWarnTime) && (!s->Warned) && (curtime >= s->NextPingTime() - (Utils->PingFreq - Utils->PingWarnTime)) && (!s->AnsweredLastPing()))
-			{
-				/* The server hasnt responded, send a warning to opers */
-				ServerInstance->SNO->WriteToSnoMask('l',"Server \002%s\002 has not responded to PING for %d seconds, high latency.", s->GetName().c_str(), Utils->PingWarnTime);
-				s->Warned = true;
-			}
+		// If warn on ping enabled and not warned and the difference is sufficient and they didn't answer the last ping...
+		if ((Utils->PingWarnTime) && (!s->Warned) && (curtime >= s->NextPingTime() - (Utils->PingFreq - Utils->PingWarnTime)) && (!s->AnsweredLastPing()))
+		{
+			/* The server hasnt responded, send a warning to opers */
+			ServerInstance->SNO->WriteToSnoMask('l',"Server \002%s\002 has not responded to PING for %d seconds, high latency.", s->GetName().c_str(), Utils->PingWarnTime);
+			s->Warned = true;
 		}
 	}
 }
diff --git a/src/modules/m_spanningtree/nickcollide.cpp b/src/modules/m_spanningtree/nickcollide.cpp
index 39490e953..e849c69ae 100644
--- a/src/modules/m_spanningtree/nickcollide.cpp
+++ b/src/modules/m_spanningtree/nickcollide.cpp
@@ -123,7 +123,7 @@ int SpanningTreeUtilities::DoCollision(User* u, TreeServer* server, time_t remot
 		 * the UID or halt the propagation of the nick change command,
 		 * so other servers don't need to see the SAVE
 		 */
-		TreeSocket* sock = server->GetRoute()->GetSocket();
+		TreeSocket* sock = server->GetSocket();
 		sock->WriteLine(":"+ServerInstance->Config->GetSID()+" SAVE "+remoteuid+" "+ ConvToStr(remotets));
 
 		if (remote)
diff --git a/src/modules/m_spanningtree/override_map.cpp b/src/modules/m_spanningtree/override_map.cpp
index 2b655097d..14d0a3b4f 100644
--- a/src/modules/m_spanningtree/override_map.cpp
+++ b/src/modules/m_spanningtree/override_map.cpp
@@ -111,7 +111,7 @@ CmdResult CommandMap::Handle(const std::vector<std::string>& parameters, User* u
 			return CMD_FAILURE;
 		}
 
-		if (s != Utils->TreeRoot)
+		if (!s->IsRoot())
 			return CMD_SUCCESS;
 	}
 
diff --git a/src/modules/m_spanningtree/override_squit.cpp b/src/modules/m_spanningtree/override_squit.cpp
index d3f1408fc..d3caa7bc8 100644
--- a/src/modules/m_spanningtree/override_squit.cpp
+++ b/src/modules/m_spanningtree/override_squit.cpp
@@ -30,7 +30,7 @@ ModResult ModuleSpanningTree::HandleSquit(const std::vector<std::string>& parame
 	TreeServer* s = Utils->FindServerMask(parameters[0]);
 	if (s)
 	{
-		if (s == Utils->TreeRoot)
+		if (s->IsRoot())
 		{
 			user->WriteNotice("*** SQUIT: Foolish mortal, you cannot make a server SQUIT itself! (" + parameters[0] + " matches local server name)");
 			return MOD_RES_DENY;
@@ -38,7 +38,7 @@ ModResult ModuleSpanningTree::HandleSquit(const std::vector<std::string>& parame
 
 		TreeSocket* sock = s->GetSocket();
 
-		if (sock)
+		if (s->IsLocal())
 		{
 			ServerInstance->SNO->WriteToSnoMask('l',"SQUIT: Server \002%s\002 removed from network by %s",parameters[0].c_str(),user->nick.c_str());
 			sock->Squit(s,"Server quit by " + user->GetFullRealHost());
diff --git a/src/modules/m_spanningtree/rsquit.cpp b/src/modules/m_spanningtree/rsquit.cpp
index 9437c174a..3b7468491 100644
--- a/src/modules/m_spanningtree/rsquit.cpp
+++ b/src/modules/m_spanningtree/rsquit.cpp
@@ -34,7 +34,6 @@ CommandRSQuit::CommandRSQuit(Module* Creator)
 CmdResult CommandRSQuit::Handle (const std::vector<std::string>& parameters, User *user)
 {
 	TreeServer *server_target; // Server to squit
-	TreeServer *server_linked; // Server target is linked to
 
 	server_target = Utils->FindServerMask(parameters[0]);
 	if (!server_target)
@@ -43,25 +42,20 @@ CmdResult CommandRSQuit::Handle (const std::vector<std::string>& parameters, Use
 		return CMD_FAILURE;
 	}
 
-	if (server_target == Utils->TreeRoot)
+	if (server_target->IsRoot())
 	{
 		((ModuleSpanningTree*)(Module*)creator)->RemoteMessage(user, "*** RSQUIT: Foolish mortal, you cannot make a server SQUIT itself! (%s matches local server name)", parameters[0].c_str());
 		return CMD_FAILURE;
 	}
 
-	server_linked = server_target->GetParent();
-
-	if (server_linked == Utils->TreeRoot)
+	if (server_target->IsLocal())
 	{
 		// We have been asked to remove server_target.
 		TreeSocket* sock = server_target->GetSocket();
-		if (sock)
-		{
-			const char *reason = parameters.size() == 2 ? parameters[1].c_str() : "No reason";
-			ServerInstance->SNO->WriteToSnoMask('l',"RSQUIT: Server \002%s\002 removed from network by %s (%s)", parameters[0].c_str(), user->nick.c_str(), reason);
-			sock->Squit(server_target, "Server quit by " + user->GetFullRealHost() + " (" + reason + ")");
-			sock->Close();
-		}
+		const char* reason = parameters.size() == 2 ? parameters[1].c_str() : "No reason";
+		ServerInstance->SNO->WriteToSnoMask('l',"RSQUIT: Server \002%s\002 removed from network by %s (%s)", parameters[0].c_str(), user->nick.c_str(), reason);
+		sock->Squit(server_target, "Server quit by " + user->GetFullRealHost() + " (" + reason + ")");
+		sock->Close();
 	}
 
 	return CMD_SUCCESS;
diff --git a/src/modules/m_spanningtree/server.cpp b/src/modules/m_spanningtree/server.cpp
index fcd326974..538d28dbd 100644
--- a/src/modules/m_spanningtree/server.cpp
+++ b/src/modules/m_spanningtree/server.cpp
@@ -39,7 +39,7 @@ CmdResult CommandServer::Handle(User* user, std::vector<std::string>& params)
 	std::string sid = params[3];
 	std::string description = params[4];
 	TreeServer* ParentOfThis = Utils->FindServer(user->server);
-	TreeSocket* socket = ParentOfThis->GetRoute()->GetSocket();
+	TreeSocket* socket = ParentOfThis->GetSocket();
 
 	if (!IS_SERVER(user))
 		return CMD_FAILURE;
@@ -67,7 +67,7 @@ CmdResult CommandServer::Handle(User* user, std::vector<std::string>& params)
 
 	Link* lnk = Utils->FindLink(servername);
 
-	TreeServer *Node = new TreeServer(servername, description, sid, ParentOfThis,NULL, lnk ? lnk->Hidden : false);
+	TreeServer* Node = new TreeServer(servername, description, sid, ParentOfThis, ParentOfThis->GetSocket(), lnk ? lnk->Hidden : false);
 
 	ParentOfThis->AddChild(Node);
 	ServerInstance->SNO->WriteToSnoMask('L', "Server \002"+ParentOfThis->GetName()+"\002 introduced server \002"+servername+"\002 ("+description+")");
diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp
index 0fad3132d..54ea5922f 100644
--- a/src/modules/m_spanningtree/treeserver.cpp
+++ b/src/modules/m_spanningtree/treeserver.cpp
@@ -288,7 +288,7 @@ void TreeServer::Tidy()
 
 CullResult TreeServer::cull()
 {
-	if (ServerUser != ServerInstance->FakeClient)
+	if (!IsRoot())
 		ServerUser->cull();
 	return classbase::cull();
 }
@@ -296,7 +296,7 @@ CullResult TreeServer::cull()
 TreeServer::~TreeServer()
 {
 	/* We'd better tidy up after ourselves, eh? */
-	if (ServerUser != ServerInstance->FakeClient)
+	if (!IsRoot())
 		delete ServerUser;
 
 	Utils->sidlist.erase(sid);
diff --git a/src/modules/m_spanningtree/treeserver.h b/src/modules/m_spanningtree/treeserver.h
index faaa47aa1..9f2e24172 100644
--- a/src/modules/m_spanningtree/treeserver.h
+++ b/src/modules/m_spanningtree/treeserver.h
@@ -45,7 +45,7 @@ class TreeServer : public classbase
 	std::string ServerName;			/* Server's name */
 	std::string ServerDesc;			/* Server's description */
 	std::string VersionString;		/* Version string or empty string */
-	TreeSocket* Socket;			/* For directly connected servers this points at the socket object */
+	TreeSocket* Socket;			/* Socket used to communicate with this server */
 	time_t NextPing;			/* After this time, the server should be PINGed*/
 	bool LastPingWasGood;			/* True if the server responded to the last PING with a PONG */
 	std::string sid;			/* Server ID */
@@ -85,6 +85,14 @@ class TreeServer : public classbase
 	 */
 	TreeServer* GetRoute();
 
+	/** Returns true if this server is the tree root (i.e.: us)
+	 */
+	bool IsRoot() const { return (this->Parent == NULL); }
+
+	/** Returns true if this server is locally connected
+	 */
+	bool IsLocal() const { return (this->Route == this); }
+
 	/** Get server name
 	 */
 	const std::string& GetName() const { return ServerName; }
diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp
index 8dbdb3839..9e0acdd12 100644
--- a/src/modules/m_spanningtree/treesocket1.cpp
+++ b/src/modules/m_spanningtree/treesocket1.cpp
@@ -168,11 +168,11 @@ void TreeSocket::Squit(TreeServer* Current, const std::string &reason)
 {
 	bool LocalSquit = false;
 
-	if (Current != Utils->TreeRoot)
+	if (!Current->IsRoot())
 	{
 		DelServerEvent(Utils->Creator, Current->GetName());
 
-		if (Current->GetParent() == Utils->TreeRoot)
+		if (Current->IsLocal())
 		{
 			ServerInstance->SNO->WriteGlobalSno('l', "Server \002"+Current->GetName()+"\002 split: "+reason);
 			LocalSquit = true;
@@ -215,7 +215,7 @@ CmdResult CommandSQuit::Handle(User* user, std::vector<std::string>& params)
 		return CMD_FAILURE;
 	}
 
-	TreeSocket* sock = Utils->FindServer(user->server)->GetRoute()->GetSocket();
+	TreeSocket* sock = Utils->FindServer(user->server)->GetSocket();
 	sock->Squit(quitting, params[1]);
 	return CMD_SUCCESS;
 }
diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp
index ddfd26b6d..ef1070a12 100644
--- a/src/modules/m_spanningtree/utils.cpp
+++ b/src/modules/m_spanningtree/utils.cpp
@@ -81,8 +81,6 @@ TreeServer* SpanningTreeUtilities::FindServer(const std::string &ServerName)
  */
 TreeServer* SpanningTreeUtilities::BestRouteTo(const std::string &ServerName)
 {
-	if (ServerName == TreeRoot->GetName() || ServerName == ServerInstance->Config->GetSID())
-		return NULL;
 	TreeServer* Found = FindServer(ServerName);
 	if (Found)
 	{
@@ -216,15 +214,10 @@ void SpanningTreeUtilities::DoOneToAllButSender(const std::string& prefix, const
 	for (unsigned int x = 0; x < items; x++)
 	{
 		TreeServer* Route = this->TreeRoot->GetChild(x);
-		// Send the line IF:
-		// The route has a socket (its a direct connection)
-		// The route isnt the one to be omitted
-		// The route isnt the path to the one to be omitted
-		if ((Route) && (Route->GetSocket()) && (Route->GetName() != omit) && (omitroute != Route))
+		// Send the line if the route isn't the path to the one to be omitted
+		if (Route != omitroute)
 		{
-			TreeSocket* Sock = Route->GetSocket();
-			if (Sock)
-				Sock->WriteLine(FullLine);
+			Route->GetSocket()->WriteLine(FullLine);
 		}
 	}
 }
@@ -237,32 +230,18 @@ void SpanningTreeUtilities::DoOneToMany(const std::string &prefix, const std::st
 	for (unsigned int x = 0; x < items; x++)
 	{
 		TreeServer* Route = this->TreeRoot->GetChild(x);
-		if (Route && Route->GetSocket())
-		{
-			TreeSocket* Sock = Route->GetSocket();
-			if (Sock)
-				Sock->WriteLine(FullLine);
-		}
+		Route->GetSocket()->WriteLine(FullLine);
 	}
 }
 
 bool SpanningTreeUtilities::DoOneToOne(const std::string& prefix, const std::string& command, const parameterlist& params, const std::string& target)
 {
 	TreeServer* Route = this->BestRouteTo(target);
-	if (Route)
-	{
-		if (Route && Route->GetSocket())
-		{
-			TreeSocket* Sock = Route->GetSocket();
-			if (Sock)
-				Sock->WriteLine(ConstructLine(prefix, command, params));
-		}
-		return true;
-	}
-	else
-	{
+	if (!Route)
 		return false;
-	}
+
+	Route->GetSocket()->WriteLine(ConstructLine(prefix, command, params));
+	return true;
 }
 
 void SpanningTreeUtilities::RefreshIPCache()