Re: [PATCH 3/6] hash algorithms: use size_t for section lengths
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jun 16, 2026, 14:48 UTC
- Message-ID
- <8acdcffb-e49f-12fe-ffd7-19f0799c91d4@gmx.de>
- In-Reply-To
- <ai-5VmawU2MRiAHQ@pks.im>
Hi Patrick,
On Tue, 16 Jun 2026, Patrick Steinhardt wrote:
Show 16 quoted lines
> 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 :)
It wasn't my idea, it was Claude Opus'. I would have left it as-is, but then decided that it's actually a good change and not worth splitting out into a separate commit.
Show 19 quoted lines
> > 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.
Good point!
Thank you for the review, Johannes