[BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[rsbecker]

In RC0, the following tests are failing (with verbose). They look like the
same root cause. Unpack("Q>".... What version does git now require for perl?
I have v5.30.3 available, nothing more recent.

expecting success of 4216.141 'Bloom reader notices too-small data chunk': 
check_corrupt_graph BDAT clear 00000000 &&
echo "warning: ignoring too-small changed-path chunk" \
"(4 < 12) in commit-graph file" >expect.err &&
test_cmp expect.err err

Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 141 - Bloom reader notices too-small data chunk
#
#check_corrupt_graph BDAT clear 00000000 &&
#echo "warning: ignoring too-small changed-path chunk" \
#"(4 < 12) in commit-graph file" >expect.err &&
#test_cmp expect.err err
#

expecting success of 4216.142 'Bloom reader notices out-of-bounds filter
offsets': 
check_corrupt_graph BIDX 12 FFFFFFFF &&
# use grep to avoid depending on exact chunk size
grep "warning: ignoring out-of-range offset (4294967295) for changed-path
filter at pos 3 of .git/objects/info/commit-graph" err

Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 142 - Bloom reader notices out-of-bounds filter offsets
#
#check_corrupt_graph BIDX 12 FFFFFFFF &&
## use grep to avoid depending on exact chunk size
#grep "warning: ignoring out-of-range offset (4294967295) for changed-path
filter at pos 3 of .git/objects/info/commit-graph" err
#

expecting success of 4216.143 'Bloom reader notices too-small index chunk': 
# replace the index with a single entry, making most
# lookups out-of-bounds
check_corrupt_graph BIDX clear 00000000 &&
echo "warning: commit-graph changed-path index chunk" \
"is too small" >expect.err &&
test_cmp expect.err err

Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 143 - Bloom reader notices too-small index chunk
#
## replace the index with a single entry, making most
## lookups out-of-bounds
#check_corrupt_graph BIDX clear 00000000 &&
#echo "warning: commit-graph changed-path index chunk" \
#"is too small" >expect.err &&
#test_cmp expect.err err
#

expecting success of 4216.144 'Bloom reader notices out-of-order index
offsets': 
# we do not know any real offsets, but we can pick
# something plausible; we should not get to the point of
# actually reading from the bogus offsets anyway.
corrupt_graph BIDX 4 0000000c00000005 &&
echo "warning: ignoring decreasing changed-path index offsets" \
"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>expect.err &&
git -c core.commitGraph=false log -- A/B/file2 >expect.out &&
git -c core.commitGraph=true log -- A/B/file2 >out 2>err &&
test_cmp expect.out out &&
test_cmp expect.err err

Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 144 - Bloom reader notices out-of-order index offsets
#
## we do not know any real offsets, but we can pick
## something plausible; we should not get to the point of
## actually reading from the bogus offsets anyway.
#corrupt_graph BIDX 4 0000000c00000005 &&
#echo "warning: ignoring decreasing changed-path index offsets" \
#"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>expect.err &&
#git -c core.commitGraph=false log -- A/B/file2 >expect.out &&
#git -c core.commitGraph=true log -- A/B/file2 >out 2>err &&
#test_cmp expect.out out &&
#test_cmp expect.err err
#

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.

RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[rsbecker]

On Friday, November 3, 2023 10:50 AM, I wrote:
>In RC0, the following tests are failing (with verbose). They look like the
same root
>cause. Unpack("Q>".... What version does git now require for perl?
>I have v5.30.3 available, nothing more recent.
>
>expecting success of 4216.141 'Bloom reader notices too-small data chunk':
>check_corrupt_graph BDAT clear 00000000 && echo "warning: ignoring
too-small
>changed-path chunk" \
>"(4 < 12) in commit-graph file" >expect.err && test_cmp expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 141 - Bloom reader notices too-small data chunk #
#check_corrupt_graph
>BDAT clear 00000000 && #echo "warning: ignoring too-small changed-path
chunk"
>\
>#"(4 < 12) in commit-graph file" >expect.err && #test_cmp expect.err err #
>
>expecting success of 4216.142 'Bloom reader notices out-of-bounds filter
>offsets':
>check_corrupt_graph BIDX 12 FFFFFFFF &&
># use grep to avoid depending on exact chunk size grep "warning: ignoring
out-of-
>range offset (4294967295) for changed-path filter at pos 3 of
>.git/objects/info/commit-graph" err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 142 - Bloom reader notices out-of-bounds filter offsets #
>#check_corrupt_graph BIDX 12 FFFFFFFF && ## use grep to avoid depending on
>exact chunk size #grep "warning: ignoring out-of-range offset (4294967295)
for
>changed-path filter at pos 3 of .git/objects/info/commit-graph" err #
>
>expecting success of 4216.143 'Bloom reader notices too-small index chunk':
># replace the index with a single entry, making most # lookups
out-of-bounds
>check_corrupt_graph BIDX clear 00000000 && echo "warning: commit-graph
>changed-path index chunk" \ "is too small" >expect.err && test_cmp
expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 143 - Bloom reader notices too-small index chunk # ## replace the
index with
>a single entry, making most ## lookups out-of-bounds #check_corrupt_graph
BIDX
>clear 00000000 && #echo "warning: commit-graph changed-path index chunk" \
>#"is too small" >expect.err && #test_cmp expect.err err #
>
>expecting success of 4216.144 'Bloom reader notices out-of-order index
>offsets':
># we do not know any real offsets, but we can pick # something plausible;
we
>should not get to the point of # actually reading from the bogus offsets
anyway.
>corrupt_graph BIDX 4 0000000c00000005 && echo "warning: ignoring decreasing
>changed-path index offsets" \
>"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>>expect.err &&
>git -c core.commitGraph=false log -- A/B/file2 >expect.out && git -c
>core.commitGraph=true log -- A/B/file2 >out 2>err && test_cmp expect.out
out &&
>test_cmp expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 144 - Bloom reader notices out-of-order index offsets # ## we do not
know
>any real offsets, but we can pick ## something plausible; we should not get
to the
>point of ## actually reading from the bogus offsets anyway.
>#corrupt_graph BIDX 4 0000000c00000005 && #echo "warning: ignoring
>decreasing changed-path index offsets" \
>#"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>>expect.err &&
>#git -c core.commitGraph=false log -- A/B/file2 >expect.out && #git -c
>core.commitGraph=true log -- A/B/file2 >out 2>err && #test_cmp expect.out
out
>&& #test_cmp expect.err err #
>

This same problem also happens in t5318, t5319, t5324
--Randall

Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[Jeff King]

On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:

> In RC0, the following tests are failing (with verbose). They look like the
> same root cause. Unpack("Q>".... What version does git now require for perl?
> I have v5.30.3 available, nothing more recent.

The perl used in the test suite is supposed to be vanilla enough to
support any ancient version. The perl5 Git import doesn't have version
tags that go back that far, but the quadwords in pack/unpack go back at
least to a commit from 1998.

So I suspect this is not a version issue, but rather a build-time config
one. The docs say:

  Q  An unsigned quad value.
      (Quads are available only if your system supports 64-bit integer
      values _and_ if Perl has been compiled to support those.  Raises
      an exception otherwise.)

It would probably be possible to rewrite the use of "Q" here to grab two
32-bit values instead. But I'd guess that on your system it is not as
simple as a shift-and-add to then treat them as a 64-bit value, since
presumably the problem is that perl's ints are all strictly 32-bit.

What does this script produce for you:

  perl -e '
    my $bytes = "\1\2\3\4\5\6\7\8";
    my $q = eval { unpack("Q>", $bytes) };
    print "Q = ", defined($q) ? $q : "($@)", "\n";
    my ($n1, $n2) = unpack("NN", $bytes);
    print "n1 = $n1\n";
    print "n2 = $n2\n";
    print "computed quad = ", ($n1 << 32) | $n2, "\n";
  '

I get:

  Q = 72623859790382904
  n1 = 16909060
  n2 = 84281144
  computed quad = 72623859790382904

but I'm guessing you get an exception report for Q, and that the
computed quad is probably equal to n2 (the shift of n1 goes totally off
the end).

We may not be without hope, though. These 64-bit values are file offsets
we're reading from the chunk files. The format naturally uses 64-bit
values here to accommodate arbitrarily large files. But in our tests,
the offsets are all going to be relatively small. So our "$n1" in
practice will always be 0.

> This same problem also happens in t5318, t5319, t5324

Yep. The offending code is in lib-chunk.sh, so the new tests added in
all of those scripts which use it will run into the same problem.

-Peff

RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[rsbecker]

On Jeff King <peff@peff.net> wrote:
>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:
>> In RC0, the following tests are failing (with verbose). They look like
>> the same root cause. Unpack("Q>".... What version does git now require for perl?
>> I have v5.30.3 available, nothing more recent.
>
>The perl used in the test suite is supposed to be vanilla enough to support any
>ancient version. The perl5 Git import doesn't have version tags that go back that far,
>but the quadwords in pack/unpack go back at least to a commit from 1998.
>
>So I suspect this is not a version issue, but rather a build-time config one. The docs
>say:
>
>  Q  An unsigned quad value.
>      (Quads are available only if your system supports 64-bit integer
>      values _and_ if Perl has been compiled to support those.  Raises
>      an exception otherwise.)
>
>It would probably be possible to rewrite the use of "Q" here to grab two 32-bit
>values instead. But I'd guess that on your system it is not as simple as a shift-and-
>add to then treat them as a 64-bit value, since presumably the problem is that perl's
>ints are all strictly 32-bit.
>
>What does this script produce for you:
>
>  perl -e '
>    my $bytes = "\1\2\3\4\5\6\7\8";
>    my $q = eval { unpack("Q>", $bytes) };
>    print "Q = ", defined($q) ? $q : "($@)", "\n";
>    my ($n1, $n2) = unpack("NN", $bytes);
>    print "n1 = $n1\n";
>    print "n2 = $n2\n";
>    print "computed quad = ", ($n1 << 32) | $n2, "\n";
>  '
>
>I get:
>
>  Q = 72623859790382904
>  n1 = 16909060
>  n2 = 84281144
>  computed quad = 72623859790382904
>
>but I'm guessing you get an exception report for Q, and that the computed quad is
>probably equal to n2 (the shift of n1 goes totally off the end).
>
>We may not be without hope, though. These 64-bit values are file offsets we're
>reading from the chunk files. The format naturally uses 64-bit values here to
>accommodate arbitrarily large files. But in our tests, the offsets are all going to be
>relatively small. So our "$n1" in practice will always be 0.
>
>> This same problem also happens in t5318, t5319, t5324
>
>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those
>scripts which use it will run into the same problem.

Explains a lot. We are only able to build git in 32-bit mode because of OS dependencies (only available in 32-bit). I did not know that 64-bit was now required for git. We will get there, but it will probably take years.

RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[rsbecker]

On November 3, 2023, Jeff King <peff@peff.net> wrote:
>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:
>
>> In RC0, the following tests are failing (with verbose). They look like
>> the same root cause. Unpack("Q>".... What version does git now require for perl?
>> I have v5.30.3 available, nothing more recent.
>
>The perl used in the test suite is supposed to be vanilla enough to support any
>ancient version. The perl5 Git import doesn't have version tags that go back that far,
>but the quadwords in pack/unpack go back at least to a commit from 1998.
>
>So I suspect this is not a version issue, but rather a build-time config one. The docs
>say:
>
>  Q  An unsigned quad value.
>      (Quads are available only if your system supports 64-bit integer
>      values _and_ if Perl has been compiled to support those.  Raises
>      an exception otherwise.)
>
>It would probably be possible to rewrite the use of "Q" here to grab two 32-bit
>values instead. But I'd guess that on your system it is not as simple as a shift-and-
>add to then treat them as a 64-bit value, since presumably the problem is that perl's
>ints are all strictly 32-bit.
>
>What does this script produce for you:
>
>  perl -e '
>    my $bytes = "\1\2\3\4\5\6\7\8";
>    my $q = eval { unpack("Q>", $bytes) };
>    print "Q = ", defined($q) ? $q : "($@)", "\n";
>    my ($n1, $n2) = unpack("NN", $bytes);
>    print "n1 = $n1\n";
>    print "n2 = $n2\n";
>    print "computed quad = ", ($n1 << 32) | $n2, "\n";
>  '
>
>I get:
>
>  Q = 72623859790382904
>  n1 = 16909060
>  n2 = 84281144
>  computed quad = 72623859790382904
>
>but I'm guessing you get an exception report for Q, and that the computed quad is
>probably equal to n2 (the shift of n1 goes totally off the end).
>
>We may not be without hope, though. These 64-bit values are file offsets we're
>reading from the chunk files. The format naturally uses 64-bit values here to
>accommodate arbitrarily large files. But in our tests, the offsets are all going to be
>relatively small. So our "$n1" in practice will always be 0.
>
>> This same problem also happens in t5318, t5319, t5324
>
>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those
>scripts which use it will run into the same problem.

What I get from Perl is 
$ perl -e '
>     my $bytes = "\1\2\3\4\5\6\7\8";
>     my $q = eval { unpack("Q>", $bytes) };
>     print "Q = ", defined($q) ? $q : "($@)", "\n";
>     my ($n1, $n2) = unpack("NN", $bytes);
>     print "n1 = $n1\n";
>     print "n2 = $n2\n";
>     print "computed quad = ", ($n1 << 32) | $n2, "\n";  '
Q = (Invalid type 'Q' in unpack at -e line 3.
)
n1 = 16909060
n2 = 84281144
computed quad = 84281144

Because perl itself is 32-bit, not 64-bit on this platform. So even moving git to 64-bit will not correct the issue.

[PATCH] t: avoid perl's pack/unpack "Q" specifier

[Jeff King]

On Fri, Nov 03, 2023 at 12:01:22PM -0400, rsbecker@nexbridge.com wrote:

> Explains a lot. We are only able to build git in 32-bit mode because
> of OS dependencies (only available in 32-bit). I did not know that
> 64-bit was now required for git. We will get there, but it will
> probably take years.

It's not required for Git. And even on 32-bit systems there is usually
some compiler support for 64-bit types via "uint64_t", etc. And likewise
perl can probably be built for 64-bit types even on a 32-bit system (the
tests do pass on our Linux 32-bit build), but your perl simply wasn't.

I think we can accommodate it, though. Did you try the snippet I showed?

If it behaves as I think it does, then this patch should hopefully fix
things for you:

-- >8 --
Subject: [PATCH] t: avoid perl's pack/unpack "Q" specifier

The perl script introduced by 86b008ee61 (t: add library for munging
chunk-format files, 2023-10-09) uses pack("Q") and unpack("Q") to read
and write 64-bit values ("quadwords" in perl parlance) from the on-disk
chunk files. However, some builds of perl may not support 64-bit
integers at all, and throw an exception here. While some 32-bit
platforms may still support 64-bit integers in perl (such as our linux32
CI environment), others reportedly don't (the NonStop 32-bit builds).

We can work around this by treating the 64-bit values as two 32-bit
values. We can't ever combine them into a single 64-bit value, but in
practice this is OK. These are representing file offsets, and our files
are much smaller than 4GB. So the upper half of the 64-bit value will
always be 0.

We can just introduce a few helper functions which perform the
translation and double-check our assumptions.

Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
Signed-off-by: Jeff King <peff@peff.net>
---
If this does work, it should go on top of jk/chunk-bounds.

 t/lib-chunk/corrupt-chunk-file.pl | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/t/lib-chunk/corrupt-chunk-file.pl b/t/lib-chunk/corrupt-chunk-file.pl
index cd6d386fef..b024bbdcb5 100644
--- a/t/lib-chunk/corrupt-chunk-file.pl
+++ b/t/lib-chunk/corrupt-chunk-file.pl
@@ -21,14 +21,38 @@ sub copy {
 	return $buf;
 }
 
+# Some platforms' perl builds don't support 64-bit integers, and hence do not
+# allow packing/unpacking quadwords with "Q". The chunk format uses 64-bit file
+# offsets to support files of any size, but in practice our test suite will
+# only use small files. So we can fake it by asking for two 32-bit values and
+# discarding the first (most significant) one, which is equivalent as long as
+# it's just zero.
+sub unpack_quad {
+	my $bytes = shift;
+	my ($n1, $n2) = unpack("NN", $bytes);
+	die "quad value exceeds 32 bits" if $n1;
+	return $n2;
+};
+sub pack_quad {
+	my $n = shift;
+	my $ret = pack("NN", 0, $n);
+	# double check that our original $n did not exceed the 32-bit limit.
+	# This is presumably impossible on a 32-bit system (which would have
+	# truncated much earlier), but would still alert us on a 64-bit build
+	# of a new test that would fail on a 32-bit build (though we'd
+	# presumably see the die() from unpack_quad() in such a case).
+	die "quad round-trip failed" if unpack_quad($ret) != $n;
+	return $ret;
+}
+
 # read until we find table-of-contents entry for chunk;
 # note that we cheat a bit by assuming 4-byte alignment and
 # that no ToC entry will accidentally look like a header.
 #
 # If we don't find the entry, copy() will hit EOF and exit
 # (which should cause the caller to fail the test).
 while (copy(4) ne $chunk) { }
-my $offset = unpack("Q>", copy(8));
+my $offset = unpack_quad(copy(8));
 
 # In clear mode, our length will change. So figure out
 # the length by comparing to the offset of the next chunk, and
@@ -38,11 +62,11 @@ sub copy {
 	my $id;
 	do {
 		$id = copy(4);
-		my $next = unpack("Q>", get(8));
+		my $next = unpack_quad(get(8));
 		if (!defined $len) {
 			$len = $next - $offset;
 		}
-		print pack("Q>", $next - $len + length($bytes));
+		print pack_quad($next - $len + length($bytes));
 	} while (unpack("N", $id));
 }
 
-- 
2.42.0.1037.g1c5f731f9d

Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[Jeff King]

On Fri, Nov 03, 2023 at 12:07:17PM -0400, rsbecker@nexbridge.com wrote:

> What I get from Perl is 
> $ perl -e '
> >     my $bytes = "\1\2\3\4\5\6\7\8";
> >     my $q = eval { unpack("Q>", $bytes) };
> >     print "Q = ", defined($q) ? $q : "($@)", "\n";
> >     my ($n1, $n2) = unpack("NN", $bytes);
> >     print "n1 = $n1\n";
> >     print "n2 = $n2\n";
> >     print "computed quad = ", ($n1 << 32) | $n2, "\n";  '
> Q = (Invalid type 'Q' in unpack at -e line 3.
> )
> n1 = 16909060
> n2 = 84281144
> computed quad = 84281144

OK, that matches what I expected. Hopefully the patch I just sent (our
mails just crossed) will fix it for you.

> Because perl itself is 32-bit, not 64-bit on this platform. So even
> moving git to 64-bit will not correct the issue.

Yep, exactly.

-Peff

RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type

[rsbecker]

On November 3, 2023 12:21 PM Jeff King wrote:
>On Fri, Nov 03, 2023 at 12:07:17PM -0400, rsbecker@nexbridge.com wrote:
>
>> What I get from Perl is
>> $ perl -e '
>> >     my $bytes = "\1\2\3\4\5\6\7\8";
>> >     my $q = eval { unpack("Q>", $bytes) };
>> >     print "Q = ", defined($q) ? $q : "($@)", "\n";
>> >     my ($n1, $n2) = unpack("NN", $bytes);
>> >     print "n1 = $n1\n";
>> >     print "n2 = $n2\n";
>> >     print "computed quad = ", ($n1 << 32) | $n2, "\n";  '
>> Q = (Invalid type 'Q' in unpack at -e line 3.
>> )
>> n1 = 16909060
>> n2 = 84281144
>> computed quad = 84281144
>
>OK, that matches what I expected. Hopefully the patch I just sent (our mails just
>crossed) will fix it for you.
>
>> Because perl itself is 32-bit, not 64-bit on this platform. So even
>> moving git to 64-bit will not correct the issue.
>
>Yep, exactly.

Your patch worked! Thanks.

Re: [PATCH] t: avoid perl's pack/unpack "Q" specifier

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> +# Some platforms' perl builds don't support 64-bit integers, and hence do not
> +# allow packing/unpacking quadwords with "Q". The chunk format uses 64-bit file
> +# offsets to support files of any size, but in practice our test suite will
> +# only use small files. So we can fake it by asking for two 32-bit values and
> +# discarding the first (most significant) one, which is equivalent as long as
> +# it's just zero.
> +sub unpack_quad {
> +	my $bytes = shift;
> +	my ($n1, $n2) = unpack("NN", $bytes);
> +	die "quad value exceeds 32 bits" if $n1;
> +	return $n2;
> +};

Is this an unnecessary ';' at the end?

> +sub pack_quad {
> +	my $n = shift;
> +	my $ret = pack("NN", 0, $n);
> +	# double check that our original $n did not exceed the 32-bit limit.
> +	# This is presumably impossible on a 32-bit system (which would have
> +	# truncated much earlier), but would still alert us on a 64-bit build
> +	# of a new test that would fail on a 32-bit build (though we'd
> +	# presumably see the die() from unpack_quad() in such a case).
> +	die "quad round-trip failed" if unpack_quad($ret) != $n;
> +	return $ret;
> +}

Nice.  Both sub are done carefully.

>  # read until we find table-of-contents entry for chunk;
>  # note that we cheat a bit by assuming 4-byte alignment and
>  # that no ToC entry will accidentally look like a header.
>  #
>  # If we don't find the entry, copy() will hit EOF and exit
>  # (which should cause the caller to fail the test).
>  while (copy(4) ne $chunk) { }
> -my $offset = unpack("Q>", copy(8));
> +my $offset = unpack_quad(copy(8));
>  
>  # In clear mode, our length will change. So figure out
>  # the length by comparing to the offset of the next chunk, and
> @@ -38,11 +62,11 @@ sub copy {
>  	my $id;
>  	do {
>  		$id = copy(4);
> -		my $next = unpack("Q>", get(8));
> +		my $next = unpack_quad(get(8));
>  		if (!defined $len) {
>  			$len = $next - $offset;
>  		}
> -		print pack("Q>", $next - $len + length($bytes));
> +		print pack_quad($next - $len + length($bytes));
>  	} while (unpack("N", $id));
>  }

Re: [PATCH] t: avoid perl's pack/unpack "Q" specifier

[Jeff King]

On Sat, Nov 04, 2023 at 10:47:30AM +0900, Junio C Hamano wrote:

> > +sub unpack_quad {
> > +	my $bytes = shift;
> > +	my ($n1, $n2) = unpack("NN", $bytes);
> > +	die "quad value exceeds 32 bits" if $n1;
> > +	return $n2;
> > +};
> 
> Is this an unnecessary ';' at the end?

Oops, yes. I'm not sure how that snuck in there. (It is not breaking
anything, but if you were to remove it while applying, I would be very
happy).

-Peff