Subject: Re: [PATCH 2/5] sha1_file: fix hardcoded size in null_sha1

From: Junio C Hamano <[email-masked]>
Date: Tue, 15 Aug 2017 11:23:01 -0700

Patryk Obara <[email-masked]> writes:

> This prevents compilation error if GIT_MAX_RAWSZ is different than 20.
>
> Signed-off-by: Patryk Obara <[email-masked]>
> ---
>  sha1_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I think this is OK for "null" thing, but in general I feel
ambivalent when I see the use of "MAX" thing.

Use of "MAX" here implies that we wish to support multiple hashes at
the same time in a single binary, so that longer or shorter hashes
fit there.  But in such a case, the callers would want a union of
some sort so that they can say "I am using SHA2, give me the null
thing"?  

I said this is OK for "null" because we assume we will use ^\0{len}$
for any hash function we choose as the "impossible" value, and for
that particular use pattern, we do not need such a union.  Just
letting the caller peek at an appropriate number of bytes at the
beginning of that NUL buffer for hash the caller wants to use is
sufficient.

Thanks.

>
> diff --git a/sha1_file.c b/sha1_file.c
> index b60ae15..f5b5bec 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -32,7 +32,7 @@
>  #define SZ_FMT PRIuMAX
>  static inline uintmax_t sz_fmt(size_t s) { return s; }
>  
> -const unsigned char null_sha1[20];
> +const unsigned char null_sha1[GIT_MAX_RAWSZ];
>  const struct object_id null_oid;
>  const struct object_id empty_tree_oid = {
>  	EMPTY_TREE_SHA1_BIN_LITERAL