(sort of..) Revert commit 7736e920fe6ccaf9ad1984afc70db19efd4fb8cd - remove dupe checking on connect classes. Now, we just delete ones with refcount 0 on rehash, and add new ones. Connection can only match a non-disabled class, so most of the braindamage should be gone from here, without causing wild pointer fun..

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@10701 e03df62e-2008-0410-955e-edbf42e46eb7

3 files changed, 55 insertions, 36 deletions

diff --git a/include/users.h b/include/users.h
index 48026d473..83782d393 100644
--- a/include/users.h
+++ b/include/users.h
@@ -139,14 +139,14 @@ public:
 		registration_timeout(source->registration_timeout), flood(source->flood), host(source->host),
 		pingtime(source->pingtime), pass(source->pass), hash(source->hash), threshold(source->threshold), sendqmax(source->sendqmax),
 		recvqmax(source->recvqmax), maxlocal(source->maxlocal), maxglobal(source->maxglobal), maxchans(source->maxchans),
-		port(source->port), RefCount(0), limit(source->limit)
+		port(source->port), RefCount(0), disabled(false), limit(source->limit)
 	{
 	}
 
 	/** Create a new connect class with no settings.
 	 */
 	ConnectClass() : type(CC_DENY), name("unnamed"), registration_timeout(0), flood(0), host(""), pingtime(0), pass(""), hash(""),
-			threshold(0), sendqmax(0), recvqmax(0), maxlocal(0), maxglobal(0), RefCount(0), limit(0)
+			threshold(0), sendqmax(0), recvqmax(0), maxlocal(0), maxglobal(0), RefCount(0), disabled(false), limit(0)
 	{
 	}
 
@@ -168,14 +168,14 @@ public:
 			const std::string &pas, const std::string &hsh, unsigned int thres, unsigned long sendq, unsigned long recvq,
 			unsigned long maxl, unsigned long maxg, unsigned int maxc, int p = 0) :
 			type(CC_ALLOW), name(thename), registration_timeout(timeout), flood(fld), host(hst), pingtime(ping), pass(pas), hash(hsh),
-			threshold(thres), sendqmax(sendq), recvqmax(recvq), maxlocal(maxl), maxglobal(maxg), maxchans(maxc), port(p), RefCount(0), limit(0) { }
+			threshold(thres), sendqmax(sendq), recvqmax(recvq), maxlocal(maxl), maxglobal(maxg), maxchans(maxc), port(p), RefCount(0), disabled(false), limit(0) { }
 
 	/** Create a new connect class to DENY connections
 	 * @param thename Name of the connect class
 	 * @param hst The IP mask to deny
 	 */
 	ConnectClass(const std::string &thename, const std::string &hst) : type(CC_DENY), name(thename), registration_timeout(0),
-			flood(0), host(hst), pingtime(0), pass(""), hash(""), threshold(0), sendqmax(0), recvqmax(0), maxlocal(0), maxglobal(0), maxchans(0), port(0), RefCount(0), limit(0)
+			flood(0), host(hst), pingtime(0), pass(""), hash(""), threshold(0), sendqmax(0), recvqmax(0), maxlocal(0), maxglobal(0), maxchans(0), port(0), RefCount(0), disabled(false), limit(0)
 	{
 	}
 
@@ -187,10 +187,20 @@ public:
 				registration_timeout(source->registration_timeout), flood(source->flood), host(source->host),
 				pingtime(source->pingtime), pass(source->pass), hash(source->hash), threshold(source->threshold), sendqmax(source->sendqmax),
 				recvqmax(source->recvqmax), maxlocal(source->maxlocal), maxglobal(source->maxglobal), maxchans(source->maxchans),
-				port(source->port), RefCount(0), limit(source->limit)
+				port(source->port), RefCount(0), disabled(false), limit(source->limit)
 	{
 	}
 
