prio-queue: factor out compare and swap operations

When manipulating the priority queue's heap, we frequently
have to compare and swap heap entries. As we are storing
only void pointers right now, this is quite easy to do
inline in a few lines. However, when we start using a more
complicated heap entry in a future patch, that will get
longer. Factoring out these operations lets us make future
changes in one place. It also makes the code a little
shorter and more readable.

Note that we actually accept indices into the queue array
instead of pointers. This is slightly less flexible than
passing pointers-to-pointers (we could not swap items from
unrelated arrays, but we would not want to), but will make
further refactoring simpler (and lets us avoid repeating
"queue->array" at each callsite, which led to some long
lines).

And finally, note that we are cleaning up an accidental use
of a "struct commit" pointer to hold a temporary entry
during swap. Even though we currently only use this code for
commits, it is supposed to be type-agnostic. In practice
this didn't matter anyway because we never dereferenced the
commit pointer (and on most systems, the pointer values
themselves are interchangeable between types).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 25 insertions, 24 deletions

diff --git a/prio-queue.c b/prio-queue.c
index c9f8c6d253..0f4fcf2755 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -1,18 +1,28 @@
 #include "cache.h"
-#include "commit.h"
 #include "prio-queue.h"
 
+static inline int compare(struct prio_queue *queue, int i, int j)
+{
+	int cmp = queue->compare(queue->array[i], queue->array[j],
+				 queue->cb_data);
+	return cmp;
+}
+
+static inline void swap(struct prio_queue *queue, int i, int j)
+{
+	void *tmp = queue->array[i];
+	queue->array[i] = queue->array[j];
+	queue->array[j] = tmp;
+}
+
 void prio_queue_reverse(struct prio_queue *queue)
 {
 	int i, j;
 
 	if (queue->compare != NULL)
 		die("BUG: prio_queue_reverse() on non-LIFO queue");
-	for (i = 0; i <= (j = (queue->nr - 1) - i); i++) {
-		struct commit *swap = queue->array[i];
-		queue->array[i] = queue->array[j];
-		queue->array[j] = swap;
-	}
+	for (i = 0; i <= (j = (queue->nr - 1) - i); i++)
+		swap(queue, i, j);
 }
 
 void clear_prio_queue(struct prio_queue *queue)
@@ -25,37 +35,32 @@ void clear_prio_queue(struct prio_queue *queue)
 
 void prio_queue_put(struct prio_queue *queue, void *thing)
 {
-	prio_queue_compare_fn compare = queue->compare;
 	int ix, parent;
 
 	/* Append at the end */
 	ALLOC_GROW(queue->array, queue->nr + 1, queue->alloc);
 	queue->array[queue->nr++] = thing;
-	if (!compare)
+	if (!queue->compare)
 		return; /* LIFO */
 
 	/* Bubble up the new one */
 	for (ix = queue->nr - 1; ix; ix = parent) {
 		parent = (ix - 1) / 2;
-		if (compare(queue->array[parent], queue->array[ix],
-			    queue->cb_data) <= 0)
+		if (compare(queue, parent, ix) <= 0)
 			break;
 
-		thing = queue->array[parent];
-		queue->array[parent] = queue->array[ix];
-		queue->array[ix] = thing;
+		swap(queue, parent, ix);
 	}
 }
 
 void *prio_queue_get(struct prio_queue *queue)
 {
-	void *result, *swap;
+	void *result;
 	int ix, child;
-	prio_queue_compare_fn compare = queue->compare;
 
 	if (!queue->nr)
 		return NULL;
-	if (!compare)
+	if (!queue->compare)
 		return queue->array[--queue->nr]; /* LIFO */
 
 	result = queue->array[0];
@@ -67,18 +72,14 @@ void *prio_queue_get(struct prio_queue *queue)
 	/* Push down the one at the root */
 	for (ix = 0; ix * 2 + 1 < queue->nr; ix = child) {
 		child = ix * 2 + 1; /* left */
-		if ((child + 1 < queue->nr) &&
-		    (compare(queue->array[child], queue->array[child + 1],
-			     queue->cb_data) >= 0))
+		if (child + 1 < queue->nr &&
+		    compare(queue, child, child + 1) >= 0)
 			child++; /* use right child */
 
-		if (compare(queue->array[ix], queue->array[child],
-			    queue->cb_data) <= 0)
+		if (compare(queue, ix, child) <= 0)
 			break;
 
-		swap = queue->array[child];
-		queue->array[child] = queue->array[ix];
-		queue->array[ix] = swap;
+		swap(queue, child, ix);
 	}
 	return result;
 }