On Tue, Apr 24, 2018 at 11:50:16AM +0200, Martin Ă…gren wrote: > On 24 April 2018 at 01:39, brian m. carlson > wrote: > > The code for reading certain pack v2 offsets had a hard-coded 5 > > representing the number of uint32_t words that we needed to skip over. > > Specify this value in terms of a value from the_hash_algo. > > > > Signed-off-by: brian m. carlson > > --- > > builtin/index-pack.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/builtin/index-pack.c b/builtin/index-pack.c > > index d81473e722..c1f94a7da6 100644 > > --- a/builtin/index-pack.c > > +++ b/builtin/index-pack.c > > @@ -1543,12 +1543,13 @@ static void read_v2_anomalous_offsets(struct packed_git *p, > > { > > const uint32_t *idx1, *idx2; > > uint32_t i; > > + const uint32_t hashwords = the_hash_algo->rawsz / sizeof(uint32_t); > > Should we round up? Or just what should we do if a length is not > divisible by 4? (I am not aware of any such hash functions, but one > could exist for all I know.) Another question is whether such an > index-pack v2 will ever contain non-SHA-1 oids to begin with. I can't > find anything suggesting that it could, but this is unfamiliar code to > me. I opted not to simply because I know that our current hash is 20 bytes and the new one will be 32, and I know those are both divisible by 4. I feel confident that any future hash we choose will also be divisible by 4, and the code is going to be complicated if it isn't. I agree that pack v2 is not going to have anything but SHA-1. However, writing all the code such that it's algorithm agnostic means that we can do testing of new algorithms by wholesale replacing the algorithm with a new one, which simplifies things considerably. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204