m_spanningtree Clean up comments

14 files changed, 43 insertions, 67 deletions

diff --git a/src/modules/m_spanningtree/cachetimer.h b/src/modules/m_spanningtree/cachetimer.h
index 89933cc4b..cffbe3578 100644
--- a/src/modules/m_spanningtree/cachetimer.h
+++ b/src/modules/m_spanningtree/cachetimer.h
@@ -19,9 +19,7 @@
 
 #pragma once
 
-/** Create a timer which recurs every second, we inherit from Timer.
- * Timer is only one-shot however, so at the end of each Tick() we simply
- * insert another of ourselves into the pending queue :)
+/** Timer that fires when we need to refresh the IP cache of servers
  */
 class CacheRefreshTimer : public Timer
 {
diff --git a/src/modules/m_spanningtree/delline.cpp b/src/modules/m_spanningtree/delline.cpp
index c76af2fb7..f790dc885 100644
--- a/src/modules/m_spanningtree/delline.cpp
+++ b/src/modules/m_spanningtree/delline.cpp
@@ -26,7 +26,7 @@ CmdResult CommandDelLine::Handle(User* user, std::vector<std::string>& params)
 {
 	const std::string& setter = user->nick;
 
-	/* NOTE: No check needed on 'user', this function safely handles NULL */
+	// XLineManager::DelLine() returns true if the xline existed, false if it didn't
 	if (ServerInstance->XLines->DelLine(params[1].c_str(), params[0], user))
 	{
 		ServerInstance->SNO->WriteToSnoMask('X',"%s removed %s%s on %s", setter.c_str(),
diff --git a/src/modules/m_spanningtree/fjoin.cpp b/src/modules/m_spanningtree/fjoin.cpp
index e29aa09d7..687bf305e 100644
--- a/src/modules/m_spanningtree/fjoin.cpp
+++ b/src/modules/m_spanningtree/fjoin.cpp
@@ -152,7 +152,7 @@ CmdResult CommandFJoin::Handle(User* srcuser, std::vector<std::string>& params)
 		}
 	}
 
-	/* First up, apply their channel modes if they won the TS war */
+	// Apply their channel modes if we have to
 	Modes::ChangeList modechangelist;
 	if (apply_other_sides_modes)
 	{
@@ -168,7 +168,7 @@ CmdResult CommandFJoin::Handle(User* srcuser, std::vector<std::string>& params)
 	// after applying theirs. If they lost, the prefix modes from their message are not forwarded.
 	FwdFJoinBuilder fwdfjoin(chan, sourceserver);
 
-	/* Now, process every 'modes,uuid' pair */
+	// Process every member in the message
 	irc::tokenstream users(params.back());
 	std::string item;
 	Modes::ChangeList* modechangelistptr = (apply_other_sides_modes ? &modechangelist : NULL);
@@ -256,10 +256,7 @@ void CommandFJoin::RemoveStatus(Channel* c)
 	{
 		ModeHandler* mh = i->second;
 
-		/* Passing a pointer to a modestacker here causes the mode to be put onto the mode stack,
-		 * rather than applied immediately. Module unloads require this to be done immediately,
-		 * for this function we require tidyness instead. Fixes bug #493
-		 */
+		// Add the removal of this mode to the changelist. This handles all kinds of modes, including prefix modes.
 		mh->RemoveMode(c, changelist);
 	}
 
diff --git a/src/modules/m_spanningtree/fmode.cpp b/src/modules/m_spanningtree/fmode.cpp
index 52e512d92..e6f49c5b9 100644
--- a/src/modules/m_spanningtree/fmode.cpp
+++ b/src/modules/m_spanningtree/fmode.cpp
@@ -21,7 +21,7 @@
 #include "inspircd.h"
 #include "commands.h"
 
-/** FMODE command - server mode with timestamp checks */
+/** FMODE command - channel mode change with timestamp checks */
 CmdResult CommandFMode::Handle(User* who, std::vector<std::string>& params)
 {
 	time_t TS = ServerCommand::ExtractTS(params[1]);
@@ -39,7 +39,7 @@ CmdResult CommandFMode::Handle(User* who, std::vector<std::string>& params)
 	if (TS > ourTS)
 		return CMD_FAILURE;
 
-	/* TS is equal or less: Merge the mode changes into ours and pass on.
+	/* TS is equal or less: apply the mode change locally and forward the message
 	 */
 
 	// Turn modes into a Modes::ChangeList; may have more elements than max modes
diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp
index e5e6e522b..98cf3188f 100644
--- a/src/modules/m_spanningtree/main.cpp
+++ b/src/modules/m_spanningtree/main.cpp
@@ -218,7 +218,7 @@ void ModuleSpanningTree::ConnectServer(Link* x, Autoconnect* y)
 	/* Do we already have an IP? If so, no need to resolve it. */
 	if (ipvalid)
 	{
-		/* Gave a hook, but it wasnt one we know */
+		// Create a TreeServer object that will start connecting immediately in the background
 		TreeSocket* newsocket = new TreeSocket(x, y, x->IPAddr);
 		if (newsocket->GetFd() > -1)
 		{
@@ -289,7 +289,7 @@ void ModuleSpanningTree::DoConnectTimeout(time_t curtime)
 
 ModResult ModuleSpanningTree::HandleVersion(const std::vector<std::string>& parameters, User* user)
 {
-	// we've already checked if pcnt > 0, so this is safe
+	// We've already confirmed that !parameters.empty(), so this is safe
 	TreeServer* found = Utils->FindServerMask(parameters[0]);
 	if (found)
 	{
@@ -527,7 +527,7 @@ void ModuleSpanningTree::OnUserQuit(User* user, const std::string &reason, const
 		}
 	}
 
-	// Regardless, We need to modify the user Counts..
+	// Regardless, update the UserCount
 	TreeServer::Get(user)->UserCount--;
 }
 
@@ -651,13 +651,13 @@ restart:
 	}
 }
 
-// note: the protocol does not allow direct umode +o except
-// via NICK with 8 params. sending OPERTYPE infers +o modechange
-// locally.
 void ModuleSpanningTree::OnOper(User* user, const std::string &opertype)
 {
 	if (user->registered != REG_ALL || !IS_LOCAL(user))
 		return;
+
+	// Note: The protocol does not allow direct umode +o;
+	// sending OPERTYPE infers +o modechange locally.
 	CommandOpertype::Builder(user).Broadcast();
 }
 
@@ -735,9 +735,7 @@ ModuleSpanningTree::~ModuleSpanningTree()
 	Server* newsrv = new Server(ServerInstance->Config->ServerName, ServerInstance->Config->ServerDesc);
 	SetLocalUsersServer(newsrv);
 
-	/* This will also free the listeners */
 	delete Utils;
-
 	delete commands;
 }
 
@@ -750,7 +748,7 @@ Version ModuleSpanningTree::GetVersion()
  * so that any activity it sees is FINAL, e.g. we arent going to send out
  * a NICK message before m_cloaking has finished putting the +x on the user,
  * etc etc.
- * Therefore, we return PRIORITY_LAST to make sure we end up at the END of
+ * Therefore, we set our priority to PRIORITY_LAST to make sure we end up at the END of
  * the module call queue.
  */
 void ModuleSpanningTree::Prioritize()
diff --git a/src/modules/m_spanningtree/netburst.cpp b/src/modules/m_spanningtree/netburst.cpp
index b81a285b5..cdafa9ded 100644
--- a/src/modules/m_spanningtree/netburst.cpp
+++ b/src/modules/m_spanningtree/netburst.cpp
@@ -108,17 +108,19 @@ void TreeSocket::DoBurst(TreeServer* s)
 		capab->auth_challenge ? "challenge-response" : "plaintext password");
 	this->CleanNegotiationInfo();
 	this->WriteLine(CmdBuilder("BURST").push_int(ServerInstance->Time()));
-	/* Send server tree */
+	// Introduce all servers behind us
 	this->SendServers(Utils->TreeRoot, s);
 
 	BurstState bs(this);
-	/* Send users and their oper status */
+	// Introduce all users
 	this->SendUsers(bs);
 
+	// Sync all channels
 	const chan_hash& chans = ServerInstance->GetChans();
 	for (chan_hash::const_iterator i = chans.begin(); i != chans.end(); ++i)
 		SyncChannel(i->second, bs);
 
+	// Send all xlines
 	this->SendXLines();
 	FOREACH_MOD(OnSyncNetwork, (bs.server));
 	this->WriteLine(CmdBuilder("ENDBURST"));
@@ -160,10 +162,7 @@ void TreeSocket::SendServers(TreeServer* Current, TreeServer* s)
 }
 
 /** Send one or more FJOINs for a channel of users.
- * If the length of a single line is more than 480-NICKMAX
- * in length, it is split over multiple lines.
- * Send one or more FMODEs for a channel with the
- * channel bans, if there's any.
+ * If the length of a single line is too long, it is split over multiple lines.
  */
 void TreeSocket::SendFJoins(Channel* c)
 {
@@ -242,7 +241,7 @@ void TreeSocket::SendListModes(Channel* chan)
 		this->WriteLine(fmode.finalize());
 }
 
-/** Send channel topic, modes and metadata */
+/** Send channel users, topic, modes and global metadata */
 void TreeSocket::SyncChannel(Channel* chan, BurstState& bs)
 {
 	SendFJoins(chan);
@@ -271,7 +270,7 @@ void TreeSocket::SyncChannel(Channel* chan)
 	SyncChannel(chan, bs);
 }
 
-/** send all users and their oper state/modes */
+/** Send all users and their state, including oper and away status and global metadata */
 void TreeSocket::SendUsers(BurstState& bs)
 {
 	ProtocolInterface::Server& piserver = bs.server;
diff --git a/src/modules/m_spanningtree/override_map.cpp b/src/modules/m_spanningtree/override_map.cpp
index 68551e84f..612df80f3 100644
--- a/src/modules/m_spanningtree/override_map.cpp
+++ b/src/modules/m_spanningtree/override_map.cpp
@@ -168,7 +168,7 @@ CmdResult CommandMap::Handle(const std::vector<std::string>& parameters, User* u
 {
 	if (parameters.size() > 0)
 	{
-		/* Remote MAP, the server is within the 1st parameter */
+		// Remote MAP, the target server is the 1st parameter
 		TreeServer* s = Utils->FindServerMask(parameters[0]);
 		if (!s)
 		{
diff --git a/src/modules/m_spanningtree/server.cpp b/src/modules/m_spanningtree/server.cpp
index 1c624f5c4..30931b90e 100644
--- a/src/modules/m_spanningtree/server.cpp
+++ b/src/modules/m_spanningtree/server.cpp
@@ -162,7 +162,7 @@ bool TreeSocket::Outbound_Reply_Server(parameterlist &params)
 
 bool TreeSocket::CheckDuplicate(const std::string& sname, const std::string& sid)
 {
-	/* Check for fully initialized instances of the server by name */
+	// Check if the server name is not in use by a server that's already fully connected
 	TreeServer* CheckDupe = Utils->FindServer(sname);
 	if (CheckDupe)
 	{
@@ -172,7 +172,7 @@ bool TreeSocket::CheckDuplicate(const std::string& sname, const std::string& sid
 		return false;
 	}
 
-	/* Check for fully initialized instances of the server by id */
+	// Check if the id is not in use by a server that's already fully connected
 	ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Looking for dupe SID %s", sid.c_str());
 	CheckDupe = Utils->FindServerID(sid);
 
diff --git a/src/modules/m_spanningtree/svsnick.cpp b/src/modules/m_spanningtree/svsnick.cpp
index bb21fc54d..b3a612ca3 100644
--- a/src/modules/m_spanningtree/svsnick.cpp
+++ b/src/modules/m_spanningtree/svsnick.cpp
@@ -62,7 +62,7 @@ CmdResult CommandSVSNick::Handle(User* user, std::vector<std::string>& parameter
 
 		if (!u->ChangeNick(nick, NickTS))
 		{
-			/* buh. UID them */
+			// Changing to 'nick' failed (it may already be in use), change to the uuid
 			u->ChangeNick(u->uuid);
 		}
 	}
diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp
index afd86c0ce..3835695dd 100644
--- a/src/modules/m_spanningtree/treeserver.cpp
+++ b/src/modules/m_spanningtree/treeserver.cpp
@@ -48,7 +48,7 @@ TreeServer::TreeServer()
 
 /** 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.
+ * the ping timer for the server.
  */
 TreeServer::TreeServer(const std::string& Name, const std::string& Desc, const std::string& id, TreeServer* Above, TreeSocket* Sock, bool Hide)
 	: Server(Name, Desc)
@@ -248,8 +248,8 @@ void TreeServer::CheckULine()
 	}
 }
 
-/** This method is used to add the structure to the
- * hash_map for linear searches. It is only called
+/** This method is used to add the server to the
+ * maps for linear searches. It is only called
  * by the constructors.
  */
 void TreeServer::AddHashEntry()
diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp
index 025bd1e61..2198d6208 100644
--- a/src/modules/m_spanningtree/treesocket1.cpp
+++ b/src/modules/m_spanningtree/treesocket1.cpp
@@ -31,10 +31,10 @@
 #include "treesocket.h"
 #include "commands.h"
 
-/** Because most of the I/O gubbins are encapsulated within
- * BufferedSocket, we just call the superclass constructor for
- * most of the action, and append a few of our own values
- * to it.
+/** Constructor for outgoing connections.
+ * Because most of the I/O gubbins are encapsulated within
+ * BufferedSocket, we just call DoConnect() for most of the action,
+ * and only do minor initialization tasks ourselves.
  */
 TreeSocket::TreeSocket(Link* link, Autoconnect* myac, const std::string& ipaddr)
 	: linkID(assign(link->Name)), LinkState(CONNECTING), MyRoot(NULL), proto_version(0)
@@ -50,9 +50,7 @@ TreeSocket::TreeSocket(Link* link, Autoconnect* myac, const std::string& ipaddr)
 	SendCapabilities(1);
 }
 
-/** When a listening socket gives us a new file descriptor,
- * we must associate it with a socket without creating a new
- * connection. This constructor is used for this purpose.
+/** Constructor for incoming connections
  */
 TreeSocket::TreeSocket(int newfd, ListenSocket* via, irc::sockets::sockaddrs* client, irc::sockets::sockaddrs* server)
 	: BufferedSocket(newfd)
@@ -92,10 +90,10 @@ TreeSocket::~TreeSocket()
 }
 
 /** When an outbound connection finishes connecting, we receive
- * this event, and must send our SERVER string to the other
+ * this event, and must do CAPAB negotiation with the other
  * side. If the other side is happy, as outlined in the server
  * to server docs on the inspircd.org site, the other side
- * will then send back its own server string.
+ * will then send back its own SERVER string eventually.
  */
 void TreeSocket::OnConnected()
 {
diff --git a/src/modules/m_spanningtree/uid.cpp b/src/modules/m_spanningtree/uid.cpp
index 398573616..72361af38 100644
--- a/src/modules/m_spanningtree/uid.cpp
+++ b/src/modules/m_spanningtree/uid.cpp
@@ -27,7 +27,7 @@
 
 CmdResult CommandUID::HandleServer(TreeServer* remoteserver, std::vector<std::string>& params)
 {
-	/** Do we have enough parameters:
+	/**
 	 *      0    1    2    3    4    5        6        7     8        9       (n-1)
 	 * UID uuid age nick host dhost ident ip.string signon +modes (modepara) :gecos
 	 */
@@ -36,10 +36,10 @@ CmdResult CommandUID::HandleServer(TreeServer* remoteserver, std::vector<std::st
 	std::string empty;
 	const std::string& modestr = params[8];
 
-	/* Is this a valid UID, and not misrouted? */
+	// Check if the length of the uuid is correct and confirm the sid portion of the uuid matches the sid of the server introducing the user
 	if (params[0].length() != UIDGenerator::UUID_LENGTH || params[0].compare(0, 3, remoteserver->GetID()))
 		throw ProtocolException("Bogus UUID");
-	/* Check parameters for validity before introducing the client, discovered by dmb */
+	// Sanity check on mode string: must begin with '+'
 	if (modestr[0] != '+')
 		throw ProtocolException("Invalid mode string");
 
@@ -72,9 +72,7 @@ CmdResult CommandUID::HandleServer(TreeServer* remoteserver, std::vector<std::st
 		}
 	}
 
-	/* IMPORTANT NOTE: For remote users, we pass the UUID in the constructor. This automatically
-	 * sets it up in the UUID hash for us.
-	 *
+	/* For remote users, we pass the UUID they sent to the constructor.
 	 * If the UUID already exists User::User() throws an exception which causes this connection to be closed.
 	 */
 	RemoteUser* _new = new RemoteUser(params[0], remoteserver);
diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp
index d81bfa934..0cd6c76c2 100644
--- a/src/modules/m_spanningtree/utils.cpp
+++ b/src/modules/m_spanningtree/utils.cpp
@@ -31,7 +31,6 @@
 
 SpanningTreeUtilities* Utils = NULL;
 
-/* Create server sockets off a listener. */
 ModResult ModuleSpanningTree::OnAcceptConnection(int newsock, ListenSocket* from, irc::sockets::sockaddrs* client, irc::sockets::sockaddrs* server)
 {
 	if (from->bind_tag->getString("type") != "servers")
@@ -52,12 +51,6 @@ ModResult ModuleSpanningTree::OnAcceptConnection(int newsock, ListenSocket* from
 	return MOD_RES_DENY;
 }
 
-/** Yay for fast searches!
- * This is hundreds of times faster than recursion
- * or even scanning a linked list, especially when
- * there are more than a few servers to deal with.
- * (read as: lots).
- */
 TreeServer* SpanningTreeUtilities::FindServer(const std::string &ServerName)
 {
 	if (InspIRCd::IsSID(ServerName))
@@ -101,10 +94,7 @@ TreeServer* SpanningTreeUtilities::BestRouteTo(const std::string &ServerName)
 }
 
 /** Find the first server matching a given glob mask.
- * Theres no find-using-glob method of hash_map [awwww :-(]
- * so instead, we iterate over the list using an iterator
- * and match each one until we get a hit. Yes its slow,
- * deal with it.
+ * We iterate over the list and match each one until we get a hit.
  */
 TreeServer* SpanningTreeUtilities::FindServerMask(const std::string &ServerName)
 {
@@ -155,7 +145,7 @@ SpanningTreeUtilities::~SpanningTreeUtilities()
 	delete TreeRoot;
 }
 
-/* returns a list of DIRECT servernames for a specific channel */
+// Returns a list of DIRECT servers for a specific channel
 void SpanningTreeUtilities::GetListOfServersForChannel(Channel* c, TreeSocketSet& list, char status, const CUList& exempt_list)
 {
 	unsigned int minrank = 0;
diff --git a/src/modules/m_spanningtree/utils.h b/src/modules/m_spanningtree/utils.h
index 3a419e2a4..5aa8e925e 100644
--- a/src/modules/m_spanningtree/utils.h
+++ b/src/modules/m_spanningtree/utils.h
@@ -25,7 +25,6 @@
 #include "inspircd.h"
 #include "cachetimer.h"
 
-/* Foward declarations */
 class TreeServer;
 class TreeSocket;
 class Link;
@@ -36,8 +35,7 @@ class CmdBuilder;
 
 extern SpanningTreeUtilities* Utils;
 
-/* This hash_map holds the hash equivalent of the server
- * tree, used for rapid linear lookups.
+/** Associative container type, mapping server names/ids to TreeServers
  */
 typedef TR1NS::unordered_map<std::string, TreeServer*, irc::insensitive, irc::StrHashComp> server_hash;
 
@@ -143,7 +141,7 @@ class SpanningTreeUtilities : public classbase
 	 */
 	void GetListOfServersForChannel(Channel* c, TreeSocketSet& list, char status, const CUList& exempt_list);
 
-	/** Find a server by name
+	/** Find a server by name or SID
 	 */
 	TreeServer* FindServer(const std::string &ServerName);