From 90abe2cd475c8ca2e81626f565d173e9f56d1251 Mon Sep 17 00:00:00 2001
From: Attila Molnar <attilamolnar@hush.com>
Date: Sat, 26 Jul 2014 15:47:52 +0200
Subject: [PATCH] m_spanningtree Update comments around collision handling

---
 src/modules/m_spanningtree/nick.cpp        |  3 ++-
 src/modules/m_spanningtree/nickcollide.cpp | 25 ++++++++--------------
 src/modules/m_spanningtree/uid.cpp         |  9 ++++----
 3 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/src/modules/m_spanningtree/nick.cpp b/src/modules/m_spanningtree/nick.cpp
index c689a0fdd..cdec280e1 100644
--- a/src/modules/m_spanningtree/nick.cpp
+++ b/src/modules/m_spanningtree/nick.cpp
@@ -45,7 +45,8 @@ CmdResult CommandNick::HandleRemote(RemoteUser* user, std::vector<std::string>&
 	User* x = ServerInstance->FindNickOnly(params[0]);
 	if ((x) && (x != user) && (x->registered == REG_ALL))
 	{
-		/* x is local, who is remote */
+		// 'x' is the already existing user using the same nick as params[0]
+		// 'user' is the user trying to change nick to the in use nick
 		int collideret = Utils->DoCollision(x, TreeServer::Get(user), newts, user->ident, user->GetIPString(), user->uuid);
 		if (collideret != 1)
 		{
diff --git a/src/modules/m_spanningtree/nickcollide.cpp b/src/modules/m_spanningtree/nickcollide.cpp
index 360014fac..3b5944e9f 100644
--- a/src/modules/m_spanningtree/nickcollide.cpp
+++ b/src/modules/m_spanningtree/nickcollide.cpp
@@ -62,15 +62,10 @@ int SpanningTreeUtilities::DoCollision(User* u, TreeServer* server, time_t remot
 	#define localident u->ident
 	#define localip u->GetIPString()
 
-	/* mmk. let's do this again. */
-	if (remotets == localts)
+	// If the timestamps are not equal only one of the users has to change nick,
+	// otherwise both have to change
+	if (remotets != localts)
 	{
-		/* equal. fuck them both! do nada, let the handler at the bottom figure this out. */
-	}
-	else
-	{
-		/* fuck. now it gets complex. */
-
 		/* first, let's see if ident@host matches. */
 		bool SamePerson = (localident == remoteident)
 				&& (localip == remoteip);
@@ -82,19 +77,18 @@ int SpanningTreeUtilities::DoCollision(User* u, TreeServer* server, time_t remot
 		if((SamePerson && remotets < localts) ||
 		   (!SamePerson && remotets > localts))
 		{
-			/* remote needs to change */
+			// Only remote needs to change
 			bChangeLocal = false;
 		}
 		else
 		{
-			/* ours needs to change */
+			// Only ours needs to change
 			bChangeRemote = false;
 		}
 	}
 
 	/*
-	 * Cheat a little here. Instead of a dedicated command to change UID,
-	 * use SAVE and accept the losing client with its UID (as we know the SAVE will
+	 * Send SAVE and accept the losing client with its UID (as we know the SAVE will
 	 * not fail under any circumstances -- UIDs are netwide exclusive).
 	 *
 	 * This means that each side of a collide will generate one extra NICK back to where
@@ -108,7 +102,7 @@ int SpanningTreeUtilities::DoCollision(User* u, TreeServer* server, time_t remot
 	{
 		/*
 		 * Local-side nick needs to change. Just in case we are hub, and
-		 * this "local" nick is actually behind us, send an SAVE out.
+		 * this "local" nick is actually behind us, send a SAVE out.
 		 */
 		CmdBuilder params("SAVE");
 		params.push_back(u->uuid);
@@ -123,9 +117,8 @@ int SpanningTreeUtilities::DoCollision(User* u, TreeServer* server, time_t remot
 	if (bChangeRemote)
 	{
 		/*
-		 * remote side needs to change. If this happens, we will modify
-		 * the UID or halt the propagation of the nick change command,
-		 * so other servers don't need to see the SAVE
+		 * Remote side needs to change. If this happens, we modify the UID or NICK and
+		 * send back a SAVE to the source.
 		 */
 		TreeSocket* sock = server->GetSocket();
 		sock->WriteLine(CmdBuilder("SAVE").push(remoteuid).push_int(remotets));
diff --git a/src/modules/m_spanningtree/uid.cpp b/src/modules/m_spanningtree/uid.cpp
index 4d8f82203..cafaee4c0 100644
--- a/src/modules/m_spanningtree/uid.cpp
+++ b/src/modules/m_spanningtree/uid.cpp
@@ -43,7 +43,7 @@ CmdResult CommandUID::HandleServer(TreeServer* remoteserver, std::vector<std::st
 	if (modestr[0] != '+')
 		throw ProtocolException("Invalid mode string");
 
-	/* check for collision */
+	// See if there is a nick collision
 	User* collideswith = ServerInstance->FindNickOnly(params[2]);
 	if ((collideswith) && (collideswith->registered != REG_ALL))
 	{
@@ -58,15 +58,14 @@ CmdResult CommandUID::HandleServer(TreeServer* remoteserver, std::vector<std::st
 	}
 	else if (collideswith)
 	{
-		/*
-		 * Nick collision.
-		 */
+		// The user on this side is registered, handle the collision
 		int collide = Utils->DoCollision(collideswith, remoteserver, age_t, params[5], params[6], params[0]);
 		ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "*** Collision on %s, collide=%d", params[2].c_str(), collide);
 
 		if (collide != 1)
 		{
-			// Remote client lost, make sure we change their nick for the hash too
+			// The client being introduced needs to change nick to uuid, change the nick in the message before
+			// processing/forwarding it.
 			params[2] = params[0];
 		}
 	}
-- 
2.39.5