From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 15 Jun 2026 08:35:34 GMT
Subject: Re: [PATCH 3/6] hash algorithms: use size_t for section lengths
Message-ID: <ai-5VmawU2MRiAHQ@pks.im>
In-Reply-To: <253d6f8004e710d05b5de1f8279d67d2220f83de.1780593313.git.gitgitgadget@gmail.com>

On Thu, Jun 04, 2026 at 05:15:09PM +0000, Philip Oakley via GitGitGadget wrote:
> diff --git a/object-file.c b/object-file.c
> index 1f5f9daf24..c648cecd80 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -581,7 +581,7 @@ static void write_object_file_prepare(const struct git_hash_algo *algo,
>  	/* Generate the header */
>  	*hdrlen = format_object_header(hdr, *hdrlen, type, len);
>  
> -	/* Sha1.. */
> +	/* Hash (function pointers) computation */
>  	hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);
>  }
>  

Thanks for updating this comment while at it :)

> diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> index 7867fd1dbf..10382a815e 100755
> --- a/t/t1007-hash-object.sh
> +++ b/t/t1007-hash-object.sh
> @@ -261,7 +261,7 @@ test_expect_success '--stdin outside of repository (uses default hash)' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
> +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
>  		'files over 4GB hash literally' '
>  	test-tool genzeros $((5*1024*1024*1024)) >big &&
>  	test_oid large5GB >expect &&

Previously we required `!LONG_IS_64BIT`, because the test would have
succeeded on platforms where it is 64 bit wide. But now that this test
works on all platforms I rather wonder whether we should completely drop
that prerequisite here, as we expect it to pass regardless of whether or
not long is 64 bit now.

Patrick