gcf2: take non-ref scalar request arg

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.

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 8ac44a5e5aa8bc1c3581907111efadd446c02b40..8e313c84845f06942282e488b8decec4c59727d5 100644 (file)
--- 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 71ee1147a7904820cffb37f3d0ed7509fd5ebed4..f8b2a9fc688c880be970a16dd7e721203c163e59 100644 (file)
--- 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 6d059cada4d84f9d3da5c2cc3421e009d4d1c33b..33ee2c9124489f9f5cfe33c39030dc5fec319b3e 100644 (file)
--- a/t/gcf2_client.t
+++ b/t/gcf2_client.t
@@ -1,10 +1,10 @@
 #!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # 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');