From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id E428B1F560 for ; Sun, 1 Oct 2023 09:54:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1696154069; bh=b6u9DQ6eTLKZ5U0YTpyiYt3Jl62NWBVWUXPJ0ZpEpAg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=UyFJgftiLUGdpBpHHUVefmzxKZxnlC/w2J9+zvBAvyOsLrrHqkgZiSJa3b8/UIN5+ zFNlnHIfeB13GlDhxmXFDdwFJZiOR/ONotUUb4JdNreaEK8L6WcIMX5BDSJ9D1a4Xh 39XwWddrvi0cSUh8lBoueER1XUo3mawh2vKMgDgE= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 01/13] gcf2: take non-ref scalar request arg Date: Sun, 1 Oct 2023 09:54:17 +0000 Message-ID: <20231001095429.2092610-2-e@80x24.org> In-Reply-To: <20231001095429.2092610-1-e@80x24.org> References: <20231001095429.2092610-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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 +# Copyright (C) all contributors # License: AGPL-3.0+ -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');