Re: [PATCH 10/11] packfile: use object_id in find_pack_entry_one()

[Taylor Blau <me@ttaylorr.com>]

On Fri, Oct 25, 2024 at 03:06:06AM -0400, Jeff King wrote:
> The one exception is get_delta_base() in packfile.c, when we are chasing
> a REF_DELTA from inside the pack (and thus we have a pointer directly to
> the mmap'd pack memory, not a struct). We can just bump the hashcpy()
> from inside find_pack_entry_one() to this one caller that needs it.

Makes sense.

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 0fc0680b40..0800714267 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1556,7 +1556,7 @@ static int want_object_in_pack_one(struct packed_git *p,
>  	if (p == *found_pack)
>  		offset = *found_offset;
>  	else
> -		offset = find_pack_entry_one(oid->hash, p);
> +		offset = find_pack_entry_one(oid, p);

This and all of the similar changes that follow it are trivially correct
and pleasing to read.

> diff --git a/packfile.c b/packfile.c
> index c51eab15a5..005ca670b4 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1239,7 +1239,9 @@ off_t get_delta_base(struct packed_git *p,
>  		*curpos += used;
>  	} else if (type == OBJ_REF_DELTA) {
>  		/* The base entry _must_ be in the same pack */
> -		base_offset = find_pack_entry_one(base_info, p);
> +		struct object_id oid;
> +		hashcpy(oid.hash, base_info, the_repository->hash_algo);
> +		base_offset = find_pack_entry_one(&oid, p);

Here's the one that needed to turn its bare hash into an object_id that
it can pass a pointer to. And...

> @@ -1971,20 +1973,18 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
>  	}
>  }
>
> -off_t find_pack_entry_one(const unsigned char *sha1,
> -				  struct packed_git *p)
> +off_t find_pack_entry_one(const struct object_id *oid,
> +			  struct packed_git *p)
>  {
>  	const unsigned char *index = p->index_data;
> -	struct object_id oid;
>  	uint32_t result;
>
>  	if (!index) {
>  		if (open_pack_index(p))
>  			return 0;
>  	}
>
> -	hashcpy(oid.hash, sha1, the_repository->hash_algo);

...here's the original location of that hashcpy() that moved to its only
useful caller in get_delta_base(). Makes sense.

The remaining conversions are trivial.

Thanks,
Taylor