[PATCH] refs.c: drop duplicate entries in sort_ref_list

From

Julian Phillips <julian@quantumfyre.co.uk>

Date

Apr 18, 2007, 21:27 UTC

Message-ID

<20070418212749.4842.62822.julian@quantumfyre.co.uk>

In-Reply-To

<7vodllfpaj.fsf@assigned-by-dhcp.cox.net>

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---

On Wed, 18 Apr 2007, Junio C Hamano wrote:

> Julian Phillips <julian@quantumfyre.co.uk> writes:
>
>> On Tue, 17 Apr 2007, Junio C Hamano wrote:
>> ...
>>> I think we would not call add_ref() to the same list with
>>> duplicate names, unless (1) filesystem is grossly corrupt, (2)
>>> somebody added a new ref while we are walking (how does
>>> readdir() behave in such a case???), or (3) packed-refs file is
>>> corrupt.
>>
>> This combined with the fact that the old code didn't check that the
>> sha1 was the same suggests to me that this behaviour may actually have
>> been a subtle bug?  Perhaps the best thing to do is die if we find two
>> entries with the same name when sorting?
>
> I am not sure what readdir() does if somebody adds a new ref
> while we are walking the directory; I am hoping we would not get
> the same thing in duplicates, but I dunno.
>
> I think the most sensible thing to do is to check for
> duplicates, discarding if the SHA-1 match and otherwise dying.
>

Something like this (on top of my previous patch)?

 refs.c |   34 ++++++++++++++++++++++++++--------
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 23982fc..5736a0e 100644
--- a/refs.c
+++ b/refs.c
@@ -65,7 +65,7 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
 /* merge sort the ref list */
 static struct ref_list *sort_ref_list(struct ref_list *list)
 {
-	int psize, qsize, last_merge_count;
+	int psize, qsize, last_merge_count, cmp;
 	struct ref_list *p, *q, *l, *e;
 	struct ref_list *new_list = list;
 	int k = 1;
@@ -103,14 +103,32 @@ static struct ref_list *sort_ref_list(struct ref_list *list)
 					e = q;
 					q = q->next;
 					qsize--;
-				} else if (strcmp(q->name, p->name) < 0) {
-					e = q;
-					q = q->next;
-					qsize--;
 				} else {
-					e = p;
-					p = p->next;
-					psize--;
+					cmp = strcmp(q->name, p->name);
+					if (cmp < 0) {
+						e = q;
+						q = q->next;
+						qsize--;
+					} else if (cmp > 0) {
+						e = p;
+						p = p->next;
+						psize--;
+					} else {
+						if (hashcmp(q->sha1, p->sha1))
+							die("Duplicated ref, "
+							    "and SHA1s don't "
+							    "match: %s",
+							    q->name);
+						warning("Duplicated ref: %s",
+							q->name);
+						e = q;
+						q = q->next;
+						qsize--;
+						free(e);
+						e = p;
+						p = p->next;
+						psize--;
+					}
 				}
 
 				e->next = NULL;

-- 
1.5.1.1