Re: [PATCH 2/2] Correct priority of lightweight tags in git-describe.

["Shawn O. Pearce" <spearce@spearce.org>]

Junio C Hamano <junkio@cox.net> wrote:
> How about this on top?
> 
>  * We seem to do "a_" more often than "_a" for parameter names
>    we type-cast.

I copied the header from the compare_names routine in
builtin-describe.c.  Maybe apply this too?

-- >8 --
diff --git a/builtin-describe.c b/builtin-describe.c
index e38c899..e514bc3 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -104,10 +104,10 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	return 0;
 }
 
-static int compare_names(const void *_a, const void *_b)
+static int compare_names(const void *a_, const void *b_)
 {
-	struct commit_name *a = *(struct commit_name **)_a;
-	struct commit_name *b = *(struct commit_name **)_b;
+	struct commit_name *a = *(struct commit_name **)a_;
+	struct commit_name *b = *(struct commit_name **)b_;
 	unsigned long a_date = a->commit->date;
 	unsigned long b_date = b->commit->date;
 	int cmp = hashcmp(a->commit->object.sha1, b->commit->object.sha1);
-- <8 --

>  * int would be enough for 'depth', not long.  Also, "return
>    (a->depth - b->depth)" is kosher only when it is signed,
>    although it works in practice on sane platforms.

I originally went with unsigned long as that matches the size
of a packfile, and I wasn't using depth in a signed way.  Until
that patch.  Good catch.

>  * I did not find mergesort(); if we want stable, explicitly do
>    so.  In practice, qsort() seems stable (as you know qsort()
>    does not have to be implemented as quicksort).

Must be a *BSD extension.  Oops.


Patch looked good.  Thanks.

-- 
Shawn.