unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
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	[thread overview]
Message-ID: <20231001095429.2092610-2-e@80x24.org> (raw)
In-Reply-To: <20231001095429.2092610-1-e@80x24.org>

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 <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');

  reply	other threads:[~2023-10-01  9:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-01  9:54 [PATCH 00/13] various warning/diagnostic fixes Eric Wong
2023-10-01  9:54 ` Eric Wong [this message]
2023-10-01  9:54 ` [PATCH 02/13] t/git: show git_version in diag output Eric Wong
2023-10-01  9:54 ` [PATCH 03/13] process_pipe: don't run `close' unless requested Eric Wong
2023-10-01  9:54 ` [PATCH 04/13] git: improve error reporting Eric Wong
2023-10-01  9:54 ` [PATCH 05/13] git: packed_bytes: deal with glob+stat TOCTTOU Eric Wong
2023-10-01  9:54 ` [PATCH 06/13] lei rediff: `git diff -O<order-file>' support Eric Wong
2023-10-01  9:54 ` [PATCH 07/13] lei: correct exit signal Eric Wong
2023-10-01  9:54 ` [PATCH 08/13] lei mail-diff: don't remove temporary subdirectory Eric Wong
2023-10-01  9:54 ` [PATCH 09/13] lei_store: unlink stderr buffer early Eric Wong
2023-10-01  9:54 ` [PATCH 10/13] overidx: fix version comparison Eric Wong
2023-10-01  9:54 ` [PATCH 11/13] treewide: enable warnings in all exec-ed processes Eric Wong
2023-10-01  9:54 ` [PATCH 12/13] lei: ->fail only allows integer exit codes Eric Wong
2023-10-01  9:54 ` [PATCH 13/13] lei: deal with clients with blocked stderr Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231001095429.2092610-2-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).