* [PATCH] spawn: actually die on (vfork|fork) failures
@ 2020-02-02 17:51 7% Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2020-02-02 17:51 UTC (permalink / raw)
To: meta
Commit 9f5a583694396f84 ("spawn (and thus popen_rd) die on failure")
was incomplete in that it only removed error checking for spawn
failures for non-(vfork|fork) calls, but the actual (vfork|fork)
PID result could still be undef.
Fixes: 9f5a583694396f84 ("spawn (and thus popen_rd) die on failure")
---
lib/PublicInbox/Spawn.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index b02d5368..727caf1c 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -210,7 +210,8 @@ sub spawn ($;$$) {
}
my $cd = $opts->{'-C'} // ''; # undef => NULL mapping doesn't work?
my $pid = pi_fork_exec($redir, $f, $cmd, \@env, $rlim, $cd);
- $pid < 0 ? undef : $pid;
+ die "fork_exec failed: $!\n" unless defined $pid;
+ $pid;
}
sub popen_rd {
^ permalink raw reply related [relevance 7%]
* [PATCH 5/5] spawn (and thus popen_rd) die on failure
2020-01-10 9:14 5% [PATCH 0/5] misc cleanups and bugfixes Eric Wong
@ 2020-01-10 9:14 5% ` Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
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) $!";
^ permalink raw reply related [relevance 5%]
* [PATCH 0/5] misc cleanups and bugfixes
@ 2020-01-10 9:14 5% Eric Wong
2020-01-10 9:14 5% ` [PATCH 5/5] spawn (and thus popen_rd) die on failure Eric Wong
0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2020-01-10 9:14 UTC (permalink / raw)
To: meta
Some minor things, probably unlikely to matter a lot
but we can trim some code a little.
Eric Wong (5):
inbox: use PublicInbox::Git::host_prefix_url for base_url
allow HTTP_HOST to be '0' via defined() checks
git: ->modified uses cat_async
git: remove ->commit_title method
spawn (and thus popen_rd) die on failure
lib/PublicInbox/Config.pm | 2 +-
lib/PublicInbox/Git.pm | 24 +++++++++++-------------
lib/PublicInbox/HTTP.pm | 2 +-
lib/PublicInbox/Import.pm | 2 --
lib/PublicInbox/Inbox.pm | 30 +++++++++++++-----------------
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 -
t/solver_git.t | 8 +-------
14 files changed, 28 insertions(+), 51 deletions(-)
^ permalink raw reply [relevance 5%]
Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-01-10 9:14 5% [PATCH 0/5] misc cleanups and bugfixes Eric Wong
2020-01-10 9:14 5% ` [PATCH 5/5] spawn (and thus popen_rd) die on failure Eric Wong
2020-02-02 17:51 7% [PATCH] spawn: actually die on (vfork|fork) failures Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
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).