Re: heads-up: git-index-pack in "next" is broken

> Junio C Hamano <junkio@cox.net> writes:
> 
> > Ah, I misread the code that uses union actually checks the type
> > in struct delta_entry (which embeds the union).  There won't be
> > any collision problem and you support both types at the same
> > time just fine.
> >
> > And your patch to compare only the first 20-bytes makes sense
> > (assuming ulong is always shorter than 20-bytes which I think is
> > safe to assume).
> 
> Does this sound fair (the code is yours, just asking about the
> log message)?
> 
> If we really wanted to be purist, we could run comparison with
> the union and obj->type as two keys, but I do not think it is
> worth it.
> 
> -- >8 --
> From: Nicolas Pitre <nico@cam.org>
> Date: Tue, 17 Oct 2006 16:23:26 -0400
> Subject: [PATCH] index-pack: compare the first 20-bytes of the key.
> 
> 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: Junio C Hamano <junkio@cox.net>