Skip to content

Commit

Permalink
index-pack: compare only the first 20-bytes of the key.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Nicolas Pitre authored and Junio C Hamano committed Oct 18, 2006
1 parent b6945f5 commit 3c55287
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions index-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3c55287

Please sign in to comment.