unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 00/30] autodie-ification and code simplifications
@ 2023-10-17 23:37 Eric Wong
  2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
                   ` (30 more replies)
  0 siblings, 31 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

Noisy code is less pleasant to work on, so use autodie more and
a few more simplifications.  There's a couple of small bugfixes
discovered along the way, too.

Eric Wong (30):
  lei_mirror: start converting to autodie
  lei_mirror: autodie most `close' calls
  lei_mirror: use autodie for most `open' calls
  git: introduce read_all function
  import: use read_all to detect short reads
  lei_mirror: use read_all
  use read_all in more places to improve safety
  xap_helper*: use autodie in more places
  xap_helper: die more easily in both implementations
  xap_helper: simplify SIGTERM exit checks
  xap_helper: autodie for getsockopt
  xap_client: autodie for pipe and socketpair
  xt/git-http-backend: remove Net::HTTP usage
  ds: introduce and use do_fork helper
  ds: get rid of SetLoopTimeout
  cindex: drop some unused functions
  syscall: common $F_SETPIPE_SZ definition
  t/lei-up: additional diagnostics for match failures
  test_common: use autodie and read_all where possible
  test_common: only hide TCP port in messages
  test_common: use $cwdfh for every run_script command
  init: drop extraneous `+'
  init: use autodie to reduce distractions
  xt/mem-imapd-tls: remove unused/broken epoll imports
  xt/mem-imapd-tls: reduce FDs for lsof use
  lei: use autodie where appropriate
  lei_auth: update comments and use v5.12
  lei_config: drop redundant open check
  convert: use read_all to simplify error checks
  idx_stack: use autodie + read_all

 lib/PublicInbox/CidxLogP.pm       |   4 +-
 lib/PublicInbox/CodeSearchIdx.pm  |   5 --
 lib/PublicInbox/DS.pm             |  36 ++++----
 lib/PublicInbox/Daemon.pm         |  16 ++--
 lib/PublicInbox/EOFpipe.pm        |   6 +-
 lib/PublicInbox/Gcf2.pm           |   7 +-
 lib/PublicInbox/Git.pm            |  19 +++--
 lib/PublicInbox/IPC.pm            |  12 +--
 lib/PublicInbox/IdxStack.pm       |  20 ++---
 lib/PublicInbox/Import.pm         |   8 +-
 lib/PublicInbox/InboxWritable.pm  |   6 +-
 lib/PublicInbox/LEI.pm            |  48 +++++------
 lib/PublicInbox/LeiALE.pm         |  11 +--
 lib/PublicInbox/LeiAuth.pm        |   7 +-
 lib/PublicInbox/LeiBlob.pm        |   6 +-
 lib/PublicInbox/LeiConfig.pm      |   4 +-
 lib/PublicInbox/LeiMailSync.pm    |   5 +-
 lib/PublicInbox/LeiMirror.pm      | 131 ++++++++++++++----------------
 lib/PublicInbox/LeiSucks.pm       |   5 +-
 lib/PublicInbox/LeiXSearch.pm     |   2 +-
 lib/PublicInbox/MultiGit.pm       |   3 +-
 lib/PublicInbox/SearchIdxShard.pm |  14 ++--
 lib/PublicInbox/Syscall.pm        |  16 ++--
 lib/PublicInbox/TestCommon.pm     |  85 +++++++++----------
 lib/PublicInbox/ViewVCS.pm        |  12 ++-
 lib/PublicInbox/WWW.pm            |   4 +-
 lib/PublicInbox/Watch.pm          |  11 +--
 lib/PublicInbox/XapClient.pm      |  11 +--
 lib/PublicInbox/XapHelper.pm      |  24 ++----
 lib/PublicInbox/XapHelperCxx.pm   |  11 +--
 lib/PublicInbox/Xapcmd.pm         |   5 +-
 lib/PublicInbox/xap_helper.h      |  60 ++++++--------
 script/public-inbox-convert       |   8 +-
 script/public-inbox-edit          |   4 +-
 script/public-inbox-init          |  30 +++----
 t/dir_idle.t                      |   2 +-
 t/ds-leak.t                       |   4 +-
 t/gcf2.t                          |   5 +-
 t/init.t                          |   7 ++
 t/lei-sigpipe.t                   |   7 +-
 t/lei-up.t                        |   4 +-
 xt/git-http-backend.t             |  30 +++----
 xt/mem-imapd-tls.t                |  21 ++---
 xt/mem-nntpd-tls.t                |   8 +-
 44 files changed, 335 insertions(+), 409 deletions(-)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 01/30] lei_mirror: start converting to autodie
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This code is too noisy and not critical for startup performance;
so autodie provides a nice noise reduction while improving error
reporting in most cases.

For places where failures are expected, the `CORE::' prefix
gives us an easy escape hatch to fall back to normal error
checking.
---
 lib/PublicInbox/LeiMirror.pm | 38 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 9d8a8963..f4c16a9d 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,6 +21,7 @@ use PublicInbox::LeiCurl;
 use PublicInbox::OnDestroy;
 use PublicInbox::SHA qw(sha256_hex sha1_hex);
 use POSIX qw(strftime);
+use autodie qw(chmod pipe readlink seek sysopen truncate unlink);
 
 our $LIVE; # pid => callback
 our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -122,7 +123,7 @@ sub ft_rename ($$$;$) {
 	my ($ft, $dst, $open_mode, $fh) = @_;
 	my @st = stat($fh // $dst);
 	my $mode = @st ? ($st[2] & 07777) : ($open_mode & ~umask);
-	chmod($mode, $ft) or croak "E: chmod($ft): $!";
+	chmod($mode, $ft);
 	require File::Copy;
 	File::Copy::mv($ft->filename, $dst) or croak "E: mv($ft => $dst): $!";
 	$ft->unlink_on_destroy(0);
@@ -170,7 +171,7 @@ sub _get_txt_done { # returns true on error (non-fatal), undef on success
 	$? = 0; # don't influence normal lei exit
 	return warn("$uri missing\n") if ($cerr >> 8) == 22;
 	return warn("# @$cmd failed (non-fatal)\n") if $cerr;
-	seek($fh, SEEK_SET, 0) or die "seek: $!";
+	seek($fh, SEEK_SET, 0);
 	$self->{"mtime.$endpoint"} = (stat($fh))[9];
 	local $/;
 	$self->{"txt.$endpoint"} = <$fh>;
@@ -183,9 +184,9 @@ sub _write_inbox_config {
 	my $dst = $self->{cur_dst} // $self->{dst};
 	my $f = "$dst/inbox.config.example";
 	my $mtime = delete $self->{'mtime._/text/config/raw'};
-	if (sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
+	if (CORE::sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
 		print $fh $buf or die "print: $!";
-		chmod(0444 & ~umask, $fh) or die "chmod($f): $!";
+		chmod(0444 & ~umask, $fh);
 		$fh->flush or die "flush($f): $!";
 		if (defined $mtime) {
 			utime($mtime, $mtime, $fh) or die "utime($f): $!";
@@ -289,7 +290,7 @@ sub upr { # feed `git update-ref --stdin -z' verbosely
 
 sub start_update_ref {
 	my ($fgrp) = @_;
-	pipe(my ($r, $w)) or die "pipe: $!";
+	pipe(my $r, my $w);
 	my $cmd = [ 'git', "--git-dir=$fgrp->{cur_dst}",
 		qw(update-ref --stdin -z) ];
 	my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp);
