* Fixed pop_timer_heap using max heap comparisons instead of min heap comparisons and protected timer functions against improper use. (should fix bugreport:1833 bugreport:1841)

git-svn-id: https://svn.code.sf.net/p/rathena/svn/trunk@12956 54d463be-8e91-2dee-dedb-b68131a5f0ec

Changelog-Trunk.txt

@@ -3,6 +3,9 @@ Date	Added
 AS OF SVN REV. 5091, WE ARE NOW USING TRUNK.  ALL UNTESTED BUGFIXES/FEATURES GO INTO TRUNK.
 IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
 
+2008/07/15
+	* Fixed pop_timer_heap using max heap comparisons instead of min heap comparisons
+	  and protected timer functions against improper use. (should fix bugreport:1833 bugreport:1841) [FlavioJS]
 2008/07/14
 	* Made homunculi have doubled regen rates (they regen twice as fast) defined on battle configs. [Brainstorm]
 	* Corrected compiler warning and error made from r12952 and r12953, respectively. [SketchyPhoenix]

src/common/timer.c

@@ -195,12 +195,12 @@ bool pop_timer_heap(int tid)
 
 		if( timer_heap[pos] == tid )
 			break;// found the timer
-		if( left < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[left]].tick) <= 0 )
+		if( left < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[left]].tick) >= 0 )
 		{// try left child
 			pos = left;
 			continue;
 		}
-		if( right < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[right]].tick) <= 0 )
+		if( right < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[right]].tick) >= 0 )
 		{// try right child
 			pos = right;
 			continue;
@@ -214,7 +214,7 @@ bool pop_timer_heap(int tid)
 				return false;// not found
 			parent = BHEAP_PARENT(pos);
 			right = BHEAP_RIGHT(parent);
-			if( pos != right && right < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[right]].tick) <= 0 )
+			if( pos != right && right < timer_heap_num && DIFF_TICK(timer_data[tid].tick, timer_data[timer_heap[right]].tick) >= 0 )
 				break;// try this right
 			pos = parent;
 		}
@@ -294,6 +294,26 @@ bool adjust_tick(unsigned int* tick)
 	return true;
 }
 
+/// Releases a timer.
+static
+void release_timer(int tid)
+{
+	if( timer_data[tid].type == 0 )
+		return;// already released
+
+	memset(&timer_data[tid], 0, sizeof(struct TimerData));
+	if( free_timer_list_num >= free_timer_list_max )
+	{
+		free_timer_list_max += 256;
+		if( free_timer_list )
+			RECREATE(free_timer_list, int, free_timer_list_max);
+		else
+			CREATE(free_timer_list, int, free_timer_list_max);
+		memset(free_timer_list + (free_timer_list_max - 256), 0, sizeof(int)*256);
+	}
+	free_timer_list[free_timer_list_num++] = tid;
+}
+
 /// Returns a free timer id.
 static int acquire_timer(void)
 {
@@ -383,7 +403,7 @@ int add_timer_interval(unsigned int tick, TimerFunc func, int id, intptr data, i
 /// Retrieves internal timer data
 const struct TimerData* get_timer(int tid)
 {
-	if( tid >= 0 && tid < timer_data_num )
+	if( tid >= 0 && tid < timer_data_num && timer_data[tid].type != 0 )
 		return &timer_data[tid];
 	return NULL;
 }
@@ -393,7 +413,7 @@ const struct TimerData* get_timer(int tid)
 /// Returns 0 on success, < 0 on failure.
 int delete_timer(int tid, TimerFunc func)
 {
-	if( tid < 0 || tid >= timer_data_num )
+	if( tid < 0 || tid >= timer_data_num || timer_data[tid].type == 0 )
 	{
 		ShowError("delete_timer error : no such timer %d (%08x(%s))\n", tid, (int)func, search_timer_func_list(func));
 		return -1;
@@ -404,9 +424,11 @@ int delete_timer(int tid, TimerFunc func)
 		return -2;
 	}
 
-	timer_data[tid].func = NULL;
-	timer_data[tid].type = TIMER_ONCE_AUTODEL;
-
+	if( timer_data[tid].type&TIMER_REMOVE_HEAP )
+		// timer func being executed, make sure it's marked for removal when it ends
+		timer_data[tid].type = TIMER_FORCE_REMOVE|TIMER_REMOVE_HEAP;
+	else if( pop_timer_heap(tid) )
+		release_timer(tid);
 	return 0;
 }
 
@@ -421,6 +443,11 @@ int addtick_timer(int tid, unsigned int tick)
 /// Returns the new tick value, or -1 if it fails.
 int settick_timer(int tid, unsigned int tick)
 {
+	if( tid < 0 || tid >= timer_data_num || timer_data[tid].type == 0 )
+	{
+		ShowError("settick_timer error : no such timer %d\n", tid);
+		return -1;
+	}
 	if( timer_data[tid].tick == tick )
 		return tick;
 
@@ -431,7 +458,8 @@ int settick_timer(int tid, unsigned int tick)
 	}
 	pop_timer_heap(tid);
 	if( tick == -1 )
-		tick = 0;
+		tick = 0;// -1 is reserved for error
+	timer_data[tid].type &= ~TIMER_REMOVE_HEAP;
 	timer_data[tid].tick = tick;
 	push_timer_heap(tid);
 	return tick;
@@ -470,24 +498,15 @@ int do_timer(unsigned int tick)
 		}
 
 		// in the case the function didn't change anything...
-		if( timer_data[tid].type & TIMER_REMOVE_HEAP )
+		if( timer_data[tid].type & TIMER_REMOVE_HEAP || timer_data[tid].type == TIMER_FORCE_REMOVE )
 		{
 			timer_data[tid].type &= ~TIMER_REMOVE_HEAP;
 
 			switch( timer_data[tid].type )
 			{
+			case TIMER_FORCE_REMOVE:
 			case TIMER_ONCE_AUTODEL:
-				timer_data[tid].type = 0;
-				if( free_timer_list_num >= free_timer_list_max )
-				{
-					free_timer_list_max += 256;
-					if( free_timer_list )
-						RECREATE(free_timer_list, int, free_timer_list_max);
-					else
-						CREATE(free_timer_list, int, free_timer_list_max);
-					memset(free_timer_list + (free_timer_list_max - 256), 0, sizeof(int)*256);
-				}
-				free_timer_list[free_timer_list_num++] = tid;
+				release_timer(tid);
 				break;
 			case TIMER_INTERVAL:
 				if( DIFF_TICK(timer_data[tid].tick, tick) < -1000 )

src/common/timer.h

@@ -15,6 +15,7 @@
 // timer flags
 #define TIMER_ONCE_AUTODEL 0x01
 #define TIMER_INTERVAL     0x02
+#define TIMER_FORCE_REMOVE 0x04
 #define TIMER_REMOVE_HEAP  0x10
 
 // Struct declaration