From 9f5a583694396f84056b9d92255bba0197b52bc8 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@yhbt.net>
Date: Fri, 10 Jan 2020 09:14:19 +0000
Subject: [PATCH] spawn (and thus popen_rd) die on failure

Most spawn and popen_rd callers die on failure to spawn,
anyways, and some are missing checks entirely.  This saves
us a bunch of verbose error-checking code in callers.

This also makes popen_rd more consistent, since it already
dies on pipe creation failures.
---
 lib/PublicInbox/Config.pm          | 2 +-
 lib/PublicInbox/Git.pm             | 3 ---
 lib/PublicInbox/Import.pm          | 2 --
 lib/PublicInbox/SearchIdx.pm       | 1 -
 lib/PublicInbox/Spamcheck/Spamc.pm | 1 -
 lib/PublicInbox/Spawn.pm           | 1 -
 lib/PublicInbox/TestCommon.pm      | 1 -
 lib/PublicInbox/V2Writable.pm      | 1 -
 lib/PublicInbox/WatchMaildir.pm    | 1 -
 lib/PublicInbox/WwwListing.pm      | 4 +---
 t/check-www-inbox.perl             | 1 -
 11 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index cc8c1eaf..56d146c2 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -158,7 +158,7 @@ sub git_config_dump {
 	return {} unless -e $file;
 	my @cmd = (qw/git config -z -l/, "--file=$file");
 	my $cmd = join(' ', @cmd);
-	my $fh = popen_rd(\@cmd) or die "popen_rd failed for $file: $!\n";
+	my $fh = popen_rd(\@cmd);
 	my $rv = config_fh_parse($fh, "\0", "\n");
 	close $fh or die "failed to close ($cmd) pipe: $?";
 	$rv;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index f3b7a0a0..8ee04e17 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -122,7 +122,6 @@ sub _bidi_pipe {
 		$redir->{2} = $fh;
 	}
 	my $p = spawn(\@cmd, undef, $redir);
-	defined $p or fail($self, "spawn failed: $!");
 	$self->{$pid} = $p;
 	$out_w->autoflush(1);
 	$self->{$out} = $out_w;
@@ -256,7 +255,6 @@ sub popen {
 sub qx {
 	my ($self, @cmd) = @_;
 	my $fh = $self->popen(@cmd);
-	defined $fh or return;
 	local $/ = "\n";
 	return <$fh> if wantarray;
 	local $/;
@@ -347,7 +345,6 @@ sub modified ($) {
 	my ($self) = @_;
 	my $modified = 0;
 	my $fh = popen($self, qw(rev-parse --branches));
-	defined $fh or return $modified;
 	cat_async_begin($self);
 	local $/ = "\n";
 	foreach my $oid (<$fh>) {
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 572e9bb9..6ac43d37 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -68,7 +68,6 @@ sub gfi_start {
 			--quiet --done --date-format=raw));
 	my $rdr = { 0 => $out_r, 1 => $in_w };
 	my $pid = spawn(\@cmd, undef, $rdr);
-	die "spawn fast-import failed: $!" unless defined $pid;
 	$out_w->autoflush(1);
 	$self->{in} = $in_r;
 	$self->{out} = $out_w;
@@ -430,7 +429,6 @@ sub add {
 sub run_die ($;$$) {
 	my ($cmd, $env, $rdr) = @_;
 	my $pid = spawn($cmd, $env, $rdr);
-	defined $pid or die "spawning ".join(' ', @$cmd)." failed: $!";
 	waitpid($pid, 0) == $pid or die join(' ', @$cmd) .' did not finish';
 	$? == 0 or die join(' ', @$cmd) . " failed: $?\n";
 }
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index f14809d2..cb554912 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -666,7 +666,6 @@ sub is_ancestor ($$$) {
 	my $cmd = [ 'git', "--git-dir=$git->{git_dir}",
 		qw(merge-base --is-ancestor), $cur, $tip ];
 	my $pid = spawn($cmd);
-	defined $pid or die "spawning ".join(' ', @$cmd)." failed: $!";
 	waitpid($pid, 0) == $pid or die join(' ', @$cmd) .' did not finish';
 	$? == 0;
 }
diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index bb288b16..d9cc47e3 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -23,7 +23,6 @@ sub spamcheck {
 
 	my $rdr = { 0 => _msg_to_fh($self, $msg) };
 	my ($fh, $pid) = popen_rd($self->{checkcmd}, undef, $rdr);
-	defined $pid or die "failed to popen_rd spamc: $!\n";
 	my $r;
 	unless (ref $out) {
 		my $buf = '';
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 1c74a596..b02d5368 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -219,7 +219,6 @@ sub popen_rd {
 	$opts ||= {};
 	$opts->{1} = fileno($w);
 	my $pid = spawn($cmd, $env, $opts);
-	return unless defined $pid;
 	return ($r, $pid) if wantarray;
 	my $ret = gensym;
 	tie *$ret, 'PublicInbox::ProcessPipe', $pid, $r;
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index d6d1e939..b3c95612 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -215,7 +215,6 @@ sub run_script ($;$$) {
 		require PublicInbox::Spawn;
 		my $cmd = [ key2script($key), @argv ];
 		my $pid = PublicInbox::Spawn::spawn($cmd, $env, $spawn_opt);
-		defined($pid) or die "spawn: $!";
 		if (defined $pid) {
 			my $r = waitpid($pid, 0);
 			defined($r) or die "waitpid: $!";
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 6021de44..51794326 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -777,7 +777,6 @@ sub diff ($$$) {
 	my $cmd = [ qw(diff -u), $an, $bn ];
 	print STDERR "# MID conflict <$mid>\n";
 	my $pid = spawn($cmd, undef, { 1 => 2 });
-	defined $pid or die "diff failed to spawn $!";
 	waitpid($pid, 0) == $pid or die "diff did not finish";
 	unlink($an, $bn);
 }
diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 8a8c1262..dfb987e8 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -7,7 +7,6 @@ package PublicInbox::WatchMaildir;
 use strict;
 use warnings;
 use PublicInbox::MIME;
-use PublicInbox::Spawn qw(spawn);
 use PublicInbox::InboxWritable;
 use File::Temp 0.19 (); # 0.19 for ->newdir
 use PublicInbox::Filter::Base qw(REJECT);
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index a52dba11..8d610037 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -136,9 +136,7 @@ sub fingerprint ($) {
 	my ($git) = @_;
 	# TODO: convert to qspawn for fairness when there's
 	# thousands of repos
-	my ($fh, $pid) = $git->popen('show-ref') or
-		die "popen($git->{git_dir} show-ref) failed: $!";
-
+	my ($fh, $pid) = $git->popen('show-ref');
 	my $dig = Digest::SHA->new(1);
 	while (read($fh, my $buf, 65536)) {
 		$dig->add($buf);
diff --git a/t/check-www-inbox.perl b/t/check-www-inbox.perl
index db292c50..40209957 100644
--- a/t/check-www-inbox.perl
+++ b/t/check-www-inbox.perl
@@ -48,7 +48,6 @@ my $atom_check = eval {
 			2 => fileno($err_fh),
 		};
 		my $pid = spawn($cmd, undef, $rdr);
-		defined $pid or die "spawn failure: $!";
 		while (waitpid($pid, 0) != $pid) {
 			next if $!{EINTR};
 			warn "waitpid(xmlstarlet, $pid) $!";
-- 
2.44.0