Fix ModuleManager::SetPriority algorithm which did not handle PRIORITY_BEFORE correctly

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

2 files changed, 40 insertions, 58 deletions

diff --git a/include/modules.h b/include/modules.h
index 71cbc8480..a12f40730 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -305,7 +305,7 @@ class dynamic_reference : public dynamic_reference_base
 
 /** Priority types which can be used by Module::Prioritize()
  */
-enum Priority { PRIORITY_FIRST, PRIORITY_DONTCARE, PRIORITY_LAST, PRIORITY_BEFORE, PRIORITY_AFTER };
+enum Priority { PRIORITY_FIRST, PRIORITY_LAST, PRIORITY_BEFORE, PRIORITY_AFTER };
 
 /** Implementation-specific flags which may be set in Module::Implements()
  */
@@ -1515,16 +1515,17 @@ class CoreExport ModuleManager
 	 * PRIO_FIRST to set the event to be first called, PRIO_LAST to
 	 * set it to be the last called, or PRIO_BEFORE and PRIORITY_AFTER
 	 * to set it to be before or after one or more other modules.
-	 * @param modules If PRIO_BEFORE or PRIORITY_AFTER is set in parameter 's',
-	 * then this contains a list of one or more modules your module must be
-	 * placed before or after. Your module will be placed before the highest
-	 * priority module in this list for PRIO_BEFORE, or after the lowest
-	 * priority module in this list for PRIORITY_AFTER.
-	 * @param sz The number of modules being passed for PRIO_BEFORE and PRIORITY_AFTER.
-	 * Defaults to 1, as most of the time you will only want to prioritize your module
-	 * to be before or after one other module.
-	 */
-	bool SetPriority(Module* mod, Implementation i, Priority s, Module** modules = NULL, size_t sz = 1);
+	 * @param which If PRIO_BEFORE or PRIORITY_AFTER is set in parameter 's',
+	 * then this contains a the module that your module must be placed before
+	 * or after.
+	 */
+	bool SetPriority(Module* mod, Implementation i, Priority s, Module* which = NULL);
+
+	/** Backwards compat interface */
+	inline bool SetPriority(Module* mod, Implementation i, Priority s, Module** dptr)
+	{
+		return SetPriority(mod, i, s, *dptr);
+	}
 
 	/** Change the priority of all events in a module.
 	 * @param mod The module to set the priority of
diff --git a/src/modules.cpp b/src/modules.cpp
index 83393c2ad..a2e772ed5 100644
--- a/src/modules.cpp
+++ b/src/modules.cpp
@@ -215,7 +215,7 @@ bool ModuleManager::SetPriority(Module* mod, Priority s)
 	return true;
 }
 
-bool ModuleManager::SetPriority(Module* mod, Implementation i, Priority s, Module** modules, size_t sz)
+bool ModuleManager::SetPriority(Module* mod, Implementation i, Priority s, Module* which)
 {
 	/** To change the priority of a module, we first find its position in the vector,
 	 * then we find the position of the other modules in the vector that this module
@@ -223,10 +223,7 @@ bool ModuleManager::SetPriority(Module* mod, Implementation i, Priority s, Modul
 	 * on which they want, and we make sure our module is *at least* before or after
 	 * the first or last of this subset, depending again on the type of priority.
 	 */
-	size_t swap_pos = 0;
-	size_t source = 0;
-	bool swap = true;
-	bool found = false;
+	size_t my_pos = 0;
 
 	/* Locate our module. This is O(n) but it only occurs on module load so we're
 	 * not too bothered about it
@@ -235,81 +232,65 @@ bool ModuleManager::SetPriority(Module* mod, Implementation i, Priority s, Modul
 	{
 		if (EventHandlers[i][x] == mod)
 		{
-			source = x;
-			found = true;
-			break;
+			my_pos = x;
+			goto found_src;
 		}
 	}
 
 	/* Eh? this module doesnt exist, probably trying to set priority on an event
 	 * theyre not attached to.
 	 */
-	if (!found)
-		return false;
+	return false;
 
+found_src:
+	size_t swap_pos = my_pos;
 	switch (s)
 	{
-		/* Dummy value */
-		case PRIORITY_DONTCARE:
-			swap = false;
-		break;
-		/* Module wants to be first, sod everything else */
 		case PRIORITY_FIRST:
 			if (prioritizationState != PRIO_STATE_FIRST)
-				swap = false;
+				return true;
 			else
 				swap_pos = 0;
-		break;
-		/* Module wants to be last. */
+			break;
 		case PRIORITY_LAST:
 			if (prioritizationState != PRIO_STATE_FIRST)
-				swap = false;
-			else if (EventHandlers[i].empty())
-				swap_pos = 0;
+				return true;
 			else
 				swap_pos = EventHandlers[i].size() - 1;
-		break;
-		/* Place this module after a set of other modules */
+			break;
 		case PRIORITY_AFTER:
 		{
-			/* Find the latest possible position */
-			swap_pos = 0;
-			swap = false;
-			for (size_t x = 0; x != EventHandlers[i].size(); ++x)
+			/* Find the latest possible position, only searching AFTER our position */
+			for (size_t x = EventHandlers[i].size() - 1; x > my_pos; --x)
 			{
-				for (size_t n = 0; n < sz; ++n)
+				if (EventHandlers[i][x] == which)
 				{
-					if ((modules[n]) && (EventHandlers[i][x] == modules[n]) && (x >= swap_pos) && (source <= swap_pos))
-					{
-						swap_pos = x;
-						swap = true;
-					}
+					swap_pos = x;
+					goto swap_now;
 				}
 			}
+			// didn't find it - either not loaded or we're already after
+			return true;
 		}
-		break;
 		/* Place this module before a set of other modules */
 		case PRIORITY_BEFORE:
 		{
-			swap_pos = EventHandlers[i].size() - 1;
-			swap = false;
-			for (size_t x = 0; x != EventHandlers[i].size(); ++x)
+			for (size_t x = 0; x < my_pos; ++x)
 			{
-				for (size_t n = 0; n < sz; ++n)
+				if (EventHandlers[i][x] == which)
 				{
-					if ((modules[n]) && (EventHandlers[i][x] == modules[n]) && (x <= swap_pos) && (source >= swap_pos))
-					{
-						swap = true;
-						swap_pos = x;
-					}
+					swap_pos = x;
+					goto swap_now;
 				}
 			}
+			// didn't find it - either not loaded or we're already before
+			return true;
 		}
-		break;
 	}
 
+swap_now:
 	/* Do we need to swap? */
-	if (swap && (swap_pos != source))
+	if (swap_pos != my_pos)
 	{
 		// We are going to change positions; we'll need to run again to verify all requirements
 		if (prioritizationState == PRIO_STATE_LAST)
@@ -317,10 +298,10 @@ bool ModuleManager::SetPriority(Module* mod, Implementation i, Priority s, Modul
 		/* Suggestion from Phoenix, "shuffle" the modules to better retain call order */
 		int incrmnt = 1;
 
-		if (source > swap_pos)
+		if (my_pos > swap_pos)
 			incrmnt = -1;
 
-		for (unsigned int j = source; j != swap_pos; j += incrmnt)
+		for (unsigned int j = my_pos; j != swap_pos; j += incrmnt)
 		{
 			if (( j + incrmnt > EventHandlers[i].size() - 1) || (j + incrmnt < 0))
 				continue;