Asking callers to pass a scalar reference is awkward and
doesn't benefit modern Perl with CoW support. Unlike
some constant error messages, it can't save any allocations
at all since there's no constant strings being passed to
libgit2.
---
lib/PublicInbox/Gcf2Client.pm | 8 ++++----
lib/PublicInbox/GitAsyncCat.pm | 4 ++--
t/gcf2_client.t | 32 ++++++++++++++++----------------
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 8ac44a5e..8e313c84 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -11,6 +11,7 @@ use PublicInbox::Spawn qw(spawn);
use Socket qw(AF_UNIX SOCK_STREAM);
use PublicInbox::Syscall qw(EPOLLIN);
use PublicInbox::ProcessPipe;
+use autodie qw(socketpair);
# fields:
# sock => socket to Gcf2::loop
@@ -26,8 +27,7 @@ sub new {
my $self = bless {}, __PACKAGE__;
# ensure the child process has the same @INC we do:
my $env = { PERL5LIB => join(':', @INC) };
- my ($s1, $s2);
- socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
+ socketpair(my $s1, my $s2, AF_UNIX, SOCK_STREAM, 0);
$s1->blocking(0);
$opt->{0} = $opt->{1} = $s2;
my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
@@ -41,8 +41,8 @@ sub new {
sub gcf2_async ($$$;$) {
my ($self, $req, $cb, $arg) = @_;
my $inflight = $self->{inflight} or return $self->close;
- PublicInbox::Git::write_all($self, $$req, \&cat_async_step, $inflight);
- push @$inflight, $req, $cb, $arg;
+ PublicInbox::Git::write_all($self, $req, \&cat_async_step, $inflight);
+ push @$inflight, \$req, $cb, $arg; # ref prevents Git.pm retries
}
# ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm
index 71ee1147..f8b2a9fc 100644
--- a/lib/PublicInbox/GitAsyncCat.pm
+++ b/lib/PublicInbox/GitAsyncCat.pm
@@ -18,7 +18,7 @@ sub ibx_async_cat ($$$$) {
require PublicInbox::Gcf2Client;
PublicInbox::Gcf2Client::new();
} // 0)) { # 0: do not retry if libgit2 or Inline::C are missing
- $GCF2C->gcf2_async(\"$oid $git->{git_dir}\n", $cb, $arg);
+ $GCF2C->gcf2_async("$oid $git->{git_dir}\n", $cb, $arg);
\undef;
} else { # read-only end of git-cat-file pipe
$git->cat_async($oid, $cb, $arg);
@@ -42,7 +42,7 @@ sub ibx_async_prefetch {
if (!defined($ibx->{topdir}) && $GCF2C) {
if (!@{$GCF2C->{inflight} // []}) {
$oid .= " $git->{git_dir}\n";
- return $GCF2C->gcf2_async(\$oid, $cb, $arg); # true
+ return $GCF2C->gcf2_async($oid, $cb, $arg); # true
}
} elsif ($git->{epwatch}) {
return $git->async_prefetch($oid, $cb, $arg);
diff --git a/t/gcf2_client.t b/t/gcf2_client.t
index 6d059cad..33ee2c91 100644
--- a/t/gcf2_client.t
+++ b/t/gcf2_client.t
@@ -1,10 +1,10 @@
#!perl -w
-# Copyright (C) 2020-2021 all contributors <[email protected]>
+# Copyright (C) all contributors <[email protected]>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
+use v5.12;
use PublicInbox::TestCommon;
-use Test::More;
use Cwd qw(getcwd);
+use autodie qw(open close);
use PublicInbox::Import;
use PublicInbox::DS;
@@ -17,7 +17,7 @@ PublicInbox::Import::init_bare($git_a);
PublicInbox::Import::init_bare($git_b);
my $fi_data = './t/git.fast-import-data';
my $rdr = {};
-open $rdr->{0}, '<', $fi_data or BAIL_OUT $!;
+open $rdr->{0}, '<', $fi_data;
xsys([qw(git fast-import --quiet)], { GIT_DIR => $git_a }, $rdr);
is($?, 0, 'fast-import succeeded');
@@ -26,9 +26,9 @@ my $called = 0;
my $err_f = "$tmpdir/err";
{
PublicInbox::DS->Reset;
- open my $err, '>>', $err_f or BAIL_OUT $!;
+ open my $err, '>>', $err_f;
my $gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
- $gcf2c->gcf2_async(\"$tree $git_a\n", sub {
+ $gcf2c->gcf2_async("$tree $git_a\n", sub {
my ($bref, $oid, $type, $size, $arg) = @_;
is($oid, $tree, 'got expected OID');
is($size, 30, 'got expected length');
@@ -39,12 +39,12 @@ my $err_f = "$tmpdir/err";
}, 'hi');
$gcf2c->cat_async_step($gcf2c->{inflight});
- open $err, '<', $err_f or BAIL_OUT $!;
+ open $err, '<', $err_f;
my $estr = do { local $/; <$err> };
is($estr, '', 'nothing in stderr');
my $trunc = substr($tree, 0, 39);
- $gcf2c->gcf2_async(\"$trunc $git_a\n", sub {
+ $gcf2c->gcf2_async("$trunc $git_a\n", sub {
my ($bref, $oid, $type, $size, $arg) = @_;
is(undef, $bref, 'missing bref is undef');
is($oid, $trunc, 'truncated OID printed');
@@ -55,30 +55,30 @@ my $err_f = "$tmpdir/err";
}, 'bye');
$gcf2c->cat_async_step($gcf2c->{inflight});
- open $err, '<', $err_f or BAIL_OUT $!;
+ open $err, '<', $err_f;
$estr = do { local $/; <$err> };
like($estr, qr/retrying/, 'warned about retry');
# try failed alternates lookup
PublicInbox::DS->Reset;
- open $err, '>', $err_f or BAIL_OUT $!;
+ open $err, '>', $err_f;
$gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
- $gcf2c->gcf2_async(\"$tree $git_b\n", sub {
+ $gcf2c->gcf2_async("$tree $git_b\n", sub {
my ($bref, $oid, $type, $size, $arg) = @_;
is(undef, $bref, 'missing bref from alt is undef');
$called++;
});
$gcf2c->cat_async_step($gcf2c->{inflight});
- open $err, '<', $err_f or BAIL_OUT $!;
+ open $err, '<', $err_f;
$estr = do { local $/; <$err> };
like($estr, qr/retrying/, 'warned about retry before alt update');
# now try successful alternates lookup
- open my $alt, '>>', "$git_b/objects/info/alternates" or BAIL_OUT $!;
- print $alt "$git_a/objects\n" or BAIL_OUT $!;
- close $alt or BAIL_OUT;
+ open my $alt, '>>', "$git_b/objects/info/alternates";
+ print $alt "$git_a/objects\n";
+ close $alt;
my $expect = xqx(['git', "--git-dir=$git_a", qw(cat-file tree), $tree]);
- $gcf2c->gcf2_async(\"$tree $git_a\n", sub {
+ $gcf2c->gcf2_async("$tree $git_a\n", sub {
my ($bref, $oid, $type, $size, $arg) = @_;
is($oid, $tree, 'oid match on alternates retry');
is($$bref, $expect, 'tree content matched');