+	void SetDisabled(bool t)
+	{
+		this->disabled = t;
+	}
+
+	bool GetDisabled()
+	{
+		return this->disabled;
+	}
+
 	/* Update an existing entry with new values
 	 */
 	void Update(unsigned int timeout, unsigned int fld, const std::string &hst, unsigned int ping,
@@ -237,6 +247,10 @@ public:
 	 */
 	unsigned long RefCount;
 
+	/** If this is true, any attempt to set a user to this class will fail. Default false. This is really private, it's only in the public section thanks to the way this class is written
+	 */
+	bool disabled;
+
 	/** How many users may be in this connect class before they are refused? (0 = disabled = default)
 	 */
 	unsigned long limit;
diff --git a/src/configreader.cpp b/src/configreader.cpp
index aea422a35..5449a12f6 100644
--- a/src/configreader.cpp
+++ b/src/configreader.cpp
@@ -457,15 +457,33 @@ bool InitConnect(ServerConfig* conf, const char*)
 {
 	conf->GetInstance()->Logs->Log("CONFIG",DEFAULT,"Reading connect classes... class list is:");
 
-	/*
-	 * Remove all connect classes.. we'll reset the pointers in user classes
-	 * once all new classes have been read from config.
-	 */
-	while (conf->Classes.begin() != conf->Classes.end())
+	for (ClassVector::iterator i = conf->Classes.begin(); i != conf->Classes.end() ; )
 	{
-		ConnectClass *c = *(conf->Classes.begin());
-		conf->Classes.erase(conf->Classes.begin());
-		delete c;
+		ConnectClass* c = *i;
+
+		/*
+		 * only delete a class with refcount 0.
+		 * this is needed to avoid trampling on a wild pointer (User::MyClass)!
+		 * it's also the most simple way to do it, given that we're looking at threads..
+		 * -- w00t
+		 */
+		if (c->RefCount == 0)
+		{
+			conf->GetInstance()->Logs->Log("CONFIG",DEFAULT, "Removing connect class, refcount is 0!");
+			
+			/* This was causing a crash, because we'd set i to .begin() just here, but then the for loop's increment would
+			 * set it to .begin() + 1. Which if it was already the last thing in the list, wasn't good.
+			 * Now the increment is in the else { } below.
+			 */
+			conf->Classes.erase(i);
+			i = conf->Classes.begin(); // start over so we don't trample on a bad iterator
+		}
+		else
+		{
+			/* also mark all existing classes disabled, if they still exist in the conf, they will be reenabled. */
+			c->SetDisabled(true);
+			i++;
+		}
 	}
 
 	return true;
@@ -562,29 +580,6 @@ bool DoConnect(ServerConfig* conf, const char*, char**, ValueList &values, int*)
  */
 bool DoneConnect(ServerConfig *conf, const char*)
 {
-	/*
-	 * Update connect classes on all users.
-	 */
-	conf->GetInstance()->Logs->Log("CONFIG",DEFAULT, "Resetting connect classes for users...");
-  	for (std::vector<User*>::iterator n = conf->GetInstance()->Users->local_users.begin(); n != conf->GetInstance()->Users->local_users.end(); n++)
-	{
-		User *u = *n;
-
-		/*
-		 * Make their existing class go away so that SetClass doesn't touch a wild ptr, important!
-		 */
-		u->MyClass = NULL;
-
-		u->SetClass();
-
-		/*
-		 * Check that the user falls into a valid class block.. if they don't,
-		 * they need to be quit, which CheckClass will do. -- w00t
-		 */
-		u->CheckClass();
-	}
-
-
 	conf->GetInstance()->Logs->Log("CONFIG",DEFAULT, "Done adding connect classes!");
 	return true;
 }
diff --git a/src/users.cpp b/src/users.cpp
index 2b6e00e0e..784783086 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -1830,6 +1830,9 @@ ConnectClass* User::SetClass(const std::string &explicit_name)
 		{
 			ConnectClass* c = *i;
 
+			if (c->GetDisabled())
+				continue; // can't possibly match, removed from conf
+
 			if (explicit_name == c->GetName())
 			{
 				ServerInstance->Logs->Log("CONNECTCLASS", DEBUG, "Explicitly set to %s", explicit_name.c_str());
@@ -1852,6 +1855,13 @@ ConnectClass* User::SetClass(const std::string &explicit_name)
 				ServerInstance->Logs->Log("CONNECTCLASS", DEBUG, "DENY %s %d %s", c->GetHost().c_str(), c->GetPort(), c->GetName().c_str());
 			}
 
+			/* if it's disabled, we can't match this one. */
+			if (c->GetDisabled())
+			{
+				ServerInstance->Logs->Log("CONNECTCLASS", DEBUG, "Class disabled");
+				continue;
+			}
+
 			/* check if host matches.. */
 			if (!InspIRCd::MatchCIDR(this->GetIPString(), c->GetHost(), NULL) && 
 			    !InspIRCd::MatchCIDR(this->host, c->GetHost(), NULL))