On Tue, 17 Oct 2006, Junio C Hamano wrote: > Junio C Hamano <junkio@xxxxxxx> 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@xxxxxxx> > 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@xxxxxxx> Signed-off-by: Nicolas Pitre <nico@xxxxxxx> - 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