On Tue, Apr 24, 2018 at 11:50:16AM +0200, Martin Ågren wrote:
> On 24 April 2018 at 01:39, brian m. carlson
> <sandals@xxxxxxxxxxxxxxxxxxxx> 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 <sandals@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > 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
Attachment:
signature.asc
Description: PGP signature
