Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2017, 00:47 UTC
Message-ID: <xmqqpoen2p2a.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170530173109.54904-26-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 26 quoted lines

> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  notes-merge.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/notes-merge.c b/notes-merge.c
> index 55dbb3659..962e9b1bc 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
>  	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
>  }
>  
> -static int path_to_sha1(const char *path, unsigned char *sha1)
> +static int path_to_oid(const char *path, struct object_id *oid)
>  {
> -	char hex_sha1[40];
> +	char hex_oid[GIT_SHA1_HEXSZ];
>  	int i = 0;
> -	while (*path && i < 40) {
> +	while (*path && i < GIT_SHA1_HEXSZ) {
>  		if (*path != '/')
> -			hex_sha1[i++] = *path;
> +			hex_oid[i++] = *path;
>  		path++;
>  	}

It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the above, but ...

> -	if (*path || i != 40)
> +	if (*path || i != GIT_SHA1_HEXSZ)
>  		return -1;

... this one is tricky.

What's in our envisioned future? Are we expecing to see object names, named with two or more hash functions, in a same repository? If so, and one is 20 bytes and another one is 32 bytes, then this should check 'i' against 40 and 64 and pass if 'i' is one of these expected lengths?

> -	return get_sha1_hex(hex_sha1, sha1);
> +	return get_oid_hex(hex_oid, oid);
>  }