index-pack: compare only the first 20-bytes of the key.
authorNicolas Pitre <nico@cam.org>
Tue, 17 Oct 2006 20:23:26 +0000 (16:23 -0400)
committerJunio C Hamano <junkio@cox.net>
Wed, 18 Oct 2006 17:07:49 +0000 (10:07 -0700)
The "union delta_base" is a strange beast.  It is a 20-byte
binary blob key to search a binary searchable deltas[] array,
each element of which uses it to represent its base object with
either a full 20-byte SHA-1 or an offset in the pack.  Which
representation is used is determined by another field of the
deltas[] array element, obj->type, so there is no room for
confusion, as long as we make sure we compare the keys for the
same type only with appropriate length.  The code compared the
full union with memcmp().

When storing the in-pack offset, the union was first cleared
before storing an unsigned long, so comparison worked fine.

On 64-bit architectures, however, the union typically is 24-byte
long; the code did not clear the remaining 4-byte alignment
padding when storing a full 20-byte SHA-1 representation.  Using
memcmp() to compare the whole union was wrong.

This fixes the comparison to look at the first 20-bytes of the
union, regardless of the architecture.  As long as ulong is
smaller than 20-bytes this works fine.

Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
index-pack.c

index fffddd25c9b9cc92b837434d7099cb7b4180ce0b..56c590e3fae5d387c2ffe42336c87b43d252fdb2 100644 (file)
@@ -23,6 +23,12 @@ union delta_base {
        unsigned long offset;
 };
 
+/*
+ * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want
+ * to memcmp() only the first 20 bytes.
+ */
+#define UNION_BASE_SZ  20
+
 struct delta_entry
 {
        struct object_entry *obj;
@@ -211,7 +217,7 @@ static int find_delta(const union delta_base *base)
                 struct delta_entry *delta = &deltas[next];
                 int cmp;
 
-                cmp = memcmp(base, &delta->base, sizeof(*base));
+                cmp = memcmp(base, &delta->base, UNION_BASE_SZ);
                 if (!cmp)
                         return next;
                 if (cmp < 0) {
@@ -232,9 +238,9 @@ static int find_delta_childs(const union delta_base *base,
 
        if (first < 0)
                return -1;
-       while (first > 0 && !memcmp(&deltas[first - 1].base, base, sizeof(*base)))
+       while (first > 0 && !memcmp(&deltas[first - 1].base, base, UNION_BASE_SZ))
                --first;
-       while (last < end && !memcmp(&deltas[last + 1].base, base, sizeof(*base)))
+       while (last < end && !memcmp(&deltas[last + 1].base, base, UNION_BASE_SZ))
                ++last;
        *first_index = first;
        *last_index = last;
@@ -312,7 +318,7 @@ static int compare_delta_entry(const void *a, const void *b)
 {
        const struct delta_entry *delta_a = a;
        const struct delta_entry *delta_b = b;
-       return memcmp(&delta_a->base, &delta_b->base, sizeof(union delta_base));
+       return memcmp(&delta_a->base, &delta_b->base, UNION_BASE_SZ);
 }
 
 static void parse_pack_objects(void)