It shouldn't happen that we read duplicate entries into the same list,
but just in case make sort_ref_list drop them. If the SHA1s don't
match then die instead, as we have no way of knowing which one is the
correct one.
Signed-off-by: Julian Phillips <julian@xxxxxxxxxxxxxxxxx>
---
On Wed, 18 Apr 2007, Junio C Hamano wrote:
> Julian Phillips <julian@xxxxxxxxxxxxxxxxx> 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html