Re: [PATCH 18/41] index-pack: abstract away hash function constant

From: Martin Ågren <martin.agren@gmail.com>
Date: Apr 24, 2018, 09:50 UTC
Message-ID: <CAN0heSouHbAj8TbiROe=XRsBJ788Vi6P4a_Wvv=7OrdsXqQXHw@mail.gmail.com>
In-Reply-To: <20180423233951.276447-19-sandals@crustytoothpaste.net>

On 24 April 2018 at 01:39, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 18 quoted lines

> 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@crustytoothpaste.net>
> ---
>  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.

Show 8 quoted lines

>         /* The address of the 4-byte offset table */
>         idx1 = (((const uint32_t *)p->index_data)
>                 + 2 /* 8-byte header */
>                 + 256 /* fan out */
> -               + 5 * p->num_objects /* 20-byte SHA-1 table */
> +               + hashwords * p->num_objects /* object ID table */
>                 + p->num_objects /* CRC32 table */
>                 );