@@ -305,8 +306,8 @@ sub fgrp_update {
 	return if !keep_going($fgrp);
 	my $srcfh = delete $fgrp->{srcfh} or return;
 	my $dstfh = delete $fgrp->{dstfh} or return;
-	seek($srcfh, SEEK_SET, 0) or die "seek(src): $!";
-	seek($dstfh, SEEK_SET, 0) or die "seek(dst): $!";
+	seek($srcfh, SEEK_SET, 0);
+	seek($dstfh, SEEK_SET, 0);
 	my %src = map { chomp; split(/\0/) } (<$srcfh>);
 	close $srcfh;
 	my %dst = map { chomp; split(/\0/) } (<$dstfh>);
@@ -359,7 +360,7 @@ sub pack_refs {
 
 sub unlink_fetch_head ($) {
 	my ($git_dir) = @_;
-	return if unlink("$git_dir/FETCH_HEAD") || $!{ENOENT};
+	return if CORE::unlink("$git_dir/FETCH_HEAD") || $!{ENOENT};
 	warn "W: unlink($git_dir/FETCH_HEAD): $!";
 }
 
@@ -447,8 +448,7 @@ sub fgrp_fetch_all {
 		if (!$self->{dry_run}) {
 			# update the config atomically via O_APPEND while
 			# respecting git-config locking
-			sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY)
-				or die "open($f.lock): $!";
+			sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY);
 			open my $fh, '>>', $f or die "open(>>$f): $!";
 			$fh->autoflush(1);
 			my $buf = '';
@@ -464,7 +464,7 @@ sub fgrp_fetch_all {
 				(map { "\t$grp = $_->{-remote}\n" } @$new));
 			print $fh $buf or die "print($f): $!";
 			close $fh or die "close($f): $!";
-			unlink("$f.lock") or die "unlink($f.lock): $!";
+			unlink("$f.lock");
 		}
 		$cmd  = [ @git, "--git-dir=$osdir", @fetch, $grp ];
 		push @$old, @$new;
@@ -515,7 +515,7 @@ EOM
 		my $f = "$o/info/alternates";
 		my $l = File::Spec->abs2rel($alt, File::Spec->rel2abs($o));
 		open my $fh, '+>>', $f or die "open($f): $!";
-		seek($fh, SEEK_SET, 0) or die "seek($f): $!";
+		seek($fh, SEEK_SET, 0);
 		chomp(my @cur = <$fh>);
 		if (!grep(/\A\Q$l\E\z/, @cur)) {
 			say $fh $l or die "say($f): $!";
@@ -535,7 +535,7 @@ sub fp_done {
 	}
 	return if !keep_going($self);
 	my $fh = delete $self->{-show_ref} // die 'BUG: no show-ref output';
-	seek($fh, SEEK_SET, 0) or die "seek(show_ref): $!";
+	seek($fh, SEEK_SET, 0);
 	$self->{-ent} // die 'BUG: no -ent';
 	my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
 	my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
@@ -735,7 +735,7 @@ sub up_fp_done {
 	my ($self) = @_;
 	return if !keep_going($self);
 	my $fh = delete $self->{-show_ref_up} // die 'BUG: no show-ref output';
-	seek($fh, SEEK_SET, 0) or die "seek(show_ref): $!";
+	seek($fh, SEEK_SET, 0);
 	$self->{-ent} // die 'BUG: no -ent';
 	my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
 	my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
@@ -811,7 +811,7 @@ sub update_ent {
 			if (lstat($ln)) {
 				if (-l _) {
 					next if readlink($ln) eq $tgt;
-					unlink($ln) or die "unlink($ln): $!";
+					unlink($ln);
 				} else {
 					push @{$self->{chg}->{badlink}}, $p;
 				}
@@ -882,7 +882,7 @@ sub v2_done { # called via OnDestroy
 	for my $i ($mg->git_epochs) { $mg->epoch_cfg_set($i) }
 	for my $edst (@{delete($self->{-read_only}) // []}) {
 		my @st = stat($edst) or die "stat($edst): $!";
-		chmod($st[2] & 0555, $edst) or die "chmod(a-w, $edst): $!";
+		chmod($st[2] & 0555, $edst);
 	}
 	write_makefile($dst, 2);
 	undef $lck; # unlock
@@ -1089,8 +1089,8 @@ sub dump_manifest ($$) {
 	# epoch they no longer want to skip
 	my $json = PublicInbox::Config->json->encode($m);
 	my $mtime = (stat($ft))[9];
-	seek($ft, SEEK_SET, 0) or die "seek($ft): $!";
-	truncate($ft, 0) or die "truncate($ft): $!";
+	seek($ft, SEEK_SET, 0);
+	truncate($ft, 0);
 	gzip(\$json => $ft) or die "gzip($ft): $GzipError";
 	$ft->flush or die "flush($ft): $!";
 	utime($mtime, $mtime, "$ft") or die "utime(..., $ft): $!";
@@ -1344,7 +1344,7 @@ sub ipc_atfork_child {
 sub write_makefile {
 	my ($dir, $ibx_ver) = @_;
 	my $f = "$dir/Makefile";
-	if (sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
+	if (CORE::sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
 		print $fh <<EOM or die "print($f) $!";
 # This is a v$ibx_ver public-inbox, see the public-inbox-v$ibx_ver-format(5)
 # manpage for more information on the format.  This Makefile is

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 02/30] lei_mirror: autodie most `close' calls
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
  2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

Another step towards reducing our code burden and having
more consistent error messages.
---
 lib/PublicInbox/LeiMirror.pm | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index f4c16a9d..84344f37 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,7 +21,7 @@ use PublicInbox::LeiCurl;
 use PublicInbox::OnDestroy;
 use PublicInbox::SHA qw(sha256_hex sha1_hex);
 use POSIX qw(strftime);
-use autodie qw(chmod pipe readlink seek sysopen truncate unlink);
+use autodie qw(chmod close pipe readlink seek sysopen truncate unlink);
 
 our $LIVE; # pid => callback
 our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -58,7 +58,7 @@ sub try_scrape {
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
 	my $fh = popen_rd($cmd, undef, $opt);
 	my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
-	close($fh) or return $lei->child_error($?, "@$cmd failed");
+	CORE::close($fh) or return $lei->child_error($?, "@$cmd failed");
 
 	# we grep with URL below, we don't want Subject/From headers
 	# making us clone random URLs.  This assumes remote instances
@@ -295,7 +295,7 @@ sub start_update_ref {
 		qw(update-ref --stdin -z) ];
 	my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp);
 	start_cmd($fgrp, $cmd, { 0 => $r, 2 => $fgrp->{lei}->{2} }, $pack);
-	close $r or die "close(r): $!";
+	close $r;
 	$fgrp->{dry_run} ? undef : $w;
 }
 
@@ -335,7 +335,7 @@ sub fgrp_update {
 			upr($lei, $w, 'create', $ref, $oid);
 		}
 	}
-	close($w) or upref_warn();
+	CORE::close($w) or upref_warn();
 }
 
 sub satellite_done {
@@ -345,7 +345,7 @@ sub satellite_done {
 		while (my ($ref, $oid) = each %$create) {
 			upr($fgrp->{lei}, $w, 'create', $ref, $oid);
 		}
-		close($w) or upref_warn();
+		CORE::close($w) or upref_warn();
 	} else {
 		pack_refs($fgrp, $fgrp->{cur_dst});
 		run_puh($fgrp);
@@ -463,7 +463,7 @@ sub fgrp_fetch_all {
 				(map { "\t$grp = $_->{-remote}\n" } @$old),
 				(map { "\t$grp = $_->{-remote}\n" } @$new));
 			print $fh $buf or die "print($f): $!";
-			close $fh or die "close($f): $!";
+			close $fh;
 			unlink("$f.lock");
 		}
 		$cmd  = [ @git, "--git-dir=$osdir", @fetch, $grp ];
@@ -490,7 +490,7 @@ sub forkgroup_prep {
 [pack]
 	island = refs/remotes/([^/]+)/
 EOM
-		close $fh or die "close($f): $!";
+		close $fh;
 	}
 	my $key = $self->{-key} // die 'BUG: no -key';
 	my $rn = substr(sha256_hex($key), 0, 16);
@@ -507,7 +507,7 @@ EOM
 ;	fetch = +refs/*:refs/*
 ;	mirror = true
 EOM
-		close $fh or die "close($f): $!";
+		close $fh;
 	}
 	if (!$self->{dry_run}) {
 		my $alt = File::Spec->rel2abs("$dir/objects");
@@ -520,7 +520,7 @@ EOM
 		if (!grep(/\A\Q$l\E\z/, @cur)) {
 			say $fh $l or die "say($f): $!";
 		}
-		close $fh or die "close($f): $!";
+		close $fh;
 	}
 	bless {
 		%$self, -osdir => $dir, -remote => $rn, -uri => $uri
@@ -712,7 +712,7 @@ EOM
 	owner = $ent->{owner}
 EOM
 	}
-	close $fh or die "close($f): $!";
+	close $fh;
 	my %map = (head => 'HEAD', description => undef);
 	while (my ($key, $fn) = each %map) {
 		my $val = $ent->{$key} // next;
@@ -720,7 +720,7 @@ EOM
 		$fn = "$edst/$fn";
 		open $fh, '>', $fn or die "open($fn): $!";
 		print $fh $val, "\n" or die "print($fn): $!";
-		close $fh or die "close($fn): $!";
+		close $fh;
 	}
 }
 
@@ -1395,7 +1395,7 @@ compact :
 
 .PHONY : help fetch update index reindex compact
 EOM
-		close $fh or die "close($f): $!";
+		close $fh;
 	} else {
 		die "open($f): $!" unless $!{EEXIST};
 	}

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 03/30] lei_mirror: use autodie for most `open' calls
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
  2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
  2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

We'll also drop most `print' checks since we need to rely on
`close' or IO::Handle->flush for error checking, anyways.
---
 lib/PublicInbox/LeiMirror.pm | 62 +++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 84344f37..b402eb5f 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,7 +21,8 @@ use PublicInbox::LeiCurl;
 use PublicInbox::OnDestroy;
 use PublicInbox::SHA qw(sha256_hex sha1_hex);
 use POSIX qw(strftime);
-use autodie qw(chmod close pipe readlink seek sysopen truncate unlink);
+use autodie qw(chdir chmod close open pipe readlink seek symlink sysopen
+		truncate unlink);
 
 our $LIVE; # pid => callback
 our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -185,7 +186,7 @@ sub _write_inbox_config {
 	my $f = "$dst/inbox.config.example";
 	my $mtime = delete $self->{'mtime._/text/config/raw'};
 	if (CORE::sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
-		print $fh $buf or die "print: $!";
+		print $fh $buf;
 		chmod(0444 & ~umask, $fh);
 		$fh->flush or die "flush($f): $!";
 		if (defined $mtime) {
@@ -381,12 +382,12 @@ sub fgrpv_done {
 		my $src = [ 'git', "--git-dir=$fgrp->{-osdir}", 'for-each-ref',
 			"--format=refs/%(refname:lstrip=3)%00%(objectname)",
 			"refs/remotes/$rn/" ];
-		open(my $sfh, '+>', undef) or die "open(src): $!";
+		open(my $sfh, '+>', undef);
 		$fgrp->{srcfh} = $sfh;
 		start_cmd($fgrp, $src, { %opt, 1 => $sfh }, $update_ref);
 		my $dst = [ 'git', "--git-dir=$fgrp->{cur_dst}", 'for-each-ref',
 			'--format=%(refname)%00%(objectname)' ];
-		open(my $dfh, '+>', undef) or die "open(dst): $!";
+		open(my $dfh, '+>', undef);
 		$fgrp->{dstfh} = $dfh;
 		start_cmd($fgrp, $dst, { %opt, 1 => $dfh }, $update_ref);
 	}
@@ -449,7 +450,7 @@ sub fgrp_fetch_all {
 			# update the config atomically via O_APPEND while
 			# respecting git-config locking
 			sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY);
-			open my $fh, '>>', $f or die "open(>>$f): $!";
+			open my $fh, '>>', $f;
 			$fh->autoflush(1);
 			my $buf = '';
 			if (@$old) {
@@ -482,9 +483,8 @@ sub forkgroup_prep {
 	my $dir = "$os/$fg.git";
 	if (!-d $dir && !$self->{dry_run}) {
 		PublicInbox::Import::init_bare($dir);
-		my $f = "$dir/config";
-		open my $fh, '+>>', $f or die "open:($f): $!";
-		print $fh <<EOM or die "print($f): $!";
+		open my $fh, '+>>', "$dir/config";
+		print $fh <<EOM;
 [repack]
 	useDeltaIslands = true
 [pack]
@@ -496,9 +496,8 @@ EOM
 	my $rn = substr(sha256_hex($key), 0, 16);
 	if (!-d $self->{cur_dst} && !$self->{dry_run}) {
 		PublicInbox::Import::init_bare($self->{cur_dst});
-		my $f = "$self->{cur_dst}/config";
-		open my $fh, '+>>', $f or die "open:($f): $!";
-		print $fh <<EOM or die "print($f): $!";
+		open my $fh, '+>>', "$self->{cur_dst}/config";
+		print $fh <<EOM;
 ; rely on the "$rn" remote in the
 ; $fg fork group for fetches
 ; only uncomment the following iff you detach from fork groups
@@ -514,11 +513,11 @@ EOM
 		my $o = "$self->{cur_dst}/objects";
 		my $f = "$o/info/alternates";
 		my $l = File::Spec->abs2rel($alt, File::Spec->rel2abs($o));
-		open my $fh, '+>>', $f or die "open($f): $!";
+		open my $fh, '+>>', $f;
 		seek($fh, SEEK_SET, 0);
 		chomp(my @cur = <$fh>);
 		if (!grep(/\A\Q$l\E\z/, @cur)) {
-			say $fh $l or die "say($f): $!";
+			say $fh $l;
 		}
 		close $fh;
 	}
@@ -556,7 +555,7 @@ sub cmp_fp_do {
 	my $dst = $self->{cur_dst} // $self->{dst};
 	my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
 	my $opt = { 2 => $self->{lei}->{2} };
-	open($opt->{1}, '+>', undef) or die "open(tmp): $!";
+	open($opt->{1}, '+>', undef);
 	$self->{-show_ref} = $opt->{1};
 	do_reap($self);
 	$self->{lei}->qerr("# @$cmd");
@@ -695,8 +694,8 @@ sub init_placeholder ($$$) {
 	my ($src, $edst, $ent) = @_;
 	PublicInbox::Import::init_bare($edst);
 	my $f = "$edst/config";
-	open my $fh, '>>', $f or die "open($f): $!";
-	print $fh <<EOM or die "print($f): $!";
+	open my $fh, '>>', $f;
+	print $fh <<EOM;
 [remote "origin"]
 	url = $src
 	fetch = +refs/*:refs/*
@@ -706,20 +705,17 @@ sub init_placeholder ($$$) {
 ; will not fetch updates for it unless write permission is added.
 ; Hint: chmod +w $edst
 EOM
-	if (defined($ent->{owner})) {
-		print $fh <<EOM or die "print($f): $!";
+	print $fh <<EOM if defined($ent->{owner});
 [gitweb]
 	owner = $ent->{owner}
 EOM
-	}
 	close $fh;
 	my %map = (head => 'HEAD', description => undef);
 	while (my ($key, $fn) = each %map) {
 		my $val = $ent->{$key} // next;
 		$fn //= $key;
-		$fn = "$edst/$fn";
-		open $fh, '>', $fn or die "open($fn): $!";
-		print $fh $val, "\n" or die "print($fn): $!";
+		open $fh, '>', "$edst/$fn";
+		say $fh $val;
 		close $fh;
 	}
 }
@@ -747,7 +743,7 @@ sub up_fp_done {
 sub atomic_write ($$$) {
 	my ($dn, $bn, $raw) = @_;
 	my $ft = File::Temp->new(DIR => $dn, TEMPLATE => "$bn-XXXX");
-	print $ft $raw or die "print($ft): $!";
+	print $ft $raw;
 	$ft->flush or die "flush($ft): $!";
 	ft_rename($ft, "$dn/$bn", 0666);
 }
@@ -778,7 +774,7 @@ sub update_ent {
 	if (defined($new) && $new ne $cur) {
 		my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
 		my $opt = { 2 => $self->{lei}->{2} };
-		open($opt->{1}, '+>', undef) or die "open(tmp): $!";
+		open($opt->{1}, '+>', undef);
 		$self->{-show_ref_up} = $opt->{1};
 		my $done = PublicInbox::OnDestroy->new($$, \&up_fp_done, $self);
 		start_cmd($self, $cmd, $opt, $done);
@@ -816,7 +812,7 @@ sub update_ent {
 					push @{$self->{chg}->{badlink}}, $p;
 				}
 			}
-			symlink($tgt, $ln) or die "symlink($tgt, $ln): $!";
+			symlink($tgt, $ln);
 			++$self->{chg}->{nr_chg};
 		}
 	}
@@ -844,7 +840,7 @@ sub v1_done { # called via OnDestroy
 	unlink_fetch_head($dst);
 	update_ent($self) if $self->{-ent};
 	my $o = "$dst/objects";
-	if (open(my $fh, '<', my $fn = "$o/info/alternates")) {;
+	if (CORE::open(my $fh, '<', my $fn = "$o/info/alternates")) {;
 		my $base = File::Spec->rel2abs($o);
 		my @l = <$fh>;
 		my $ft;
@@ -855,7 +851,7 @@ sub v1_done { # called via OnDestroy
 						DIR => "$o/info");
 		}
 		if ($ft) {
-			print $ft @l or die "print($ft): $!";
+			print $ft @l;
 			$ft->flush or die "flush($ft): $!";
 			ft_rename($ft, $fn, 0666, $fh);
 		}
@@ -964,7 +960,7 @@ sub decode_manifest ($$$) {
 sub load_current_manifest ($) {
 	my ($self) = @_;
 	my $fn = $self->{-manifest} // return;
-	if (open(my $fh, '<', $fn)) {
+	if (CORE::open(my $fh, '<', $fn)) {
 		decode_manifest($fh, $fn, $fn);
 	} elsif ($!{ENOENT}) { # non-fatal, we can just do it slowly
 		warn "open($fn): $!\n" if !$self->{-initial_clone};
@@ -1102,12 +1098,12 @@ sub dump_project_list ($$) {
 	my $old = defined($f) ? PublicInbox::Git::try_cat($f) : '';
 	my %new;
 
-	open my $dh, '<', '.' or die "open(.): $!";
+	open my $dh, '<', '.';
 	if (!$self->{dry_run} || -d $self->{dst}) {
-		chdir($self->{dst}) or die "chdir($self->{dst}): $!";
+		chdir($self->{dst});
 	}
 	my @local = grep { -e $_ ? ($new{$_} = undef) : 1 } split(/\n/s, $old);
-	chdir($dh) or die "chdir(restore): $!";
+	chdir($dh);
 
 	$new{substr($_, 1)} = 1 for keys %$m; # drop leading '/'
 	my @list = sort keys %new;
@@ -1345,7 +1341,7 @@ sub write_makefile {
 	my ($dir, $ibx_ver) = @_;
 	my $f = "$dir/Makefile";
 	if (CORE::sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
-		print $fh <<EOM or die "print($f) $!";
+		print $fh <<EOM;
 # This is a v$ibx_ver public-inbox, see the public-inbox-v$ibx_ver-format(5)
 # manpage for more information on the format.  This Makefile is
 # intended as a familiar wrapper for users unfamiliar with
@@ -1359,7 +1355,7 @@ sub write_makefile {
 # so you may edit it freely with your own convenience targets
 # and notes.  public-inbox-fetch will recreate it if removed.
 EOM
-		print $fh <<'EOM' or die "print($f): $!";
+		print $fh <<'EOM';
 # the default target:
 help :
 	@echo Common targets:

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 04/30] git: introduce read_all function
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (2 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This makes it easier to improve error checking, since the
`do { local $/; readline(FH) }' construct does not detect
errors (autodie does not cover `readline' or `<FH>').

I'm not sure exactly where this should be, but PublicInbox::Git
is used nearly everywhere in our code base and it's probably
not worth creating a new package for it.
---
 lib/PublicInbox/Git.pm | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 448cfaf7..35bd10ef 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -10,7 +10,7 @@ package PublicInbox::Git;
 use strict;
 use v5.10.1;
 use parent qw(Exporter PublicInbox::DS);
-use autodie qw(socketpair);
+use autodie qw(socketpair read);
 use POSIX ();
 use Socket qw(AF_UNIX SOCK_STREAM);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
@@ -26,7 +26,7 @@ use Carp qw(croak carp);
 use PublicInbox::SHA ();
 our %HEXLEN2SHA = (40 => 1, 64 => 256);
 our %OFMT2HEXLEN = (sha1 => 40, sha256 => 64);
-our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN);
+our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN read_all);
 our $in_cleanup;
 our $RDTIMEO = 60_000; # milliseconds
 our $async_warn; # true in read-only daemons
@@ -548,11 +548,19 @@ sub modified ($;$) {
 	(split(/ /, <$fh> // time))[0] + 0; # integerize for JSON
 }
 
+# read_all/try_cat can probably be moved somewhere else...
+
+sub read_all ($;$) {
+	my ($fh, $len) = @_;
+	my $r = read($fh, my $buf, $len //= -s $fh);
+	croak("$fh read ($r != $len)") if $len != $r;
+	$buf;
+}
+
 sub try_cat {
 	my ($path) = @_;
 	open(my $fh, '<', $path) or return '';
-	local $/;
-	<$fh> // '';
+	read_all($fh);
 }
 
 sub cat_desc ($) {
@@ -606,7 +614,7 @@ sub manifest_entry {
 		}
 	}
 	my $dig = PublicInbox::SHA->new(1);
-	while (read($sr, $buf, 65536)) { $dig->add($buf) }
+	while (CORE::read($sr, $buf, 65536)) { $dig->add($buf) }
 	CORE::close $sr or return; # empty, uninitialized git repo
 	$ent->{fingerprint} = $dig->hexdigest;
 	$ent->{modified} = modified(undef, $mod);

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 05/30] import: use read_all to detect short reads
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (3 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

Our Import package was depending on Git, anyways.
---
 lib/PublicInbox/Import.pm | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 6bb2c66d..fb70b91b 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -21,6 +21,7 @@ use POSIX qw(strftime);
 use autodie qw(read close socketpair);
 use Carp qw(croak);
 use Socket qw(AF_UNIX SOCK_STREAM);
+use PublicInbox::Git qw(read_all);
 
 sub default_branch () {
 	state $default_branch = do {
@@ -114,8 +115,7 @@ sub _cat_blob ($$) {
 	local $/ = "\n";
 	my $info = <$io> // die "EOF from fast-import / cat-blob: $!";
 	$info =~ /\A[a-f0-9]{40,} blob ([0-9]+)\n\z/ or return;
-	my $n = read($io, my $buf, my $len = $1 + 1);
-	$n == $len or croak "cat-blob: short read: $n < $len";
+	my $buf = read_all($io, my $len = $1 + 1);
 	my $lf = chop $buf;
 	croak "bad read on final byte: <$lf>" if $lf ne "\n";
 	\$buf;
@@ -562,9 +562,7 @@ sub replace_oids {
 		} elsif (/^data ([0-9]+)/) {
 			# only commit message, so $len is small:
 			push @buf, $_;
-			my $n = read($rd, my $buf, my $len = $1);
-			$len == $n or croak "short read ($n < $len)";
-			push @buf, $buf;
+			push @buf, read_all($rd, my $len = $1);
 		} elsif (/^M 100644 ([a-f0-9]+) (\w+)/) {
 			my ($oid, $path) = ($1, $2);
 			$tree->{$path} = 1;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 06/30] lei_mirror: use read_all
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (4 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This gives us better error checking for regular files.
---
 lib/PublicInbox/LeiMirror.pm | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index b402eb5f..47fb767b 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -16,7 +16,7 @@ use Carp qw(croak);
 use URI;
 use PublicInbox::Config qw(glob2re);
 use PublicInbox::Inbox;
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
 use PublicInbox::LeiCurl;
 use PublicInbox::OnDestroy;
 use PublicInbox::SHA qw(sha256_hex sha1_hex);
@@ -174,8 +174,7 @@ sub _get_txt_done { # returns true on error (non-fatal), undef on success
 	return warn("# @$cmd failed (non-fatal)\n") if $cerr;
 	seek($fh, SEEK_SET, 0);
 	$self->{"mtime.$endpoint"} = (stat($fh))[9];
-	local $/;
-	$self->{"txt.$endpoint"} = <$fh>;
+	$self->{"txt.$endpoint"} = read_all($fh, -s _);
 	undef; # success
 }
 
@@ -537,7 +536,7 @@ sub fp_done {
 	seek($fh, SEEK_SET, 0);
 	$self->{-ent} // die 'BUG: no -ent';
 	my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
-	my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
+	my $B = sha1_hex(read_all($fh));
 	return $cb->($self, @arg) if $A ne $B;
 	$self->{lei}->qerr("# $self->{-key} up-to-date");
 }
@@ -734,7 +733,7 @@ sub up_fp_done {
 	seek($fh, SEEK_SET, 0);
 	$self->{-ent} // die 'BUG: no -ent';
 	my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
-	my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
+	my $B = sha1_hex(read_all($fh));
 	return if $A eq $B;
 	$self->{-ent}->{fingerprint} = $B;
 	push @{$self->{chg}->{fp_mismatch}}, $self->{-key};
@@ -948,7 +947,7 @@ failed to extract epoch number from $src
 sub decode_manifest ($$$) {
 	my ($fh, $fn, $uri) = @_;
 	my $js;
-	my $gz = do { local $/; <$fh> } // die "slurp($fn): $!";
+	my $gz = read_all($fh);
 	gunzip(\$gz => \$js, MultiStream => 1) or
 		die "gunzip($uri): $GunzipError\n";
 	my $m = eval { PublicInbox::Config->json->decode($js) };

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 07/30] use read_all in more places to improve safety
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (5 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

`readline' ops may not detect errors on partial reads.
This saves us some code to reduce cognitive overhead for
readers.  We'll also support reusing a destination buffers so it
can work more nicely with existing code.
---
 lib/PublicInbox/Gcf2.pm          |  7 +++----
 lib/PublicInbox/Git.pm           |  9 +++++----
 lib/PublicInbox/InboxWritable.pm |  6 +++---
 lib/PublicInbox/LeiALE.pm        | 11 +++--------
 lib/PublicInbox/LeiBlob.pm       |  6 +++---
 lib/PublicInbox/LeiConfig.pm     |  2 +-
 lib/PublicInbox/LeiMailSync.pm   |  5 ++---
 lib/PublicInbox/LeiSucks.pm      |  5 +++--
 lib/PublicInbox/MultiGit.pm      |  3 ++-
 lib/PublicInbox/ViewVCS.pm       | 12 +++++-------
 lib/PublicInbox/WWW.pm           |  4 ++--
 lib/PublicInbox/XapHelper.pm     |  3 ++-
 lib/PublicInbox/XapHelperCxx.pm  |  6 +++---
 script/public-inbox-edit         |  4 ++--
 script/public-inbox-init         |  3 ++-
 15 files changed, 41 insertions(+), 45 deletions(-)

diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index 37262e28..4f163cde 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -9,7 +9,7 @@ use PublicInbox::Spawn qw(which popen_rd); # may set PERL_INLINE_DIRECTORY
 use Fcntl qw(SEEK_SET);
 use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
 use IO::Handle; # autoflush
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
 use PublicInbox::Lock;
 
 BEGIN {
@@ -43,12 +43,11 @@ BEGIN {
 		# build them.
 		my $f = "$dir/gcf2_libgit2.h";
 		open my $src, '<', $f;
-		local $/;
-		$c_src = <$src> // die "read $f: $!";
+		$c_src = read_all($src);
 	}
 	unless ($c_src) {
 		seek($err, 0, SEEK_SET);
-		$err = do { local $/; <$err> };
+		$err = read_all($err);
 		die "E: libgit2 not installed: $err\n";
 	}
 	# append pkg-config results to the source to ensure Inline::C
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 35bd10ef..a460d155 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -550,11 +550,12 @@ sub modified ($;$) {
 
 # read_all/try_cat can probably be moved somewhere else...
 
-sub read_all ($;$) {
-	my ($fh, $len) = @_;
-	my $r = read($fh, my $buf, $len //= -s $fh);
+sub read_all ($;$$) {
+	my ($fh, $len, $bref) = @_;
+	$bref //= \(my $buf);
+	my $r = read($fh, $$bref, $len //= -s $fh);
 	croak("$fh read ($r != $len)") if $len != $r;
-	$buf;
+	$$bref;
 }
 
 sub try_cat {
diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index 65952aa2..6af72e71 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -7,6 +7,7 @@ use strict;
 use v5.10.1;
 use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter);
 use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
 use PublicInbox::Filter::Base qw(REJECT);
 use Errno qw(ENOENT);
 our @EXPORT_OK = qw(eml_from_path);
@@ -114,9 +115,8 @@ sub filter {
 sub eml_from_path ($) {
 	my ($path) = @_;
 	if (sysopen(my $fh, $path, O_RDONLY|O_NONBLOCK)) {
-		return unless -f $fh; # no FIFOs or directories
-		my $str = do { local $/; <$fh> } or return;
-		PublicInbox::Eml->new(\$str);
+		return unless -f $fh && -s _; # no FIFOs or directories
+		PublicInbox::Eml->new(\(my $str = read_all($fh, -s _)));
 	} else { # ENOENT is common with Maildir
 		warn "failed to open $path: $!\n" if $! != ENOENT;
 		undef;
diff --git a/lib/PublicInbox/LeiALE.pm b/lib/PublicInbox/LeiALE.pm
index cc9a2095..b198af1c 100644
--- a/lib/PublicInbox/LeiALE.pm
+++ b/lib/PublicInbox/LeiALE.pm
@@ -9,7 +9,7 @@ package PublicInbox::LeiALE;
 use strict;
 use v5.10.1;
 use parent qw(PublicInbox::LeiSearch PublicInbox::Lock);
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
 use PublicInbox::Import;
 use PublicInbox::LeiXSearch;
 use Fcntl qw(SEEK_SET);
@@ -54,11 +54,7 @@ sub refresh_externals {
 	$self->git->cleanup;
 	my $lk = $self->lock_for_scope;
 	my $cur_lxs = ref($lxs)->new;
-	my $orig = do {
-		local $/;
-		readline($self->{lockfh}) //
-				die "readline($self->{lock_path}): $!";
-	};
+	my $orig = read_all($self->{lockfh});
 	my $new = '';
 	my $old = '';
 	my $gone = 0;
@@ -91,8 +87,7 @@ sub refresh_externals {
 	$new = $old = '';
 	my $f = $self->git->{git_dir}.'/objects/info/alternates';
 	if (open my $fh, '<', $f) {
-		local $/;
-		$old = <$fh> // die "readline($f): $!";
+		read_all($fh, -s $fh, \$old);
 	}
 	for my $x (@ibxish) {
 		$new .= $lei->canonpath_harder($x->git->{git_dir})."/objects\n";
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index d069d4a8..40f64bd9 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -10,6 +10,7 @@ use parent qw(PublicInbox::IPC);
 use PublicInbox::Spawn qw(run_wait popen_rd which);
 use PublicInbox::DS;
 use PublicInbox::Eml;
+use PublicInbox::Git qw(read_all);
 
 sub get_git_dir ($$) {
 	my ($lei, $d) = @_;
@@ -137,9 +138,8 @@ sub lei_blob {
 					extract_attach($lei, $blob, $bref) :
 					$lei->out($$bref);
 		if ($opt->{mail}) {
-			my $eh = $rdr->{2};
-			seek($eh, 0, 0);
-			return $lei->child_error($cerr, do { local $/; <$eh> });
+			seek($rdr->{2}, 0, 0);
+			return $lei->child_error($cerr, read_all($rdr->{2}));
 		} # else: fall through to solver below
 	}
 
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index b3495487..c47708d8 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -28,7 +28,7 @@ sub cfg_edit_done { # PktOp lei->do_env cb
 		$lei->cfg_dump($self->{-f});
 	} or do {
 		seek($fh, 0, SEEK_SET);
-		return cfg_do_edit($self, do { local $/; <$fh> });
+		return cfg_do_edit($self, read_all($fh));
 	};
 	$self->cfg_verify($cfg) if $self->can('cfg_verify');
 }
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index 415459d5..74ef1362 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -10,7 +10,7 @@ use PublicInbox::Compat qw(uniqstr);
 use DBI qw(:sql_types); # SQL_BLOB
 use PublicInbox::ContentHash qw(git_sha);
 use Carp ();
-use PublicInbox::Git qw(%HEXLEN2SHA);
+use PublicInbox::Git qw(%HEXLEN2SHA read_all);
 
 sub dbh_new {
 	my ($self) = @_;
@@ -456,8 +456,7 @@ WHERE b.oidbin = ?
 			open my $fh, '<', $f or next;
 			# some (buggy) Maildir writers are non-atomic:
 			next unless -s $fh;
-			local $/;
-			my $raw = <$fh>;
+			my $raw = read_all($fh, -s _);
 			if ($vrfy) {
 				my $sha = $HEXLEN2SHA{length($oidhex)};
 				my $got = git_sha($sha, \$raw)->hexdigest;
diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm
index 35d0a8de..82aea8d4 100644
--- a/lib/PublicInbox/LeiSucks.pm
+++ b/lib/PublicInbox/LeiSucks.pm
@@ -12,6 +12,7 @@ use Config;
 use POSIX ();
 use PublicInbox::Config;
 use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
 
 sub lei_sucks {
 	my ($lei, @argv) = @_;
@@ -58,8 +59,8 @@ sub lei_sucks {
 	for my $m (grep(m{^PublicInbox/}, sort keys %INC)) {
 		my $f = $INC{$m} // next; # lazy require failed (missing dep)
 		open my $fh, '<', $f or do { warn "open($f): $!"; next };
-		my $hex = sha1_hex('blob '.(-s $fh)."\0".
-				(do { local $/; <$fh> } // die("read: $!")));
+		my $size = -s $fh;
+		my $hex = sha1_hex("blob $size\0".read_all($fh, $size));
 		push @out, '  '.$hex.' '.$m."\n";
 	}
 	push @out, <<'EOM';
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 74a9e1df..35bd0251 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -9,6 +9,7 @@ use PublicInbox::Spawn qw(run_die popen_rd);
 use PublicInbox::Import;
 use File::Temp 0.19;
 use List::Util qw(max);
+use PublicInbox::Git qw(read_all);
 
 sub new {
 	my ($cls, $topdir, $all, $epfx) = @_;
@@ -31,7 +32,7 @@ sub read_alternates {
 			qr!\A\Q../../$self->{epfx}\E/([0-9]+)\.git/objects\z! :
 			undef;
 		$$moderef = (stat($fh))[2] & 07777;
-		for my $rel (split(/^/m, do { local $/; <$fh> })) {
+		for my $rel (split(/^/m, read_all($fh, -s _))) {
 			chomp(my $dir = $rel);
 			my $score;
 			if (defined($is_edir) && $dir =~ $is_edir) {
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 8a90c2d6..86c46e69 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -17,6 +17,7 @@ use strict;
 use v5.10.1;
 use File::Temp 0.19 (); # newdir
 use PublicInbox::SolverGit;
+use PublicInbox::Git qw(read_all);
 use PublicInbox::GitAsyncCat;
 use PublicInbox::WwwStream qw(html_oneshot);
 use PublicInbox::Linkify;
@@ -61,12 +62,9 @@ sub dbg_log ($) {
 		warn "seek(log): $!";
 		return '<pre>debug log seek error</pre>';
 	}
-	$log = do { local $/; <$log> } // do {
-		if (!eof($log)) {
-			warn "readline(log): $!";
-			return '<pre>debug log read error</pre>';
-		}
-		'';
+	$log = eval { read_all($log) } // do {
+		warn "read(log): $@";
+		return '<pre>debug log read error</pre>';
 	};
 	return '' if $log eq '';
 	$ctx->{-linkify} //= PublicInbox::Linkify->new;
@@ -251,7 +249,7 @@ EOM
 	if (-s $fh > $MAX_SIZE) {
 		print $zfh "---\n patch is too large to show\n";
 	} else { # prepare flush_diff:
-		read($fh, $x, -s _);
+		read_all($fh, -s _, \$x);
 		utf8_maybe($x);
 		$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
 		$x =~ s/\r?\n/\n/gs;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 9919f975..86fd7e22 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -587,9 +587,9 @@ sub stylesheets_prepare ($$) {
 				next;
 			};
 			my $ctime = 0;
-			my $local = do { local $/; <$fh> };
+			my $local = read_all($fh, -s $fh);
 			if ($local =~ /\S/) {
-				$ctime = sprintf('%x',(stat($fh))[10]);
+				$ctime = sprintf('%x',(stat(_))[10]);
 				$local = $mini->($local);
 			}
 
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ae907766..ca993ca8 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -10,6 +10,7 @@ $GLP->configure(qw(require_order bundling no_ignore_case no_auto_abbrev));
 use PublicInbox::Search qw(xap_terms);
 use PublicInbox::CodeSearch;
 use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
 use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
 use PublicInbox::DS qw(awaitpid);
 use POSIX qw(:signal_h);
@@ -123,7 +124,7 @@ sub cmd_dump_roots {
 	$req->{A} or return warn('dump_roots requires -A PREFIX');
 	open my $fh, '<', $root2id_file or die "open($root2id_file): $!";
 	my $root2id; # record format: $OIDHEX "\0" uint32_t
-	my @x = split(/\0/, do { local $/; <$fh> } // die "readline: $!");
+	my @x = split(/\0/, read_all($fh));
 	while (@x) {
 		my $oidhex = shift @x;
 		$root2id->{$oidhex} = shift @x;
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index dbb0a915..83015379 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -8,6 +8,7 @@
 package PublicInbox::XapHelperCxx;
 use v5.12;
 use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Git qw(read_all);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
 use Config;
@@ -34,7 +35,7 @@ sub xap_cfg (@) {
 	chomp(my $ret = do { local $/; <$rd> });
 	return $ret if close($rd);
 	seek($err, 0, SEEK_SET) or die "seek: $!";
-	$err = do { local $/; <$err> };
+	$err = read_all($err);
 	die <<EOM;
 @$cmd failed: Xapian development files missing? (\$?=$?)
 $err
@@ -70,8 +71,7 @@ sub build () {
 	for (@srcs) {
 		say $fh qq(# line 1 "$_");
 		open my $rfh, '<', $_;
-		local $/;
-		print $fh readline($rfh);
+		print $fh read_all($rfh);
 	}
 	print $fh PublicInbox::Search::generate_cxx();
 	print $fh PublicInbox::CodeSearch::generate_cxx();
diff --git a/script/public-inbox-edit b/script/public-inbox-edit
index 1fb6f32b..77028817 100755
--- a/script/public-inbox-edit
+++ b/script/public-inbox-edit
@@ -15,6 +15,7 @@ PublicInbox::Admin::check_require('-index');
 use PublicInbox::Eml;
 use PublicInbox::InboxWritable qw(eml_from_path);
 use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
 
 my $help = <<'EOF';
 usage: public-inbox-edit -m MESSAGE-ID [--all] [INBOX_DIRS]
@@ -184,8 +185,7 @@ retry_edit:
 	# rename/relink $edit_fn
 	open my $new_fh, '<', $edit_fn or
 		die "can't read edited file ($edit_fn): $!\n";
-	defined(my $new_raw = do { local $/; <$new_fh> }) or die
-		"read $edit_fn: $!\n";
+	my $new_raw = read_all($new_fh);
 
 	if (!$opt->{raw}) {
 		PublicInbox::Eml::strip_from($new_raw);
diff --git a/script/public-inbox-init b/script/public-inbox-init
index b3a16cfb..33bee310 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -125,13 +125,14 @@ my $auto_unlink = PublicInbox::OnDestroy->new($$, sub { unlink $lockfile });
 my $perm = 0644 & ~umask;
 my %seen;
 if (-e $pi_config) {
+	require PublicInbox::Git;
 	open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";
 	my @st = stat($oh);
 	$perm = $st[2];
 	defined $perm or die "(f)stat failed on $pi_config: $!\n";
 	chmod($perm & 07777, $fh) or
 		die "(f)chmod failed on future $pi_config: $!\n";
-	defined(my $old = do { local $/; <$oh> }) or die "read $pi_config: $!\n";
+	my $old = PublicInbox::Git::read_all($oh);
 	print $fh $old or die "failed to write: $!\n";
 	close $oh or die "failed to close $pi_config: $!\n";
 

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 08/30] xap_helper*: use autodie in more places
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (6 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
                   ` (22 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This ought to lower the cognitive overhead of reading our code.
---
 lib/PublicInbox/XapHelperCxx.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 83015379..bd616b6f 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -29,12 +29,13 @@ my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
 my $xap_modversion;
 
 sub xap_cfg (@) {
-	open my $err, '+>', undef or die "open(undef): $!";
+	use autodie qw(open seek);
+	open my $err, '+>', undef;
 	my $cmd = [ $ENV{PKG_CONFIG} // 'pkg-config', @_, 'xapian-core' ];
 	my $rd = popen_rd($cmd, undef, { 2 => $err });
 	chomp(my $ret = do { local $/; <$rd> });
 	return $ret if close($rd);
-	seek($err, 0, SEEK_SET) or die "seek: $!";
+	seek($err, 0, SEEK_SET);
 	$err = read_all($err);
 	die <<EOM;
 @$cmd failed: Xapian development files missing? (\$?=$?)

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 09/30] xap_helper: die more easily in both implementations
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (7 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

We don't need to tolerate bad requests since it's only handling
requests from the parent process.  So simplify error management
and just die||exit if we get a bad request.
---
 lib/PublicInbox/XapHelper.pm | 11 +++--------
 lib/PublicInbox/xap_helper.h | 32 ++++++++++----------------------
 2 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ca993ca8..fe8d20f4 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -13,6 +13,7 @@ use PublicInbox::IPC;
 use PublicInbox::Git qw(read_all);
 use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
 use PublicInbox::DS qw(awaitpid);
+use autodie qw(open);
 use POSIX qw(:signal_h);
 use Fcntl qw(LOCK_UN LOCK_EX);
 my $X = \%PublicInbox::Search::X;
@@ -122,7 +123,7 @@ sub cmd_dump_roots {
 	$qry_str // return
 		warn('usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR');
 	$req->{A} or return warn('dump_roots requires -A PREFIX');
-	open my $fh, '<', $root2id_file or die "open($root2id_file): $!";
+	open my $fh, '<', $root2id_file;
 	my $root2id; # record format: $OIDHEX "\0" uint32_t
 	my @x = split(/\0/, read_all($fh));
 	while (@x) {
@@ -184,13 +185,7 @@ sub recv_loop {
 		PublicInbox::DS::block_signals();
 		my $req = bless {}, __PACKAGE__;
 		my $i = 0;
-		for my $fd (@fds) {
-			open($req->{$i++}, '+<&=', $fd) and next;
-			warn("open(+<&=$fd) (FD=$i): $!");
-			undef $req;
-			last;
-		}
-		$req or next;
+		open($req->{$i++}, '+<&=', $_) for @fds;
 		local $stderr = $req->{1} // \*STDERR;
 		if (chop($rbuf) ne "\0") {
 			warn "not NUL-terminated";
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index 3fa615a5..c68202c3 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -668,40 +668,28 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
 		size_t len = cmsg.hdr.cmsg_len;
 		int *fdp = (int *)CMSG_DATA(&cmsg.hdr);
 		size_t i;
-		bool fd_ok = true;
 		for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= len; i++) {
 			int fd = *fdp++;
 			const char *mode = NULL;
-			int fl = fd_ok ? fcntl(fd, F_GETFL) : 0;
-			if (fl == 0) {
-				continue; // hit previous error
-			} else if (fl == -1) {
-				warnx("invalid fd=%d", fd);
-				fd_ok = false;
+			int fl = fcntl(fd, F_GETFL);
+			if (fl == -1) {
+				errx(EXIT_FAILURE, "invalid fd=%d", fd);
 			} else if (fl & O_WRONLY) {
 				mode = "w";
 			} else if (fl & O_RDWR) {
 				mode = "r+";
 				if (i == 0) req->has_input = true;
 			} else {
-				warnx("invalid mode from F_GETFL: 0x%x", fl);
-				fd_ok = false;
-			}
-			if (!fd_ok) {
-				xclose(fd);
-			} else {
-				req->fp[i] = fdopen(fd, mode);
-				if (!req->fp[i]) {
-					warn("fdopen(fd=%d)", fd);
-					fd_ok = false;
-				}
+				errx(EXIT_FAILURE,
+					"invalid mode from F_GETFL: 0x%x", fl);
 			}
+			req->fp[i] = fdopen(fd, mode);
+			if (!req->fp[i])
+				err(EXIT_FAILURE, "fdopen(fd=%d)", fd);
 		}
-		for (i = 0; !fd_ok && i < MY_ARRAY_SIZE(req->fp); i++)
-			if (req->fp[i]) fclose(req->fp[i]);
-		return fd_ok;
+		return true;
 	}
-	warnx("no FD received in %zd-byte request", r);
+	errx(EXIT_FAILURE, "no FD received in %zd-byte request", r);
 	return false;
 }
 

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 10/30] xap_helper: simplify SIGTERM exit checks
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (8 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

We can just close the socket FD to ensure things fail ASAP
when a SIGTERM hits instead of wasting time making getppid(2)
syscalls.
---
 lib/PublicInbox/XapHelper.pm |  7 +++----
 lib/PublicInbox/xap_helper.h | 28 ++++++++++++++++------------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index fe8d20f4..c31fe9a2 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -17,7 +17,7 @@ use autodie qw(open);
 use POSIX qw(:signal_h);
 use Fcntl qw(LOCK_UN LOCK_EX);
 my $X = \%PublicInbox::Search::X;
-our (%SRCH, %WORKERS, $parent_pid, $alive, $nworker, $workerset);
+our (%SRCH, %WORKERS, $alive, $nworker, $workerset);
 our $stderr = \*STDERR;
 
 # only short options for portability in C++ implementation
@@ -177,7 +177,8 @@ sub recv_loop {
 	local $SIG{__WARN__} = sub { print $stderr @_ };
 	my $rbuf;
 	my $in = \*STDIN;
-	while (!defined($parent_pid) || getppid == $parent_pid) {
+	local $SIG{TERM} = sub { undef $in };
+	while (defined($in)) {
 		PublicInbox::DS::sig_setmask($workerset);
 		my @fds = $PublicInbox::IPC::recv_cmd->($in, $rbuf, 4096*33);
 		scalar(@fds) or exit(66); # EX_NOINPUT
@@ -218,7 +219,6 @@ sub start_worker ($) {
 	if ($pid == 0) {
 		undef %WORKERS;
 		PublicInbox::DS::Reset();
-		$SIG{TERM} = sub { $parent_pid = -1 };
 		$SIG{TTIN} = $SIG{TTOU} = 'IGNORE';
 		$SIG{CHLD} = 'DEFAULT'; # Xapian may use this
 		recv_loop();
@@ -267,7 +267,6 @@ sub start (@) {
 	for (POSIX::SIGTERM, POSIX::SIGCHLD) {
 		$workerset->delset($_) or die "delset($_): $!";
 	}
-	local $parent_pid = $$;
 	my $sig = {
 		TTIN => sub {
 			if ($alive) {
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index c68202c3..fafb392d 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -72,8 +72,8 @@
 	assert(ckvar______ == (expect) && "BUG" && __FILE__ && __LINE__); \
 } while (0)
 
-static const int sock_fd = STDIN_FILENO; // SOCK_SEQPACKET as stdin :P
-static volatile pid_t parent_pid; // may be set in worker sighandler (sigw)
+// sock_fd is modified in signal handler, yes, it's SOCK_SEQPACKET
+static volatile int sock_fd = STDIN_FILENO;
 static sigset_t fullset, workerset;
 static bool alive = true;
 #if STDERR_ASSIGNABLE
@@ -640,6 +640,7 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
 	union my_cmsg cmsg = {};
 	struct msghdr msg = {};
 	struct iovec iov;
+	ssize_t r;
 	iov.iov_base = rbuf;
 	iov.iov_len = *len;
 	msg.msg_iov = &iov;
@@ -650,25 +651,29 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
 	// allow SIGTERM to hit
 	CHECK(int, 0, sigprocmask(SIG_SETMASK, &workerset, NULL));
 
-	ssize_t r = recvmsg(sock_fd, &msg, 0);
+again:
+	r = recvmsg(sock_fd, &msg, 0);
 	if (r == 0) {
 		exit(EX_NOINPUT); /* grandparent went away */
 	} else if (r < 0) {
-		if (errno == EINTR)
-			return false; // retry recv_loop
-		err(EXIT_FAILURE, "recvmsg");
+		switch (errno) {
+		case EINTR: goto again;
+		case EBADF: if (sock_fd < 0) exit(0);
+			// fall-through
+		default: err(EXIT_FAILURE, "recvmsg");
+		}
 	}
 
 	// success! no signals for the rest of the request/response cycle
 	CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, NULL));
 
 	*len = r;
-	if (r > 0 && cmsg.hdr.cmsg_level == SOL_SOCKET &&
+	if (cmsg.hdr.cmsg_level == SOL_SOCKET &&
 			cmsg.hdr.cmsg_type == SCM_RIGHTS) {
-		size_t len = cmsg.hdr.cmsg_len;
+		size_t clen = cmsg.hdr.cmsg_len;
 		int *fdp = (int *)CMSG_DATA(&cmsg.hdr);
 		size_t i;
-		for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= len; i++) {
+		for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= clen; i++) {
 			int fd = *fdp++;
 			const char *mode = NULL;
 			int fl = fcntl(fd, F_GETFL);
@@ -923,7 +928,7 @@ static void stderr_restore(FILE *tmp_err)
 
 static void sigw(int sig) // SIGTERM handler for worker
 {
-	parent_pid = -1; // break out of recv_loop
+	sock_fd = -1; // break out of recv_loop
 }
 
 static void recv_loop(void) // worker process loop
@@ -934,7 +939,7 @@ static void recv_loop(void) // worker process loop
 
 	CHECK(int, 0, sigaction(SIGTERM, &sa, NULL));
 
-	while (!parent_pid || getppid() == parent_pid) {
+	while (sock_fd == 0) {
 		size_t len = sizeof(rbuf);
 		struct req req = {};
 		if (!recv_req(&req, rbuf, &len))
@@ -1197,7 +1202,6 @@ int main(int argc, char *argv[])
 	}
 	CHECK(int, 0, sigdelset(&workerset, SIGTERM));
 	CHECK(int, 0, sigdelset(&workerset, SIGCHLD));
-	parent_pid = getpid();
 	nworker_hwm = nworker;
 	worker_pids = (pid_t *)calloc(nworker, sizeof(pid_t));
 	if (!worker_pids) err(EXIT_FAILURE, "calloc");

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 11/30] xap_helper: autodie for getsockopt
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (9 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
                   ` (19 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

Only caveat is we can't use bareword filehandles, but that's
a minor inconvenience.
---
 lib/PublicInbox/XapHelper.pm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index c31fe9a2..eea10a44 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -13,11 +13,11 @@ use PublicInbox::IPC;
 use PublicInbox::Git qw(read_all);
 use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
 use PublicInbox::DS qw(awaitpid);
-use autodie qw(open);
+use autodie qw(open getsockopt);
 use POSIX qw(:signal_h);
 use Fcntl qw(LOCK_UN LOCK_EX);
 my $X = \%PublicInbox::Search::X;
-our (%SRCH, %WORKERS, $alive, $nworker, $workerset);
+our (%SRCH, %WORKERS, $alive, $nworker, $workerset, $in);
 our $stderr = \*STDERR;
 
 # only short options for portability in C++ implementation
@@ -176,7 +176,6 @@ sub dispatch {
 sub recv_loop {
 	local $SIG{__WARN__} = sub { print $stderr @_ };
 	my $rbuf;
-	my $in = \*STDIN;
 	local $SIG{TERM} = sub { undef $in };
 	while (defined($in)) {
 		PublicInbox::DS::sig_setmask($workerset);
@@ -247,7 +246,7 @@ sub xh_alive { $alive || scalar(keys %WORKERS) }
 
 sub start (@) {
 	my (@argv) = @_;
-	my $c = getsockopt(STDIN, SOL_SOCKET, SO_TYPE) or die "getsockopt: $!";
+	my $c = getsockopt($in = \*STDIN, SOL_SOCKET, SO_TYPE);
 	unpack('i', $c) == SOCK_SEQPACKET or die 'stdin is not SOCK_SEQPACKET';
 
 	local (%SRCH, %WORKERS);

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 12/30] xap_client: autodie for pipe and socketpair
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (10 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This saves us a few lines of code.
---
 lib/PublicInbox/XapClient.pm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/XapClient.pm b/lib/PublicInbox/XapClient.pm
index 9e2d71a0..21c89265 100644
--- a/lib/PublicInbox/XapClient.pm
+++ b/lib/PublicInbox/XapClient.pm
@@ -11,14 +11,12 @@ use v5.12;
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_SEQPACKET);
 use PublicInbox::IPC;
+use autodie qw(pipe socketpair);
 
 sub mkreq {
 	my ($self, $ios, @arg) = @_;
-	my ($r, $w, $n);
-	if (!defined($ios->[0])) {
-		pipe($r, $w) or die "pipe: $!";
-		$ios->[0] = $w;
-	}
+	my ($r, $n);
+	pipe($r, $ios->[0]) if !defined($ios->[0]);
 	my @fds = map fileno($_), @$ios;
 	my $buf = join("\0", @arg, '');
 	$n = $PublicInbox::IPC::send_cmd->($self->{io}, \@fds, $buf, 0) //
@@ -29,8 +27,7 @@ sub mkreq {
 
 sub start_helper {
 	my @argv = @_;
-	socketpair(my $sock, my $in, AF_UNIX, SOCK_SEQPACKET, 0) or
-		die "socketpair: $!";
+	socketpair(my $sock, my $in, AF_UNIX, SOCK_SEQPACKET, 0);
 	my $cls = ($ENV{PI_NO_CXX} ? undef : eval {
 			require PublicInbox::XapHelperCxx;
 			PublicInbox::XapHelperCxx::check_build();

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (11 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
                   ` (17 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

HTTP::Tiny is part of the Perl standard library since Perl 5.14
while Net::HTTP has never been (unlike Net::NNTP or Net::POP3).
For the test which forces server-side buffering, we'll just use
regular socket handle.
---
 xt/git-http-backend.t | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/xt/git-http-backend.t b/xt/git-http-backend.t
index d78fe79f..6c384faf 100644
--- a/xt/git-http-backend.t
+++ b/xt/git-http-backend.t
@@ -12,7 +12,7 @@ use PublicInbox::TestCommon;
 my $git_dir = $ENV{GIANT_GIT_DIR};
 plan 'skip_all' => 'GIANT_GIT_DIR not defined' unless $git_dir;
 require_mods(qw(BSD::Resource Plack::Util Plack::Builder
-		HTTP::Date HTTP::Status Net::HTTP));
+		HTTP::Date HTTP::Status HTTP::Tiny));
 my $psgi = "./t/git-http-backend.psgi";
 my ($tmpdir, $for_destroy) = tmpdir();
 my $err = "$tmpdir/stderr.log";
@@ -20,15 +20,12 @@ my $out = "$tmpdir/stdout.log";
 my $sock = tcp_server();
 my ($host, $port) = tcp_host_port($sock);
 my $td;
+my $http = HTTP::Tiny->new;
 
 my $get_maxrss = sub {
-        my $http = Net::HTTP->new(Host => "$host:$port");
-	ok($http, 'Net::HTTP object created for maxrss');
-        $http->write_request(GET => '/');
-        my ($code, $mess, %h) = $http->read_response_headers;
-	is($code, 200, 'success reading maxrss');
-	my $n = $http->read_entity_body(my $buf, 256);
-	ok(defined $n, 'read response body');
+	my $res = $http->get("http://$host:$port/");
+	is($res->{status}, 200, 'success reading maxrss');
+	my $buf = $res->{content};
 	like($buf, qr/\A\d+\n\z/, 'got memory response');
 	ok(int($buf) > 0, 'got non-zero memory response');
 	int($buf);
@@ -55,16 +52,15 @@ SKIP: {
 	if ($pack !~ m!(/objects/pack/pack-[a-f0-9]{40,64}.pack)\z!) {
 		skip "bad pack name: $pack";
 	}
-	my $url = $1;
-	my $http = Net::HTTP->new(Host => "$host:$port");
-	ok($http, 'Net::HTTP object created');
-	$http->write_request(GET => $url);
-	my ($code, $mess, %h) = $http->read_response_headers;
-	is(200, $code, 'got 200 success for pack');
-	is($max, $h{'Content-Length'}, 'got expected Content-Length for pack');
+	my $s = tcp_connect($sock);
+	print $s "GET $1 HTTP/1.1\r\nHost: $host:$port\r\n\r\n" or xbail $!;
+	my $hdr = do { local $/ = "\r\n\r\n"; readline($s) };
+	like $hdr, qr!\AHTTP/1\.1\s+200\b!, 'got 200 success for pack';
+	like $hdr, qr/^content-length:\s*$max\r\n/ims,
+		'got expected Content-Length for pack';
 
-	# no $http->read_entity_body, here, since we want to force buffering
-	foreach my $i (1..3) {
+	# don't read the body
+	for my $i (1..3) {
 		sleep 1;
 		my $diff = $get_maxrss->() - $mem_a;
 		note "${diff}K memory increase after $i seconds";

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 14/30] ds: introduce and use do_fork helper
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (12 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
  To: meta

This ensures we handle RNG reseeding and resetting the event
loop properly in child processes after forking.
---
 lib/PublicInbox/DS.pm         | 12 ++++++++++++
 lib/PublicInbox/Daemon.pm     | 16 +++++-----------
 lib/PublicInbox/IPC.pm        | 12 ++----------
 lib/PublicInbox/TestCommon.pm |  3 +--
 lib/PublicInbox/Watch.pm      | 11 ++---------
 lib/PublicInbox/Xapcmd.pm     |  5 ++---
 6 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index eefbdcc3..9960937d 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -34,6 +34,7 @@ use PublicInbox::Tmpfile;
 use PublicInbox::Select;
 use Errno qw(EAGAIN EINVAL ECHILD);
 use Carp qw(carp croak);
+use autodie qw(fork);
 our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer);
 
 my $nextq; # queue for next_tick
@@ -737,6 +738,17 @@ sub awaitpid {
 	}
 }
 
+sub do_fork () {
+	my $seed = rand(0xffffffff);
+	my $pid = fork;
+	if ($pid == 0) {
+		srand($seed);
+		eval { Net::SSLeay::randomize() };
+		Reset();
+	}
+	$pid;
+}
+
 package PublicInbox::DummyPoller; # only used during Reset
 use v5.12;
 
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 520cef72..f33f6f17 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -541,17 +541,11 @@ sub reap_worker { # awaitpid CB
 
 sub start_worker ($) {
 	my ($nr) = @_;
-	my $seed = rand(0xffffffff);
 	return unless @listeners;
-	my $pid = fork;
-	if (!defined($pid)) {
-		warn "fork: $!";
-	} elsif ($pid == 0) {
+	my $pid = PublicInbox::DS::do_fork;
+	if ($pid == 0) {
 		undef %WORKERS;
-		PublicInbox::DS::Reset();
 		local $PublicInbox::DS::Poller; # allow epoll/kqueue
-		srand($seed);
-		eval { Net::SSLeay::randomize() };
 		$set_user->() if $set_user;
 		PublicInbox::EOFpipe->new($parent_pipe, \&worker_quit);
 		worker_loop();
@@ -563,9 +557,9 @@ sub start_worker ($) {
 }
 
 sub start_workers {
-	for my $nr (grep { !defined($WORKERS{$_}) } (0..($nworker - 1))) {
-		start_worker($nr);
-	}
+	my @idx = grep { !defined($WORKERS{$_}) } (0..($nworker - 1)) or return;
+	eval { start_worker($_) for @idx };
+	warn "E: $@\n" if $@;
 }
 
 sub trim_workers {
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 5964645e..3292d960 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -102,12 +102,8 @@ sub ipc_worker_spawn {
 	pipe(my $r_res, my $w_res);
 	my $sigset = $oldset // PublicInbox::DS::block_signals();
 	$self->ipc_atfork_prepare;
-	my $seed = rand(0xffffffff);
-	my $pid = fork;
+	my $pid = PublicInbox::DS::do_fork;
 	if ($pid == 0) {
-		srand($seed);
-		eval { Net::SSLeay::randomize() };
-		eval { PublicInbox::DS->Reset };
 		delete @$self{qw(-wq_s1 -wq_s2 -wq_workers -wq_ppid)};
 		$w_req = $r_res = undef;
 		$w_res->autoflush(1);
@@ -341,13 +337,9 @@ sub _wq_worker_start {
 	my ($self, $oldset, $fields, $one, @cb_args) = @_;
 	my ($bcast1, $bcast2);
 	$one or socketpair($bcast1, $bcast2, AF_UNIX, SOCK_SEQPACKET, 0);
-	my $seed = rand(0xffffffff);
-	my $pid = fork;
+	my $pid = PublicInbox::DS::do_fork;
 	if ($pid == 0) {
-		srand($seed);
-		eval { Net::SSLeay::randomize() };
 		undef $bcast1;
-		eval { PublicInbox::DS->Reset };
 		delete @$self{qw(-wq_s1 -wq_ppid)};
 		$self->{-wq_worker_nr} =
 				keys %{delete($self->{-wq_workers}) // {}};
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 323152b4..77da822b 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -549,9 +549,8 @@ sub start_script {
 	require PublicInbox::OnDestroy;
 	my $tmp_mask = PublicInbox::OnDestroy->new(
 					\&PublicInbox::DS::sig_setmask, $oset);
-	my $pid = fork // die "fork: $!";
+	my $pid = PublicInbox::DS::do_fork();
 	if ($pid == 0) {
-		eval { PublicInbox::DS->Reset };
 		for (@{delete($opt->{-CLOFORK}) // []}) {
 			close($_) or die "close $!";
 		}
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 3426d4a7..41b77dc1 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -385,7 +385,6 @@ sub watch_atfork_child ($) {
 	my ($self) = @_;
 	delete $self->{pids};
 	delete $self->{opendirs};
-	PublicInbox::DS->Reset;
 	my $sig = delete $self->{sig};
 	$sig->{CHLD} = $sig->{HUP} = $sig->{USR1} = 'DEFAULT';
 	# TERM/QUIT/INT call ->quit, which works in both parent+child
@@ -413,11 +412,8 @@ sub imap_idle_reap { # awaitpid callback
 sub imap_idle_fork {
 	my ($self, $uri, $intvl) = @_;
 	return if $self->{quit};
-	my $seed = rand(0xffffffff);
-	my $pid = fork // die "fork: $!";
+	my $pid = PublicInbox::DS::do_fork;
 	if ($pid == 0) {
-		srand($seed);
-		eval { Net::SSLeay::randomize() };
 		watch_atfork_child($self);
 		watch_imap_idle_1($self, $uri, $intvl);
 		_exit(0);
@@ -477,11 +473,8 @@ sub poll_fetch_fork { # DS::add_timer callback
 	my @imap = grep { # push() always returns > 0
 		$_->scheme =~ m!\Aimaps?!i ? 1 : (push(@nntp, $_) < 0)
 	} @$uris;
-	my $seed = rand(0xffffffff);
-	my $pid = fork // die "fork: $!";
+	my $pid = PublicInbox::DS::do_fork;
 	if ($pid == 0) {
-		srand($seed);
-		eval { Net::SSLeay::randomize() };
 		watch_atfork_child($self);
 		watch_imap_fetch_all($self, \@imap) if @imap;
 		watch_nntp_fetch_all($self, \@nntp) if @nntp;
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 4e055acf..c2b66e69 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -11,6 +11,7 @@ use PublicInbox::SearchIdx;
 use File::Temp 0.19 (); # ->newdir
 use File::Path qw(remove_tree);
 use POSIX qw(WNOHANG _exit);
+use PublicInbox::DS;
 
 # support testing with dev versions of Xapian which installs
 # commands with a version number suffix (e.g. "xapian-compact-1.5")
@@ -102,10 +103,8 @@ sub commit_changes ($$$$) {
 
 sub cb_spawn {
 	my ($cb, $args, $opt) = @_; # $cb = cpdb() or compact()
-	my $seed = rand(0xffffffff);
-	my $pid = fork // die "fork: $!";
+	my $pid = PublicInbox::DS::do_fork;
 	return $pid if $pid > 0;
-	srand($seed);
 	$SIG{__DIE__} = sub { warn @_; _exit(1) }; # don't jump up stack
 	$cb->($args, $opt);
 	_exit(0);

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 15/30] ds: get rid of SetLoopTimeout
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (13 preceding siblings ...)
  2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

It's not worth the code and memory to have a setter method we
never use outside of tests.
---
 lib/PublicInbox/DS.pm | 24 +++++++-----------------
 t/dir_idle.t          |  2 +-
 t/ds-leak.t           |  4 ++--
 xt/mem-imapd-tls.t    |  8 ++++----
 xt/mem-nntpd-tls.t    |  8 ++++----
 5 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 9960937d..6041c6b5 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -47,7 +47,7 @@ our (%AWAIT_PIDS, # pid => [ $callback, @args ]
 
      @post_loop_do,              # subref + args to call at the end of each loop
 
-     $LoopTimeout,               # timeout of event loop in milliseconds
+     $loop_timeout,               # timeout of event loop in milliseconds
      @Timers,                    # timers
      %UniqTimer,
      $in_loop,
@@ -89,20 +89,10 @@ sub Reset {
 		scalar(@{$cur_runq // []})); # do not vivify cur_runq
 
 	$reap_armed = undef;
-	$LoopTimeout = -1;  # no timeout by default
+	$loop_timeout = -1;  # no timeout by default
 	$Poller = PublicInbox::Select->new;
 }
 
-=head2 C<< CLASS->SetLoopTimeout( $timeout ) >>
-
-Set the loop timeout for the event loop to some value in milliseconds.
-
-A timeout of 0 (zero) means poll forever. A timeout of -1 means poll and return
-immediately.
-
-=cut
-sub SetLoopTimeout { $LoopTimeout = $_[1] + 0 }
-
 sub _add_named_timer {
 	my ($name, $secs, $coderef, @args) = @_;
 	my $fire_time = now() + $secs;
@@ -163,7 +153,7 @@ sub next_tick () {
 sub RunTimers {
 	next_tick();
 
-	return (($nextq || $ToClose) ? 0 : $LoopTimeout) unless @Timers;
+	return (($nextq || $ToClose) ? 0 : $loop_timeout) unless @Timers;
 
 	my $now = now();
 
@@ -177,16 +167,16 @@ sub RunTimers {
 	# timers may enqueue into nextq:
 	return 0 if ($nextq || $ToClose);
 
-	return $LoopTimeout unless @Timers;
+	return $loop_timeout unless @Timers;
 
 	# convert time to an even number of milliseconds, adding 1
 	# extra, otherwise floating point fun can occur and we'll
 	# call RunTimers like 20-30 times, each returning a timeout
 	# of 0.0000212 seconds
-	my $timeout = int(($Timers[0][0] - $now) * 1000) + 1;
+	my $t = int(($Timers[0][0] - $now) * 1000) + 1;
 
 	# -1 is an infinite timeout, so prefer a real timeout
-	($LoopTimeout < 0 || $LoopTimeout >= $timeout) ? $timeout : $LoopTimeout
+	($loop_timeout < 0 || $loop_timeout >= $t) ? $t : $loop_timeout
 }
 
 sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
@@ -305,7 +295,7 @@ sub event_loop (;$$) {
 		sig_setmask($oldset) if $oldset;
 		sigprocmask(SIG_UNBLOCK, unblockset($sig)) or
 			die "SIG_UNBLOCK: $!";
-		PublicInbox::DS->SetLoopTimeout(1000);
+		$loop_timeout = 1000;
 	}
 	$_[0] = $sigfd = $sig = undef; # $_[0] == sig
 	local $in_loop = 1;
diff --git a/t/dir_idle.t b/t/dir_idle.t
index 02759b54..35c800f9 100644
--- a/t/dir_idle.t
+++ b/t/dir_idle.t
@@ -11,7 +11,7 @@ my @x;
 my $cb = sub { push @x, \@_ };
 my $di = PublicInbox::DirIdle->new($cb);
 $di->add_watches(["$tmpdir/a", "$tmpdir/c"], 1);
-PublicInbox::DS->SetLoopTimeout(1000);
+$PublicInbox::DS::loop_timeout = 1000;
 my $end = 3 + now;
 local @PublicInbox::DS::post_loop_do = (sub { scalar(@x) == 0 && now < $end });
 rmdir("$tmpdir/a/b") or xbail "rmdir $!";
diff --git a/t/ds-leak.t b/t/ds-leak.t
index eaca05b8..179997eb 100644
--- a/t/ds-leak.t
+++ b/t/ds-leak.t
@@ -11,7 +11,7 @@ if ('close-on-exec for epoll and kqueue') {
 	my $pid;
 	my $evfd_re = qr/(?:kqueue|eventpoll)/i;
 
-	PublicInbox::DS->SetLoopTimeout(0);
+	$PublicInbox::DS::loop_timeout = 0;
 	local @PublicInbox::DS::post_loop_do = (sub { 0 });
 
 	# make sure execve closes if we're using fork()
@@ -54,7 +54,7 @@ SKIP: {
 	}
 	my $cb = sub {};
 	for my $i (0..$n) {
-		PublicInbox::DS->SetLoopTimeout(0);
+		$PublicInbox::DS::loop_timeout = 0;
 		local @PublicInbox::DS::post_loop_do = ($cb);
 		PublicInbox::DS::event_loop();
 		PublicInbox::DS->Reset;
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index 00199a9b..58581017 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -81,7 +81,7 @@ sub once { 0 }; # stops event loop
 
 # setup the event loop so that it exits at every step
 # while we're still doing connect(2)
-PublicInbox::DS->SetLoopTimeout(0);
+$PublicInbox::DS::loop_timeout = 0;
 local @PublicInbox::DS::post_loop_do = (\&once);
 my $pid = $td->{pid};
 if ($^O eq 'linux' && open(my $f, '<', "/proc/$pid/status")) {
@@ -100,21 +100,21 @@ foreach my $n (1..$nfd) {
 	# try not to overflow the listen() backlog:
 	if (!($n % 128) && $DONE != $n) {
 		diag("nr: ($n) $DONE/$nfd");
-		PublicInbox::DS->SetLoopTimeout(-1);
+		$PublicInbox::DS::loop_timeout = -1;
 		local @PublicInbox::DS::post_loop_do = (sub { $DONE != $n });
 
 		# clear the backlog:
 		PublicInbox::DS::event_loop();
 
 		# resume looping
-		PublicInbox::DS->SetLoopTimeout(0);
+		$PublicInbox::DS::loop_timeout = 0;
 	}
 }
 
 # run the event loop normally, now:
 diag "done?: @".time." $DONE/$nfd";
 if ($DONE != $nfd) {
-	PublicInbox::DS->SetLoopTimeout(-1);
+	$PublicInbox::DS::loop_timeout = -1;
 	local @PublicInbox::DS::post_loop_do = (sub { $DONE != $nfd });
 	PublicInbox::DS::event_loop();
 }
diff --git a/xt/mem-nntpd-tls.t b/xt/mem-nntpd-tls.t
index f9b98a6b..ec639a8b 100644
--- a/xt/mem-nntpd-tls.t
+++ b/xt/mem-nntpd-tls.t
@@ -104,7 +104,7 @@ sub once { 0 }; # stops event loop
 
 # setup the event loop so that it exits at every step
 # while we're still doing connect(2)
-PublicInbox::DS->SetLoopTimeout(0);
+$PublicInbox::DS::loop_timeout = 0;
 local @PublicInbox::DS::post_loop_do = (\&once);
 
 foreach my $n (1..$nfd) {
@@ -119,14 +119,14 @@ foreach my $n (1..$nfd) {
 	# try not to overflow the listen() backlog:
 	if (!($n % 128) && $n != $DONE) {
 		diag("nr: ($n) $DONE/$nfd");
-		PublicInbox::DS->SetLoopTimeout(-1);
+		$PublicInbox::DS::loop_timeout = -1;
 		@PublicInbox::DS::post_loop_do = (sub { $DONE != $n });
 
 		# clear the backlog:
 		PublicInbox::DS::event_loop();
 
 		# resume looping
-		PublicInbox::DS->SetLoopTimeout(0);
+		$PublicInbox::DS::loop_timeout = 0;
 		@PublicInbox::DS::post_loop_do = (\&once);
 	}
 }
@@ -140,7 +140,7 @@ $dump_rss->();
 
 # run the event loop normally, now:
 if ($DONE != $nfd) {
-	PublicInbox::DS->SetLoopTimeout(-1);
+	$PublicInbox::DS::loop_timeout = -1;
 	@PublicInbox::DS::post_loop_do = (sub {
 		diag "done: ".time." $DONE";
 		$DONE != $nfd;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 16/30] cindex: drop some unused functions
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (14 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

They're no longer needed with the way PublicInbox::CidxLogP is
currently implemented.
---
 lib/PublicInbox/CodeSearchIdx.pm | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index feb37be8..36d00aea 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -1222,9 +1222,4 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap
 	PublicInbox::DS::enqueue_reap() if !shards_active(); # once more for PLC
 }
 
-sub do_quit { $DO_QUIT }
-
-sub tmpdir { $TMPDIR }
-
-
 1;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (15 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

We use this in various places to minimize or maximize pipe
size on Linux.  So keep it all in one place.
---
 lib/PublicInbox/CidxLogP.pm       |  4 ++--
 lib/PublicInbox/EOFpipe.pm        |  6 +++---
 lib/PublicInbox/LeiXSearch.pm     |  2 +-
 lib/PublicInbox/SearchIdxShard.pm | 14 +++++++-------
 lib/PublicInbox/Syscall.pm        | 16 ++++++++--------
 t/gcf2.t                          |  5 +++--
 t/lei-sigpipe.t                   |  7 +++----
 7 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/lib/PublicInbox/CidxLogP.pm b/lib/PublicInbox/CidxLogP.pm
index 7877d5ac..34f7201d 100644
--- a/lib/PublicInbox/CidxLogP.pm
+++ b/lib/PublicInbox/CidxLogP.pm
@@ -10,12 +10,12 @@
 package PublicInbox::CidxLogP;
 use v5.12;
 use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT $F_SETPIPE_SZ);
 
 sub new {
 	my ($cls, $rd, $cidx, $git, $roots) = @_;
 	my $self = bless { cidx => $cidx, git => $git, roots => $roots }, $cls;
-	fcntl($rd, 1031, 1048576) if $^O eq 'linux'; # fatter pipes
+	fcntl($rd, $F_SETPIPE_SZ, 1048576) if $F_SETPIPE_SZ;
 	$self->SUPER::new($rd, EPOLLIN|EPOLLONESHOT);
 }
 
diff --git a/lib/PublicInbox/EOFpipe.pm b/lib/PublicInbox/EOFpipe.pm
index 628e9366..3474874f 100644
--- a/lib/PublicInbox/EOFpipe.pm
+++ b/lib/PublicInbox/EOFpipe.pm
@@ -4,13 +4,13 @@
 package PublicInbox::EOFpipe;
 use v5.12;
 use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT $F_SETPIPE_SZ);
 
 sub new {
 	my (undef, $rd, $cb) = @_;
 	my $self = bless { cb => $cb }, __PACKAGE__;
-	# 1031: F_SETPIPE_SZ, 4096: page size
-	fcntl($rd, 1031, 4096) if $^O eq 'linux';
+	# 4096: page size
+	fcntl($rd, $F_SETPIPE_SZ, 4096) if $F_SETPIPE_SZ;
 	$self->SUPER::new($rd, EPOLLIN|EPOLLONESHOT);
 }
 
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index d83a403c..25b66b3b 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -21,6 +21,7 @@ use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
 use PublicInbox::ContentHash qw(git_sha);
 use POSIX qw(strftime);
 use autodie qw(open read seek truncate);
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
 
 sub new {
 	my ($class) = @_;
@@ -536,7 +537,6 @@ sub do_query {
 		if ($lei->{opt}->{augment} && delete $lei->{early_mua}) {
 			$lei->start_mua;
 		}
-		my $F_SETPIPE_SZ = $^O eq 'linux' ? 1031 : undef;
 		if ($l2m->{-wq_nr_workers} > 1 &&
 				$l2m->{base_type} =~ /\A(?:maildir|mbox)\z/) {
 			# setup two barriers to coordinate ->has_entries
diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm
index 21bd56c2..1630eb4a 100644
--- a/lib/PublicInbox/SearchIdxShard.pm
+++ b/lib/PublicInbox/SearchIdxShard.pm
@@ -7,6 +7,7 @@ package PublicInbox::SearchIdxShard;
 use v5.12;
 use parent qw(PublicInbox::SearchIdx PublicInbox::IPC);
 use PublicInbox::OnDestroy;
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
 
 sub new {
 	my ($class, $v2w, $shard) = @_; # v2w may be ExtSearchIdx
@@ -20,13 +21,12 @@ sub new {
 	if ($v2w->{parallel}) {
 		local $self->{-v2w_afc} = $v2w;
 		$self->ipc_worker_spawn("shard[$shard]");
-		# F_SETPIPE_SZ = 1031 on Linux; increasing the pipe size for
-		# inputs speeds V2Writable batch imports across 8 cores by
-		# nearly 20%.  Since any of our responses are small, make
-		# the response pipe as small as possible
-		if ($^O eq 'linux') {
-			fcntl($self->{-ipc_req}, 1031, 1048576);
-			fcntl($self->{-ipc_res}, 1031, 4096);
+		# Increasing the pipe size for requests speeds V2 batch imports
+		# across 8 cores by nearly 20%.  Since many of our responses
+		# are small, make the response pipe as small as possible
+		if ($F_SETPIPE_SZ) {
+			fcntl($self->{-ipc_req}, $F_SETPIPE_SZ, 1048576);
+			fcntl($self->{-ipc_res}, $F_SETPIPE_SZ, 4096);
 		}
 	}
 	$self;
diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index e83beb6a..78181bb6 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -28,7 +28,7 @@ our @EXPORT_OK = qw(epoll_ctl epoll_create epoll_wait
                   EPOLLIN EPOLLOUT EPOLLET
                   EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
                   EPOLLONESHOT EPOLLEXCLUSIVE
-                  signalfd rename_noreplace %SIGNUM);
+                  signalfd rename_noreplace %SIGNUM $F_SETPIPE_SZ);
 use constant {
 	EPOLLIN => 1,
 	EPOLLOUT => 4,
@@ -55,13 +55,12 @@ use constant {
 
 my @BYTES_4_hole = BYTES_4_hole ? (0) : ();
 
-our (
-     $SYS_epoll_create,
-     $SYS_epoll_ctl,
-     $SYS_epoll_wait,
-     $SYS_signalfd4,
-     $SYS_renameat2,
-     );
+our ($SYS_epoll_create,
+	$SYS_epoll_ctl,
+	$SYS_epoll_wait,
+	$SYS_signalfd4,
+	$SYS_renameat2,
+	$F_SETPIPE_SZ);
 
 my ($SYS_sendmsg, $SYS_recvmsg);
 my $SYS_fstatfs; # don't need fstatfs64, just statfs.f_type
@@ -70,6 +69,7 @@ my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC
 our $no_deprecated = 0;
 
 if ($^O eq "linux") {
+	$F_SETPIPE_SZ = 1031;
     my (undef, undef, $release, undef, $machine) = POSIX::uname();
     my ($maj, $min) = ($release =~ /\A([0-9]+)\.([0-9]+)/);
     $SYS_renameat2 = 0 if "$maj.$min" < 3.15;
diff --git a/t/gcf2.t b/t/gcf2.t
index d12a4420..33f3bbca 100644
--- a/t/gcf2.t
+++ b/t/gcf2.t
@@ -10,6 +10,7 @@ use POSIX qw(_exit);
 use Cwd qw(abs_path);
 require_mods('PublicInbox::Gcf2');
 use_ok 'PublicInbox::Gcf2';
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
 use PublicInbox::Import;
 my ($tmpdir, $for_destroy) = tmpdir();
 
@@ -109,7 +110,7 @@ SKIP: {
 	for my $blk (1, 0) {
 		my ($r, $w);
 		pipe($r, $w) or BAIL_OUT $!;
-		fcntl($w, 1031, 4096) or
+		fcntl($w, $F_SETPIPE_SZ, 4096) or
 			skip('Linux too old for F_SETPIPE_SZ', 14);
 		$w->blocking($blk);
 		seek($fh, 0, SEEK_SET) or BAIL_OUT "seek: $!";
@@ -129,7 +130,7 @@ SKIP: {
 		$ck_copying->("pipe blocking($blk)");
 
 		pipe($r, $w) or BAIL_OUT $!;
-		fcntl($w, 1031, 4096) or BAIL_OUT $!;
+		fcntl($w, $F_SETPIPE_SZ, 4096) or BAIL_OUT $!;
 		$w->blocking($blk);
 		close $r;
 		local $SIG{PIPE} = 'IGNORE';
diff --git a/t/lei-sigpipe.t b/t/lei-sigpipe.t
index 55c208e2..622598a4 100644
--- a/t/lei-sigpipe.t
+++ b/t/lei-sigpipe.t
@@ -6,6 +6,7 @@ use v5.10.1;
 use PublicInbox::TestCommon;
 use POSIX qw(WTERMSIG WIFSIGNALED SIGPIPE);
 use PublicInbox::OnDestroy;
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
 
 # undo systemd (and similar) ignoring SIGPIPE, since lei expects to be run
 # from an interactive terminal:
@@ -21,10 +22,8 @@ test_lei(sub {
 	my $imported;
 	for my $out ([], [qw(-f mboxcl2)], [qw(-f text)]) {
 		pipe(my ($r, $w)) or BAIL_OUT $!;
-		my $size = 65536;
-		if ($^O eq 'linux' && fcntl($w, 1031, 4096)) {
-			$size = 4096;
-		}
+		my $size = $F_SETPIPE_SZ && fcntl($w, $F_SETPIPE_SZ, 4096) ?
+			4096 : 65536;
 		unless (-f $f) {
 			open my $fh, '>', $f or xbail "open $f: $!";
 			print $fh <<'EOM' or xbail;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 18/30] t/lei-up: additional diagnostics for match failures
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (16 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
                   ` (12 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

I'm not sure why, but this test just failed for some odd reason
from `make check-run' on my Debian bullseye workstatation.
---
 t/lei-up.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/lei-up.t b/t/lei-up.t
index baed6507..2d3afd82 100644
--- a/t/lei-up.t
+++ b/t/lei-up.t
@@ -18,11 +18,11 @@ test_lei(sub {
 		gunzip("$home/$x.mbox.gz" => \$uc, MultiStream => 1) or
 				xbail "gunzip $GunzipError";
 		ok(index($uc, $qp->body_raw) >= 0,
-			"original mail in $x.mbox.gz");
+			"original mail in $x.mbox.gz") or diag $uc;
 		open my $fh, '<', "$home/$x" or xbail $!;
 		$uc = do { local $/; <$fh> } // xbail $!;
 		ok(index($uc, $qp->body_raw) >= 0,
-			"original mail in uncompressed $x");
+			"original mail in uncompressed $x") or diag $uc;
 	}
 	lei_ok qw(ls-search);
 	$s = eml_load('t/utf8.eml')->as_string;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 19/30] test_common: use autodie and read_all where possible
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (17 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

Noise reduction to help my aging eyes.
---
 lib/PublicInbox/TestCommon.pm | 73 ++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 39 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 77da822b..3839ab45 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -16,6 +16,7 @@ our @EXPORT;
 my $lei_loud = $ENV{TEST_LEI_ERR_LOUD};
 my $tail_cmd = $ENV{TAIL};
 our ($lei_opt, $lei_out, $lei_err, $lei_cwdfh);
+use autodie qw(chdir close fcntl open opendir seek unlink);
 
 $_ = File::Spec->rel2abs($_) for (grep(!m!^/!, @INC));
 
@@ -46,11 +47,16 @@ sub require_bsd (;$) {
 
 sub xbail (@) { BAIL_OUT join(' ', map { ref() ? (explain($_)) : ($_) } @_) }
 
+sub read_all ($;$$) {
+	require PublicInbox::Git;
+	PublicInbox::Git::read_all($_[0], $_[1], $_[2])
+}
+
 sub eml_load ($) {
 	my ($path, $cb) = @_;
-	open(my $fh, '<', $path) or die "open $path: $!";
+	open(my $fh, '<', $path);
 	require PublicInbox::Eml;
-	PublicInbox::Eml->new(\(do { local $/; <$fh> }));
+	PublicInbox::Eml->new(\(read_all($fh)));
 }
 
 sub tmpdir (;$) {
@@ -243,9 +249,9 @@ sub _prepare_redirects ($) {
 	for (my $fd = 0; $fd <= $#io_mode; $fd++) {
 		my $fh = $fhref->[$fd] or next;
 		my ($oldfh, $mode) = @{$io_mode[$fd]};
-		open my $orig, $mode, $oldfh or die "$oldfh $mode stash: $!";
+		open(my $orig, $mode, $oldfh);
 		$orig_io->[$fd] = $orig;
-		open $oldfh, $mode, $fh or die "$oldfh $mode redirect: $!";
+		open $oldfh, $mode, $fh;
 	}
 	$orig_io;
 }
@@ -255,7 +261,7 @@ sub _undo_redirects ($) {
 	for (my $fd = 0; $fd <= $#io_mode; $fd++) {
 		my $fh = $orig_io->[$fd] or next;
 		my ($oldfh, $mode) = @{$io_mode[$fd]};
-		open $oldfh, $mode, $fh or die "$$oldfh $mode redirect: $!";
+		open $oldfh, $mode, $fh;
 	}
 }
 
@@ -281,8 +287,8 @@ sub key2sub ($) {
 	my ($key) = @_;
 	$cached_scripts{$key} //= do {
 		my $f = key2script($key);
-		open my $fh, '<', $f or die "open $f: $!";
-		my $str = do { local $/; <$fh> };
+		open my $fh, '<', $f;
+		my $str = read_all($fh);
 		my $pkg = (split(m!/!, $f))[-1];
 		$pkg =~ s/([a-z])([a-z0-9]+)(\.t)?\z/\U$1\E$2/;
 		$pkg .= "_T" if $3;
@@ -329,7 +335,7 @@ sub _run_sub ($$$) {
 sub no_coredump (@) {
 	my @dirs = @_;
 	my $cwdfh;
-	if (@dirs) { opendir($cwdfh, '.') or die "opendir(.): $!" }
+	opendir($cwdfh, '.') if @dirs;
 	my @found;
 	for (@dirs, '.') {
 		chdir $_;
@@ -375,7 +381,7 @@ sub run_script ($;$$) {
 			next if $fd > 0;
 			$fh->autoflush(1);
 			print $fh $$redir or die "print: $!";
-			seek($fh, 0, SEEK_SET) or die "seek: $!";
+			seek($fh, 0, SEEK_SET);
 		} elsif ($ref eq 'GLOB') {
 			$spawn_opt->{$fd} = $fhref->[$fd] = $redir;
 		} elsif ($ref) {
@@ -406,15 +412,13 @@ sub run_script ($;$$) {
 		my $orig_io = _prepare_redirects($fhref);
 		my $cwdfh = $lei_cwdfh;
 		if (my $d = $opt->{'-C'}) {
-			unless ($cwdfh) {
-				opendir $cwdfh, '.' or die "opendir .: $!";
-			}
-			chdir $d or die "chdir $d: $!";
+			$cwdfh or opendir $cwdfh, '.';
+			chdir $d;
 		}
 		_run_sub($sub, $key, \@argv);
 		# n.b. all our uses of PublicInbox::DS should be fine
 		# with this and we can't Reset here.
-		die "fchdir(restore): $!" if $cwdfh && !chdir($cwdfh);
+		chdir($cwdfh) if $cwdfh;
 		_undo_redirects($orig_io);
 		select STDOUT;
 		umask($umask);
@@ -425,10 +429,8 @@ sub run_script ($;$$) {
 	for my $fd (1..2) {
 		my $fh = $fhref->[$fd] or next;
 		next unless -f $fh;
-		seek($fh, 0, SEEK_SET) or die "seek: $!";
-		my $redir = $opt->{$fd};
-		local $/;
-		$$redir = <$fh>;
+		seek($fh, 0, SEEK_SET);
+		${$opt->{$fd}} = read_all($fh);
 	}
 	no_coredump($opt->{-C} ? ($opt->{-C}) : ());
 	$? == 0;
@@ -458,7 +460,7 @@ sub wait_for_tail {
 		$ino[0] =~ s!/fd/!/fdinfo/!;
 		my @info;
 		do {
-			if (open my $fh, '<', $ino[0]) {
+			if (CORE::open(my $fh, '<', $ino[0])) {
 				local $/ = "\n";
 				@info = grep(/^inotify wd:/, <$fh>);
 			}
@@ -500,7 +502,6 @@ sub tail_f (@) {
 	$tail_cmd or return; # "tail -F" or "tail -f"
 	my $opt = (ref($f[-1]) eq 'HASH') ? pop(@f) : {};
 	my $clofork = $opt->{-CLOFORK} // [];
-	use autodie qw(fcntl open);
 	my @cfmap = map {
 		my $fl = fcntl($_, F_GETFD, 0);
 		fcntl($_, F_SETFD, $fl | FD_CLOEXEC) unless $fl & FD_CLOEXEC;
@@ -551,9 +552,7 @@ sub start_script {
 					\&PublicInbox::DS::sig_setmask, $oset);
 	my $pid = PublicInbox::DS::do_fork();
 	if ($pid == 0) {
-		for (@{delete($opt->{-CLOFORK}) // []}) {
-			close($_) or die "close $!";
-		}
+		close($_) for (@{delete($opt->{-CLOFORK}) // []});
 		# pretend to be systemd (cf. sd_listen_fds(3))
 		# 3 == SD_LISTEN_FDS_START
 		my $fd;
@@ -561,7 +560,7 @@ sub start_script {
 			my $io = $opt->{$fd} // next;
 			my $old = fileno($io);
 			if ($old == $fd) {
-				fcntl($io, F_SETFD, 0) // die "F_SETFD: $!";
+				fcntl($io, F_SETFD, 0);
 			} else {
 				dup2($old, $fd) // die "dup2($old, $fd): $!";
 			}
@@ -572,7 +571,7 @@ sub start_script {
 			$ENV{LISTEN_PID} = $$;
 			$ENV{LISTEN_FDS} = $fds;
 		}
-		if ($opt->{-C}) { chdir($opt->{-C}) or die "chdir: $!" }
+		if ($opt->{-C}) { chdir($opt->{-C}) }
 		$0 = join(' ', @$cmd);
 		local @SIG{keys %SIG} = map { undef } values %SIG;
 		local $SIG{FPE} = 'IGNORE'; # Perl default
@@ -657,7 +656,6 @@ sub need_scm_rights () {
 
 # returns a pipe with FD_CLOEXEC disabled on the write-end
 sub quit_waiter_pipe () {
-	use autodie qw(fcntl pipe);
 	pipe(my $r, my $w);
 	fcntl($w, F_SETFD, fcntl($w, F_GETFD, 0) & ~FD_CLOEXEC);
 	($r, $w);
@@ -675,7 +673,7 @@ SKIP: {
 	my ($cb) = pop @_;
 	my $test_opt = shift // {};
 	local $lei_cwdfh;
-	use autodie qw(mkdir open opendir);
+	use autodie qw(mkdir);
 	opendir $lei_cwdfh, '.';
 	require_git(2.6, 1);
 	my $mods = $test_opt->{mods} // [ 'lei' ];
@@ -766,7 +764,7 @@ sub setup_public_inboxes () {
 				'--newsgroup', "t.v$V", "t$V",
 				"$test_home/t$V", "http://example.com/t$V",
 				"t$V\@example.com" ]) or xbail "init v$V";
-		unlink "$test_home/t$V/description" or xbail "unlink $!";
+		unlink "$test_home/t$V/description";
 	}
 	require PublicInbox::Config;
 	require PublicInbox::InboxWritable;
@@ -786,7 +784,7 @@ sub setup_public_inboxes () {
 		$im->done;
 	});
 	$seen or BAIL_OUT 'no imports';
-	open my $fh, '>', $stamp or BAIL_OUT "open $stamp: $!";
+	open my $fh, '>', $stamp;
 	@ret;
 }
 
@@ -815,13 +813,12 @@ sub create_coderepo ($$;@) {
 	my $scope = $lk->lock_for_scope;
 	my $tmpdir = delete $opt{tmpdir};
 	if (!-f "$dir/creat.stamp") {
-		opendir(my $dfh, '.') or xbail "opendir .: $!";
-		chdir($dir) or xbail "chdir($dir): $!";
+		opendir(my $dfh, '.');
+		chdir($dir);
 		local %ENV = (%ENV, %COMMIT_ENV);
 		$cb->($dir);
-		chdir($dfh) or xbail "cd -: $!";
-		open my $s, '>', "$dir/creat.stamp" or
-			BAIL_OUT "error creating $dir/creat.stamp: $!";
+		chdir($dfh);
+		open my $s, '>', "$dir/creat.stamp";
 	}
 	return $dir if !defined($tmpdir);
 	xsys_e([qw(/bin/cp -Rp), $dir, $tmpdir]);
@@ -868,8 +865,7 @@ sub create_inbox ($$;@) {
 				xsys_e([ qw(git gc -q) ], { GIT_DIR => $dir });
 			}
 		}
-		open my $s, '>', "$dir/creat.stamp" or
-			BAIL_OUT "error creating $dir/creat.stamp: $!";
+		open my $s, '>', "$dir/creat.stamp";
 	}
 	if ($tmpdir) {
 		undef $ibx;
@@ -904,8 +900,8 @@ sub test_httpd ($$;$$) {
 							ua => $ua);
 		$cb->() if $cb;
 		$td->join('TERM');
-		open my $fh, '<', $err or BAIL_OUT $!;
-		my $e = do { local $/; <$fh> };
+		open my $fh, '<', $err;
+		my $e = read_all($fh);
 		if ($e =~ s/^Plack::Middleware::ReverseProxy missing,\n//gms) {
 			$e =~ s/^URL generation for redirects .*\n//gms;
 		}
@@ -934,7 +930,6 @@ sub no_pollerfd ($) {
 
 sub cfg_new ($;@) {
 	my ($tmpdir, @body) = @_;
-	use autodie;
 	require PublicInbox::Config;
 	my $f = "$tmpdir/tmp_cfg";
 	open my $fh, '>', $f;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 20/30] test_common: only hide TCP port in messages
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (18 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
                   ` (10 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

v2:// lei outputs are on the filesystem, so putting $HOST:$PORT
is nonsensical.  We'll also keep `127.0.0.1' or `[::1]' since
it's harmless and can point out obvious errors in system
configuration when testing with old Perls or libraries.
---
 lib/PublicInbox/TestCommon.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 3839ab45..96663731 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -619,7 +619,7 @@ sub lei_ok (@) {
 	my @msg = ref($_[0]) eq 'ARRAY' ? @{$_[0]} : @_;
 	if (!$lei_loud) {
 		for (@msg) {
-			s!\A([a-z0-9]+://)[^/]+/!$1\$HOST_PORT/!;
+			s!(127\.0\.0\.1|\[::1\]):(?:\d+)!$1:\$PORT!g;
 			s!$tmpdir\b/(?:[^/]+/)?!\$TMPDIR/!g;
 			s!\Q$PWD\E\b!\$PWD!g;
 		}

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 21/30] test_common: use $cwdfh for every run_script command
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (19 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

It's not lei-specific since we have `-C' to perform chdir
for multiple admin commands.
---
 lib/PublicInbox/TestCommon.pm | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 96663731..5ad12942 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -15,7 +15,7 @@ use Carp ();
 our @EXPORT;
 my $lei_loud = $ENV{TEST_LEI_ERR_LOUD};
 my $tail_cmd = $ENV{TAIL};
-our ($lei_opt, $lei_out, $lei_err, $lei_cwdfh);
+our ($lei_opt, $lei_out, $lei_err);
 use autodie qw(chdir close fcntl open opendir seek unlink);
 
 $_ = File::Spec->rel2abs($_) for (grep(!m!^/!, @INC));
@@ -410,15 +410,12 @@ sub run_script ($;$$) {
 		local $SIG{FPE} = 'IGNORE'; # Perl default
 		local $0 = join(' ', @$cmd);
 		my $orig_io = _prepare_redirects($fhref);
-		my $cwdfh = $lei_cwdfh;
-		if (my $d = $opt->{'-C'}) {
-			$cwdfh or opendir $cwdfh, '.';
-			chdir $d;
-		}
+		opendir(my $cwdfh, '.');
+		chdir $opt->{-C} if defined $opt->{-C};
 		_run_sub($sub, $key, \@argv);
 		# n.b. all our uses of PublicInbox::DS should be fine
 		# with this and we can't Reset here.
-		chdir($cwdfh) if $cwdfh;
+		chdir($cwdfh);
 		_undo_redirects($orig_io);
 		select STDOUT;
 		umask($umask);
@@ -672,9 +669,7 @@ sub test_lei {
 SKIP: {
 	my ($cb) = pop @_;
 	my $test_opt = shift // {};
-	local $lei_cwdfh;
 	use autodie qw(mkdir);
-	opendir $lei_cwdfh, '.';
 	require_git(2.6, 1);
 	my $mods = $test_opt->{mods} // [ 'lei' ];
 	require_mods(@$mods, 2);

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 22/30] init: drop extraneous `+'
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (20 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
                   ` (8 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

It's actually valid Perl syntax, but still confusing to look at.

Fixes: add90b9504f4 ("support -C (chdir) for most non-daemon commands")
---
 script/public-inbox-init | 2 +-
 t/init.t                 | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/script/public-inbox-init b/script/public-inbox-init
index 33bee310..b3e94faf 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -60,7 +60,7 @@ my $inboxdir = shift @ARGV or $usage_cb->();
 my $http_url = shift @ARGV or $usage_cb->();
 my (@address) = @ARGV;
 @address or $usage_cb->();
-+PublicInbox::Admin::do_chdir(\@chdir);
+PublicInbox::Admin::do_chdir(\@chdir);
 
 @c_extra = map {
 	my ($k, $v) = split(/=/, $_, 2);
diff --git a/t/init.t b/t/init.t
index abe3a372..177c22b6 100644
--- a/t/init.t
+++ b/t/init.t
@@ -216,6 +216,13 @@ SKIP: {
 	is($n, 13, 'V1 NNTP article numbers skipped via --skip-artnum');
 }
 
+{
+	my $cmd = [ qw(-init -C), "$tmpdir", qw(chdirlist chdirlist),
+		   qw(http://example.com/chdirlist chdirlist@example.com)];
+	ok(run_script($cmd), '-init with -C (chdir)');
+	ok(-d "$tmpdir/chdirlist", '-C processed correctly');
+}
+
 done_testing();
 
 sub read_indexlevel {

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 23/30] init: use autodie to reduce distractions
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (21 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
                   ` (7 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

This hurts startup time a bit, but our tests use run_script
by default and I don't think normal users call -init enough
to care.
---
 script/public-inbox-init | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/script/public-inbox-init b/script/public-inbox-init
index b3e94faf..6420db7e 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -4,6 +4,7 @@
 use strict;
 use v5.10.1;
 use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
+use autodie qw(open chmod close rename);
 use Fcntl qw(:DEFAULT);
 my $help = <<EOF; # the following should fit w/o scrolling in 80x24 term:
 usage: public-inbox-init NAME INBOX_DIR HTTP_URL ADDRESS [ADDRESS..]
@@ -126,15 +127,12 @@ my $perm = 0644 & ~umask;
 my %seen;
 if (-e $pi_config) {
 	require PublicInbox::Git;
-	open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";
-	my @st = stat($oh);
+	open(my $oh, '<', $pi_config);
+	my @st = stat($oh) or die "(f)stat failed on $pi_config: $!\n";
 	$perm = $st[2];
-	defined $perm or die "(f)stat failed on $pi_config: $!\n";
-	chmod($perm & 07777, $fh) or
-		die "(f)chmod failed on future $pi_config: $!\n";
-	my $old = PublicInbox::Git::read_all($oh);
-	print $fh $old or die "failed to write: $!\n";
-	close $oh or die "failed to close $pi_config: $!\n";
+	chmod($perm & 07777, $fh);
+	print $fh PublicInbox::Git::read_all($oh);
+	close $oh;
 
 	# yes, this conflict checking is racy if multiple instances of this
 	# script are run by the same $PI_DIR
@@ -161,7 +159,7 @@ if (-e $pi_config) {
 	$indexlevel //= $ibx->{indexlevel} if $ibx;
 }
 my $pi_config_tmp = $fh->filename;
-close($fh) or die "failed to close $pi_config_tmp: $!\n";
+close($fh);
 
 my $pfx = "publicinbox.$name";
 my @x = (qw/git config/, "--file=$pi_config_tmp");
@@ -216,8 +214,8 @@ $ibx->init_inbox(0, $skip_epoch, $skip_artnum);
 
 my $f = "$inboxdir/description";
 if (sysopen $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
-	print $fh "public inbox for $address[0]\n" or die "print($f): $!";
-	close $fh or die "close($f): $!";
+	print $fh "public inbox for $address[0]\n";
+	close $fh;
 }
 
 # needed for git prior to v2.1.0
@@ -248,9 +246,6 @@ for my $kv (@c_extra) {
 }
 
 # needed for git prior to v2.1.0
-chmod($perm & 07777, $pi_config_tmp) or
-	die "(f)chmod failed on future $pi_config: $!\n";
-
-rename $pi_config_tmp, $pi_config or
-	die "failed to rename `$pi_config_tmp' to `$pi_config': $!\n";
+chmod($perm & 07777, $pi_config_tmp);
+rename $pi_config_tmp, $pi_config;
 undef $auto_unlink; # trigger ->DESTROY

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (22 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
                   ` (6 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

The `:epoll' tag has been gone for a few weeks, and EPOLLIN
isn't used in this file anywhere.

Fixes: 3005c1bc5d05 (ds: use object-oriented API for epoll)
---
 xt/mem-imapd-tls.t | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index 58581017..bb2f6c35 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -7,7 +7,6 @@ use strict;
 use v5.10.1;
 use Socket qw(SOCK_STREAM IPPROTO_TCP SOL_SOCKET);
 use PublicInbox::TestCommon;
-use PublicInbox::Syscall qw(:epoll);
 use PublicInbox::DS;
 require_mods(qw(-imapd));
 my $inboxdir = $ENV{GIANT_INBOX_DIR};
@@ -134,7 +133,7 @@ package IMAPC;
 use strict;
 use parent qw(PublicInbox::DS);
 # fields: step: state machine, zin: Zlib inflate context
-use PublicInbox::Syscall qw(EPOLLIN EPOLLOUT EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLOUT EPOLLONESHOT);
 use Errno qw(EAGAIN);
 # determines where we start event_step
 use constant FIRST_STEP => ($ENV{TEST_COMPRESS} // 1) ? -2 : 0;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (23 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

We need some pipes in our parent process to capture the output
of lsof(1), so give us some more padding for temporary FDs.
---
 xt/mem-imapd-tls.t | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index bb2f6c35..53adb11b 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -6,6 +6,7 @@
 use strict;
 use v5.10.1;
 use Socket qw(SOCK_STREAM IPPROTO_TCP SOL_SOCKET);
+use PublicInbox::Spawn qw(which);
 use PublicInbox::TestCommon;
 use PublicInbox::DS;
 require_mods(qw(-imapd));
@@ -71,7 +72,7 @@ if ($TEST_TLS) {
 	$ssl_opt{SSL_startHandshake} = 0;
 }
 chomp(my $nfd = `/bin/sh -c 'ulimit -n'`);
-$nfd -= 10;
+$nfd -= 20;
 ok($nfd > 0, 'positive FD count');
 my $MAX_FD = 10000;
 $nfd = $MAX_FD if $nfd >= $MAX_FD;
@@ -118,10 +119,11 @@ if ($DONE != $nfd) {
 	PublicInbox::DS::event_loop();
 }
 is($nfd, $DONE, "$nfd/$DONE done");
-if ($^O eq 'linux' && open(my $f, '<', "/proc/$pid/status")) {
+my $lsof = which('lsof');
+if ($^O eq 'linux' && $lsof && open(my $f, '<', "/proc/$pid/status")) {
 	diag(grep(/RssAnon/, <$f>));
-	diag "  SELF lsof | wc -l ".`lsof -p $$ |wc -l`;
-	diag "SERVER lsof | wc -l ".`lsof -p $pid |wc -l`;
+	diag "  SELF lsof | wc -l ".`$lsof -p $$ |wc -l`;
+	diag "SERVER lsof | wc -l ".`$lsof -p $pid |wc -l`;
 }
 PublicInbox::DS->Reset;
 $td->kill;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 26/30] lei: use autodie where appropriate
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (24 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

This makes us a bit harsher with misbehaving clients, but we
only have one client implementation at the moment.
---
 lib/PublicInbox/LEI.pm | 48 ++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 1ff6d67f..3ccdd4f7 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,6 +9,7 @@ package PublicInbox::LEI;
 use v5.12;
 use parent qw(PublicInbox::DS PublicInbox::LeiExternal
 	PublicInbox::LeiQuery);
+use autodie qw(bind chdir fork open socket socketpair unlink);
 use Getopt::Long ();
 use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
 use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -29,7 +30,6 @@ use File::Path ();
 use File::Spec;
 use Carp ();
 use Sys::Syslog qw(openlog syslog closelog);
-use Scalar::Util qw(looks_like_number);
 our $quit = \&CORE::exit;
 our ($current_lei, $errors_log, $listener, $oldset, $dir_idle);
 my $GLP = Getopt::Long::Parser->new;
@@ -574,12 +574,12 @@ sub _lei_atfork_child {
 	# we need to explicitly close things which are on stack
 	my $cfg = $self->{cfg};
 	if ($persist) {
-		open $self->{3}, '<', '/' or die "open(/) $!";
+		open $self->{3}, '<', '/';
 		fchdir($self);
 		close($_) for (grep(defined, delete @$self{qw(0 1 2 sock)}));
 		delete @$cfg{qw(-lei_store -watches -lei_note_event)};
 	} else { # worker, Net::NNTP (Net::Cmd) uses STDERR directly
-		open STDERR, '+>&='.fileno($self->{2}) or warn "open $!";
+		open STDERR, '+>&='.fileno($self->{2});
 		STDERR->autoflush(1);
 		POSIX::setpgid(0, $$) // die "setpgid(0, $$): $!";
 		delete @$cfg{qw(-watches -lei_note_event)};
@@ -813,10 +813,9 @@ sub dispatch {
 		if (my $chdir = $self->{opt}->{C}) {
 			for my $d (@$chdir) {
 				next if $d eq ''; # same as git(1)
-				chdir $d or return fail($self, "cd $d: $!");
+				chdir $d;
 			}
-			open $self->{3}, '<', '.' or
-				return fail($self, "open . $!");
+			open($self->{3}, '<', '.');
 		}
 		$cb->($self, @argv);
 	} elsif (grep(/\A-/, $cmd, @argv)) { # --help or -h only
@@ -851,7 +850,7 @@ sub _lei_cfg ($;$) {
 		}
 		my ($cfg_dir) = ($f =~ m!(.*?/)[^/]+\z!);
 		File::Path::mkpath($cfg_dir);
-		open my $fh, '>>', $f or die "open($f): $!\n";
+		open my $fh, '>>', $f;
 		@st = stat($fh) or die "fstat($f): $!\n";
 		$cur_st = pack('dd', $st[10], $st[7]);
 		qerr($self, "# $f created") if $self->{cmd} ne 'config';
@@ -1148,10 +1147,7 @@ sub accept_dispatch { # Listener {post_accept} callback
 		return send($sock, $msg, 0);
 	} else {
 		my $i = 0;
-		for my $fd (@fds) {
-			open($self->{$i++}, '+<&=', $fd) and next;
-			send($sock, "open(+<&=$fd) (FD=$i): $!", 0);
-		}
+		open($self->{$i++}, '+<&=', $_) for @fds;
 		$i == 4 or return send($sock, 'not enough FDs='.($i-1), 0)
 	}
 	# $ENV_STR = join('', map { "\0$_=$ENV{$_}" } keys %ENV);
@@ -1236,12 +1232,11 @@ sub dump_and_clear_log {
 sub cfg2lei ($) {
 	my ($cfg) = @_;
 	my $lei = bless { env => { %{$cfg->{-env}} } }, __PACKAGE__;
-	open($lei->{0}, '<&', \*STDIN) or die "dup 0: $!";
-	open($lei->{1}, '>>&', \*STDOUT) or die "dup 1: $!";
-	open($lei->{2}, '>>&', \*STDERR) or die "dup 2: $!";
-	open($lei->{3}, '<', '/') or die "open /: $!";
-	my ($x, $y);
-	socketpair($x, $y, AF_UNIX, SOCK_SEQPACKET, 0) or die "socketpair: $!";
+	open($lei->{0}, '<&', \*STDIN);
+	open($lei->{1}, '>>&', \*STDOUT);
+	open($lei->{2}, '>>&', \*STDERR);
+	open($lei->{3}, '<', '/');
+	socketpair(my $x, my $y, AF_UNIX, SOCK_SEQPACKET, 0);
 	$lei->{sock} = $x;
 	require PublicInbox::LeiSelfSocket;
 	PublicInbox::LeiSelfSocket->new($y); # adds to event loop
@@ -1317,17 +1312,15 @@ sub lazy_start {
 	my $lk = PublicInbox::Lock->new($errors_log);
 	umask(077) // die("umask(077): $!");
 	$lk->lock_acquire;
-	socket($listener, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!";
+	socket($listener, AF_UNIX, SOCK_SEQPACKET, 0);
 	if ($errno == ECONNREFUSED || $errno == ENOENT) {
 		return if connect($listener, $addr); # another process won
-		if ($errno == ECONNREFUSED && -S $path) {
-			unlink($path) or die "unlink($path): $!";
-		}
+		unlink($path) if $errno == ECONNREFUSED && -S $path;
 	} else {
 		$! = $errno; # allow interpolation to stringify in die
 		die "connect($path): $!";
 	}
-	bind($listener, $addr) or die "bind($path): $!";
+	bind($listener, $addr);
 	$lk->lock_release;
 	undef $lk;
 	my @st = stat($path) or die "stat($path): $!";
@@ -1340,11 +1333,11 @@ sub lazy_start {
 	require PublicInbox::Listener;
 	require PublicInbox::PktOp;
 	(-p STDOUT) or die "E: stdout must be a pipe\n";
-	open(STDIN, '+>>', $errors_log) or die "open($errors_log): $!";
+	open(STDIN, '+>>', $errors_log);
 	STDIN->autoflush(1);
 	dump_and_clear_log();
 	POSIX::setsid() > 0 or die "setsid: $!";
-	my $pid = fork // die "fork: $!";
+	my $pid = fork;
 	return if $pid;
 	$0 = "lei-daemon $path";
 	local %PATH2CFG;
@@ -1385,8 +1378,8 @@ sub lazy_start {
 	};
 	local $SIG{PIPE} = 'IGNORE';
 	local $SIG{ALRM} = 'IGNORE';
-	open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
-	open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";
+	open STDERR, '>&STDIN';
+	open STDOUT, '>&STDIN';
 	# $daemon pipe to `lei' closed, main loop begins:
 	eval { PublicInbox::DS::event_loop($sig, $oldset) };
 	warn "event loop error: $@\n" if $@;
@@ -1424,8 +1417,7 @@ sub wq_done_wait { # awaitpid cb (via wq_eof)
 
 sub fchdir {
 	my ($lei) = @_;
-	my $dh = $lei->{3} // die 'BUG: lei->{3} (CWD) gone';
-	chdir($dh) || die "fchdir: $!";
+	chdir($lei->{3} // die 'BUG: lei->{3} (CWD) gone');
 }
 
 sub wq_eof { # EOF callback for main daemon

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 27/30] lei_auth: update comments and use v5.12
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (25 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

There's nothing affected by the `unicode_strings' feature, so
it's safe to use v5.12, here.
---
 lib/PublicInbox/LeiAuth.pm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 76a4410d..020dd125 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -1,8 +1,8 @@
-# Copyright (C) 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>
 
 # Authentication worker for anything that needs auth for read/write IMAP
-# (eventually for read-only NNTP access)
+# and read-only NNTP access
 #
 # timelines
 # lei-daemon              |  LeiAuth worker #0      | other WQ workers
@@ -22,8 +22,7 @@
 #                         |
 # call net_merge_all_done ->-> do per-WQ-class defined actions
 package PublicInbox::LeiAuth;
-use strict;
-use v5.10.1;
+use v5.12;
 
 sub do_auth_atfork { # used by IPC WQ workers
 	my ($self, $wq) = @_;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 28/30] lei_config: drop redundant open check
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (26 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
                   ` (2 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

autodie already takes care of checking for open failures.
---
 lib/PublicInbox/LeiConfig.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index c47708d8..796bb4f5 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -22,7 +22,7 @@ sub cfg_do_edit ($;$) {
 
 sub cfg_edit_done { # PktOp lei->do_env cb
 	my ($lei, $self) = @_;
-	open my $fh, '+>', undef or die "open($!)";
+	open my $fh, '+>', undef;
 	my $cfg = do {
 		local $lei->{2} = $fh;
 		$lei->cfg_dump($self->{-f});

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 29/30] convert: use read_all to simplify error checks
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (27 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
  2023-10-19  1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

There wasn't a need to loop anyways with Perl `read' since
the default PerlIO layer will retry.
---
 script/public-inbox-convert | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/script/public-inbox-convert b/script/public-inbox-convert
index 0cc52777..96f6d2ea 100755
--- a/script/public-inbox-convert
+++ b/script/public-inbox-convert
@@ -130,14 +130,8 @@ while (<$rd>) {
 	} elsif (/^commit /) {
 		$state = 'commit';
 	} elsif (/^data ([0-9]+)/) {
-		my $len = $1;
 		print $io $_ or $im->wfail;
-		while ($len) {
-			my $n = read($rd, my $tmp, $len) or die "read: $!";
-			warn "$n != $len\n" if $n != $len;
-			$len -= $n;
-			print $io $tmp or $im->wfail;
-		}
+		print $io PublicInbox::Git::read_all($rd, $1) or $im->wfail;
 		next;
 	} elsif ($state eq 'commit') {
 		if (m{^M 100644 :([0-9]+) (${h}{2}/${h}{38})}o) {

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 30/30] idx_stack: use autodie + read_all
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (28 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
  2023-10-19  1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
  To: meta

We'll also add a note to support multi-hash git repos once git
itself gains that ability.
---
 lib/PublicInbox/IdxStack.pm | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/IdxStack.pm b/lib/PublicInbox/IdxStack.pm
index 54d480bd..cc9e0125 100644
--- a/lib/PublicInbox/IdxStack.pm
+++ b/lib/PublicInbox/IdxStack.pm
@@ -1,16 +1,18 @@
-# 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>
 
 # temporary stack for public-inbox-index
+# FIXME: needs to support multi-hash in the same repo once git itself can
 package PublicInbox::IdxStack;
-use v5.10.1;
-use strict;
+use v5.12;
 use Fcntl qw(:seek);
 use constant PACK_FMT => eval { pack('Q', 1) } ? 'A1QQH*H*' : 'A1IIH*H*';
+use autodie qw(open seek);
+use PublicInbox::Git qw(read_all);
 
 # start off in write-only mode
 sub new {
-	open(my $io, '+>', undef) or die "open: $!";
+	open(my $io, '+>', undef);
 	# latest_cmt is still useful when the newest revision is a `d'(elete),
 	# otherwise we favor $sync->{latest_cmt} for checkpoints and {quit}
 	bless { wr => $io, latest_cmt => $_[1] }, __PACKAGE__
@@ -27,7 +29,7 @@ sub push_rec {
 		$self->{rec_size} = length($rec);
 		$self->{unpack_fmt} = $fmt;
 	};
-	print { $self->{wr} } $rec or die "print: $!";
+	print { $self->{wr} } $rec;
 	$self->{tot_size} += length($rec);
 }
 
@@ -49,12 +51,8 @@ sub pop_rec {
 	my $sz = $self->{rec_size} or return;
 	my $rec_pos = $self->{tot_size} -= $sz;
 	return if $rec_pos < 0;
-	my $io = $self->{rd};
-	seek($io, $rec_pos, SEEK_SET) or die "seek: $!";
-	my $r = read($io, my $buf, $sz);
-	defined($r) or die "read: $!";
-	$r == $sz or die "read($r != $sz)";
-	unpack($self->{unpack_fmt}, $buf);
+	seek($self->{rd}, $rec_pos, SEEK_SET);
+	unpack($self->{unpack_fmt}, read_all($self->{rd}, $sz));
 }
 
 1;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 31/30] lei: simplify startq/au_done wakeup notifications
  2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
                   ` (29 preceding siblings ...)
  2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
@ 2023-10-19  1:14 ` Eric Wong
  30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-19  1:14 UTC (permalink / raw)
  To: meta

We only need to write one byte at MUA start instead of a byte
for every LeiXSearch worker.  Also, make sure it succeeds by
enabling autodie for syswrite.

When reading, we can rely on `:perlio' layer `read' semantics
to retry on EINTR to avoid looping and other error checking.
---
 lib/PublicInbox/LEI.pm        |  9 +++++----
 lib/PublicInbox/LeiXSearch.pm | 28 +++++++++-------------------
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 3ccdd4f7..56e4c001 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,7 +9,7 @@ package PublicInbox::LEI;
 use v5.12;
 use parent qw(PublicInbox::DS PublicInbox::LeiExternal
 	PublicInbox::LeiQuery);
-use autodie qw(bind chdir fork open socket socketpair unlink);
+use autodie qw(bind chdir fork open socket socketpair syswrite unlink);
 use Getopt::Long ();
 use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
 use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -1031,9 +1031,10 @@ sub start_mua {
 		$io->[0] = $self->{1} if $self->{opt}->{stdin} && -t $self->{1};
 		send_exec_cmd($self, $io, \@cmd, {});
 	}
-	if ($self->{lxs} && $self->{au_done}) { # kick wait_startq
-		syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0));
-	}
+
+	# kick wait_startq:
+	syswrite($self->{au_done}, 'q') if $self->{lxs} && $self->{au_done};
+
 	return unless -t $self->{2}; # XXX how to determine non-TUI MUAs?
 	$self->{opt}->{quiet} = 1;
 	delete $self->{-progress};
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 25b66b3b..241b9dab 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -122,26 +122,16 @@ sub _mset_more ($$) {
 	$size >= $mo->{limit} && (($mo->{offset} += $size) < $mo->{total});
 }
 
-# $startq will EOF when do_augment is done augmenting and allow
+# $startq will see `q' in do_post_augment -> start_mua if spawning MUA.
+# Otherwise $startq will EOF when do_augment is done augmenting and allow
 # query_combined_mset and query_thread_mset to proceed.
 sub wait_startq ($) {
 	my ($lei) = @_;
-	my $startq = delete $lei->{startq} or return;
-	while (1) {
-		my $n = sysread($startq, my $do_augment_done, 1);
-		if (defined $n) {
-			return if $n == 0; # no MUA
-			if ($do_augment_done eq 'q') {
-				$lei->{opt}->{quiet} = 1;
-				delete $lei->{opt}->{verbose};
-				delete $lei->{-progress};
-			} else {
-				die "BUG: do_augment_done=`$do_augment_done'";
-			}
-			return;
-		}
-		die "wait_startq: $!" unless $!{EINTR};
-	}
+	read(delete($lei->{startq}) // return, my $buf, 1) or return; # EOF
+	die "BUG: wrote `$buf' to au_done" if $buf ne 'q';
+	$lei->{opt}->{quiet} = 1;
+	delete $lei->{opt}->{verbose};
+	delete $lei->{-progress};
 }
 
 sub mset_progress {
@@ -451,10 +441,10 @@ sub do_post_augment {
 		$lei->fail("$err");
 	}
 	if (!$err && delete $lei->{early_mua}) { # non-augment case
-		eval { $lei->start_mua };
+		eval { $lei->start_mua }; # may trigger wait_startq
 		$lei->fail($@) if $@;
 	}
-	close(delete $lei->{au_done}); # triggers wait_startq in lei_xsearch
+	close(delete $lei->{au_done}); # trigger wait_startq if start_mua didn't
 }
 
 sub incr_post_augment { # called whenever an l2m shard finishes augment

^ permalink raw reply related	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2023-10-19  1:14 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
2023-10-19  1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong

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).