Re: [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 18/02/17 00:06, brian m. carlson wrote:
> There are places in the code where we would like to provide a struct
> object_id *, yet read the hash directly from the pack.  Provide an
> nth_packed_object_oid function that is similar to the
> nth_packed_object_sha1 function.
> 
> In order to avoid a potentially invalid cast, nth_packed_object_oid
> provides a variable into which to store the value, which it returns on
> success; on error, it returns NULL, as nth_packed_object_sha1 does.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  cache.h     |  6 ++++++
>  sha1_file.c | 17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/cache.h b/cache.h
> index e03a672d15..4f3bfc5ee7 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1608,6 +1608,12 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
>   * error.
>   */
>  extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
> +/*
> + * Like nth_packed_object_oid, but write the data into the object specified by
                             ^^^
... Like nth_packed_object_sha1, but ...

Having said that, if the intent is to eventually replace that function with
the new nth_packed_object_oid(), then it is probably not a good idea to
describe this function in terms of the function it will obsolete. ;-)

ATB,
Ramsay Jones

> + * the the first argument.  Returns the first argument on success, and NULL on
> + * error.
> + */
> +extern const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
>  
>  /*
>   * Return the offset of the nth object within the specified packfile.
> diff --git a/sha1_file.c b/sha1_file.c
> index ec957db5e1..777b8e8eae 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2628,6 +2628,17 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
>  	}
>  }
>  
> +const struct object_id *nth_packed_object_oid(struct object_id *oid,
> +					      struct packed_git *p,
> +					      uint32_t n)
> +{
> +	const unsigned char *hash = nth_packed_object_sha1(p, n);
> +	if (!hash)
> +		return NULL;
> +	hashcpy(oid->hash, hash);
> +	return oid;
> +}
> +
>  void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
>  {
>  	const unsigned char *ptr = vptr;
> @@ -3788,13 +3799,13 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
>  	int r = 0;
>  
>  	for (i = 0; i < p->num_objects; i++) {
> -		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
> +		struct object_id oid;
>  
> -		if (!sha1)
> +		if (!nth_packed_object_oid(&oid, p, i))
>  			return error("unable to get sha1 of object %u in %s",
>  				     i, p->pack_name);
>  
> -		r = cb(sha1, p, i, data);
> +		r = cb(oid.hash, p, i, data);
>  		if (r)
>  			break;
>  	}
>