lib/PublicInbox/Admin.pm | 3 +-- lib/PublicInbox/Git.pm | 4 ++-- lib/PublicInbox/GitHTTPBackend.pm | 2 +- lib/PublicInbox/Import.pm | 2 +- lib/PublicInbox/SolverGit.pm | 2 +- lib/PublicInbox/Spawn.pm | 40 +++++++++++++++++++++++----------------- lib/PublicInbox/SpawnPP.pm | 18 +++++++----------- lib/PublicInbox/TestCommon.pm | 2 +- lib/PublicInbox/V2Writable.pm | 2 +- lib/PublicInbox/Xapcmd.pm | 2 +- t/git.t | 2 +- t/httpd-corner.t | 2 +- t/import.t | 2 +- t/qspawn.t | 5 +++++ t/solver_git.t | 2 +- t/spawn.t | 2 +- diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm index 32a9f65edfb7f5a6b808ce5bb0489cd8089d1be9..5a3554cfe460c9efb8189f93422fc90a9a5efe9b 100644 --- a/lib/PublicInbox/Admin.pm +++ b/lib/PublicInbox/Admin.pm @@ -241,8 +241,7 @@ } if ($opt->{quiet}) { open my $null, '>', '/dev/null' or die "failed to open /dev/null: $!\n"; - $opt->{1} = fileno($null); # suitable for spawn() redirect - $opt->{-dev_null} = $null; + $opt->{1} = $null; # suitable for spawn() redirect } else { $opt->{verbose} ||= 1; $opt->{-progress} = sub { print STDERR @_ }; diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index af3a5712cf79dec86c90e4f92eb9a2df87a3683e..8d587469f3eb5828ca6dbb4756f54a66db1cc1b7 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -114,12 +114,12 @@ } my @cmd = (qw(git), "--git-dir=$self->{git_dir}", qw(-c core.abbrev=40 cat-file), $batch); - my $redir = { 0 => fileno($out_r), 1 => fileno($in_w) }; + my $redir = { 0 => $out_r, 1 => $in_w }; if ($err) { my $id = "git.$self->{git_dir}$batch.err"; my $fh = tmpfile($id) or fail($self, "tmpfile($id): $!"); $self->{$err} = $fh; - $redir->{2} = fileno($fh); + $redir->{2} = $fh; } my $p = spawn(\@cmd, undef, $redir); defined $p or fail($self, "spawn failed: $!"); diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm index b7640d42a02aecde1c3643e3be7e7177b370df7c..8dd27a75e6c625205789b3989e1eee5217872a48 100644 --- a/lib/PublicInbox/GitHTTPBackend.pm +++ b/lib/PublicInbox/GitHTTPBackend.pm @@ -164,7 +164,7 @@ unless (defined(sysseek($in, 0, SEEK_SET))) { err($env, "error seeking temporary file: $!"); return; } - { 0 => fileno($in), -hold => $in }; + { 0 => $in }; } sub parse_cgi_headers { diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm index 46de09c40792c511b835de6bded17b8c8173a1a9..34f7d122af67526306dc6feebce30f2c0d9e1d9b 100644 --- a/lib/PublicInbox/Import.pm +++ b/lib/PublicInbox/Import.pm @@ -66,7 +66,7 @@ my $git_dir = $git->{git_dir}; my @cmd = ('git', "--git-dir=$git_dir", qw(fast-import --quiet --done --date-format=raw)); - my $rdr = { 0 => fileno($out_r), 1 => fileno($in_w) }; + 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); diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm index 17a430606dfc64678583fd85a16b217a399213d8..036666469ee1750b0886d676c27575d4c9665fbc 100644 --- a/lib/PublicInbox/SolverGit.pm +++ b/lib/PublicInbox/SolverGit.pm @@ -259,7 +259,7 @@ $in->flush or die "flush: $!"; sysseek($in, 0, 0) or die "seek: $!"; dbg($self, 'preparing index'); - my $rdr = { 0 => fileno($in), -hold => $in }; + my $rdr = { 0 => $in }; my $cmd = [ qw(git update-index -z --index-info) ]; my $qsp = PublicInbox::Qspawn->new($cmd, $self->{git_env}, $rdr); $path_a = git_quote($path_a); diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index c64812ddf4a3fb01e6bb13f3c1489316a78693c1..6d42d5bc70c39318865848013d2f7e96b77acbf8 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -79,20 +79,14 @@ writev(2, iov, 3); _exit(1); } -#define REDIR(var,fd) do { \ - if (var != fd && dup2(var, fd) < 0) \ - xerr("error redirecting std"#var ": "); \ -} while (0) - /* - * unstable internal API. This was easy to implement but does not - * support arbitrary redirects. It'll be updated depending on + * unstable internal API. It'll be updated depending on * whatever we'll need in the future. * Be sure to update PublicInbox::SpawnPP if this changes */ -int pi_fork_exec(int in, int out, int err, - SV *file, SV *cmdref, SV *envref, SV *rlimref) +int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref) { + AV *redir = (AV *)SvRV(redirref); AV *cmd = (AV *)SvRV(cmdref); AV *env = (AV *)SvRV(envref); AV *rlim = (AV *)SvRV(rlimref); @@ -112,11 +106,16 @@ assert(ret == 0 && "BUG calling sigprocmask to block"); pid = vfork(); if (pid == 0) { int sig; - I32 i, max; + I32 i, child_fd, max = av_len(redir); - REDIR(in, 0); - REDIR(out, 1); - REDIR(err, 2); + for (child_fd = 0; child_fd <= max; child_fd++) { + SV **parent = av_fetch(redir, child_fd, 0); + int parent_fd = SvIV(*parent); + if (parent_fd == child_fd) + continue; + if (dup2(parent_fd, child_fd) < 0) + xerr("dup2"); + } for (sig = 1; sig < NSIG; sig++) signal(sig, SIG_DFL); /* ignore errors on signals */ @@ -196,9 +195,16 @@ my %env = $env ? (%ENV, %$env) : %ENV; while (my ($k, $v) = each %env) { push @env, "$k=$v"; } - my $in = $opts->{0} || 0; - my $out = $opts->{1} || 1; - my $err = $opts->{2} || 2; + my $redir = []; + for my $child_fd (0..2) { + my $parent_fd = $opts->{$child_fd}; + if (defined($parent_fd) && $parent_fd !~ /\A[0-9]+\z/) { + defined(my $fd = fileno($parent_fd)) or + die "$parent_fd not an IO GLOB? $!"; + $parent_fd = $fd; + } + $redir->[$child_fd] = $parent_fd // $child_fd; + } my $rlim = []; foreach my $l (RLIMITS()) { @@ -210,7 +216,7 @@ next; } push @$rlim, $r, @$v; } - my $pid = pi_fork_exec($in, $out, $err, $f, $cmd, \@env, $rlim); + my $pid = pi_fork_exec($redir, $f, $cmd, \@env, $rlim); $pid < 0 ? undef : $pid; } diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm index 29b1337103f59c164a6bd1c2fe79ea2d125cc743..2ac02c564c76e36b24353f1b32d85e3677c0c9ad 100644 --- a/lib/PublicInbox/SpawnPP.pm +++ b/lib/PublicInbox/SpawnPP.pm @@ -9,8 +9,8 @@ use warnings; use POSIX qw(dup2 :signal_h); # Pure Perl implementation for folks that do not use Inline::C -sub pi_fork_exec ($$$$$$) { - my ($in, $out, $err, $f, $cmd, $env, $rlim) = @_; +sub pi_fork_exec ($$$$$) { + my ($redir, $f, $cmd, $env, $rlim) = @_; my $old = POSIX::SigSet->new(); my $set = POSIX::SigSet->new(); $set->fillset or die "fillset failed: $!"; @@ -27,16 +27,12 @@ my ($r, $soft, $hard) = splice(@$rlim, 0, 3); BSD::Resource::setrlimit($r, $soft, $hard) or warn "failed to set $r=[$soft,$hard]\n"; } - if ($in != 0) { - dup2($in, 0) or die "dup2 failed for stdin: $!"; - } - if ($out != 1) { - dup2($out, 1) or die "dup2 failed for stdout: $!"; - } - if ($err != 2) { - dup2($err, 2) or die "dup2 failed for stderr: $!"; + for my $child_fd (0..$#$redir) { + my $parent_fd = $redir->[$child_fd]; + next if $parent_fd == $child_fd; + dup2($parent_fd, $child_fd) or + die "dup2($parent_fd, $child_fd): $!\n"; } - if ($ENV{MOD_PERL}) { exec which('env'), '-i', @$env, @$cmd; die "exec env -i ... $cmd->[0] failed: $!\n"; diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index b0b1f4d9e17de11b50837f0d9b8b27d2939195f3..532cbee676b2ac5b5184ddf24df2ed417c757bf0 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -194,7 +194,7 @@ my $redir = $opt->{$fd}; next unless ref($redir); open my $fh, '+>', undef or die "open: $!"; $fhref->[$fd] = $fh; - $spawn_opt->{$fd} = fileno($fh); + $spawn_opt->{$fd} = $fh; next if $fd > 0; $fh->autoflush(1); print $fh $$redir or die "print: $!"; diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index 77b3bde4d7b35939ba633ae961c671489d5f3146..c614e20c382518e6d257b59871b4eb109d839869 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -473,7 +473,7 @@ sysseek($tmp_fh, 0, 0) or die "seek failed: $!"; my ($r, $w); pipe($r, $w) or die "failed to create pipe: $!"; - my $rdr = { 0 => fileno($tmp_fh), 1 => fileno($w) }; + my $rdr = { 0 => $tmp_fh, 1 => $w }; my $git_dir = $self->{-inbox}->git->{git_dir}; my $cmd = ['git', "--git-dir=$git_dir", qw(hash-object --stdin)]; my $pid = spawn($cmd, undef, $rdr); diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm index 9f897dad05f31d098c68183891aace44d1bc2deb..544242a36663594d489aa4a54bfa13e77e30cdd6 100644 --- a/lib/PublicInbox/Xapcmd.pm +++ b/lib/PublicInbox/Xapcmd.pm @@ -286,7 +286,7 @@ foreach my $fd (0..2) { defined(my $dfd = $opt->{$fd}) or next; $rdr->{$fd} = $dfd; } - $rdr->{1} = fileno($w) if $pr && pipe($r, $w); + $rdr->{1} = $w if $pr && pipe($r, $w); # we rely on --no-renumber to keep docids synched to NNTP my $cmd = [ $XAPIAN_COMPACT, '--no-renumber' ]; diff --git a/t/git.t b/t/git.t index 6cfadd085f20d82c46c47c015da870b1e74476e9..89a276bf87f18f29035fb42dee21729f02bfaf94 100644 --- a/t/git.t +++ b/t/git.t @@ -92,7 +92,7 @@ my ($alt, $alt_obj) = tmpdir(); my @cmd = ('git', "--git-dir=$alt", qw(hash-object -w --stdin)); is(system(qw(git init -q --bare), $alt), 0, 'create alt directory'); open my $fh, '<', "$alt/config" or die "open failed: $!\n"; - my $rd = popen_rd(\@cmd, {}, { 0 => fileno($fh) } ); + my $rd = popen_rd(\@cmd, {}, { 0 => $fh } ); close $fh or die "close failed: $!"; chomp(my $remote = <$rd>); my $gcf = PublicInbox::Git->new($dir); diff --git a/t/httpd-corner.t b/t/httpd-corner.t index 4ef1618a60f171bb5abe1c746994671f3ae443df..4ed34934e500649470ba411629764afbe286f044 100644 --- a/t/httpd-corner.t +++ b/t/httpd-corner.t @@ -266,7 +266,7 @@ pipe($r, $w) or die "pipe: $!"; my $cmd = [qw(curl --tcp-nodelay --no-buffer -T- -HExpect: -sS), $url]; open my $cout, '+>', undef or die; open my $cerr, '>', undef or die; - my $rdr = { 0 => fileno($r), 1 => fileno($cout), 2 => fileno($cerr) }; + my $rdr = { 0 => $r, 1 => $cout, 2 => $cerr }; my $pid = spawn($cmd, undef, $rdr); close $r or die "close read pipe: $!"; foreach my $c ('a'..'z') { diff --git a/t/import.t b/t/import.t index 3cf7e2d2e4ea4e1e6168c7caae797ba945645785..cfbe501b102bca5061856ac52609529ed650c37e 100644 --- a/t/import.t +++ b/t/import.t @@ -44,7 +44,7 @@ print $in $mime->as_string or die "write failed: $!"; $in->flush or die "flush failed: $!"; $in->seek(0, SEEK_SET); my $out = tempfile(); - my $pid = spawn(\@cmd, {}, { 0 => fileno($in), 1 => fileno($out)}); + my $pid = spawn(\@cmd, {}, { 0 => $in, 1 => $out }); is(waitpid($pid, 0), $pid, 'waitpid succeeds on hash-object'); is($?, 0, 'hash-object'); $out->seek(0, SEEK_SET); diff --git a/t/qspawn.t b/t/qspawn.t index 8bc88e0e0469f439c78cb31482955ee64ace56b1..ab3764b87baaeb9358389477106baf0d46508189 100644 --- a/t/qspawn.t +++ b/t/qspawn.t @@ -10,6 +10,11 @@ my $qsp = PublicInbox::Qspawn->new($cmd, {}, { 2 => 1 }); my $res; $qsp->psgi_qx({}, undef, sub { $res = ${$_[0]} }); is($res, "err\nout\n", 'captured stderr and stdout'); + + $res = undef; + $qsp = PublicInbox::Qspawn->new($cmd, {}, { 2 => \*STDOUT }); + $qsp->psgi_qx({}, undef, sub { $res = ${$_[0]} }); + is($res, "err\nout\n", 'captured stderr and stdout'); } sub finish_err ($) { diff --git a/t/solver_git.t b/t/solver_git.t index 0c272d777c752ce9a10b3a003f009d1c0dbc5f4a..55746994d9f94cc887d2b5c71f46315f73cb9445 100644 --- a/t/solver_git.t +++ b/t/solver_git.t @@ -127,7 +127,7 @@ my $env = { GIT_DIR => $binfoo }; while (my ($label, $size) = each %bin) { pipe(my ($rout, $wout)) or die; pipe(my ($rin, $win)) or die; - my $rdr = { 0 => fileno($rin), 1 => fileno($wout) }; + my $rdr = { 0 => $rin, 1 => $wout }; my $pid = spawn($cmd , $env, $rdr); $wout = $rin = undef; print { $win } ("\0" x $size) or die; diff --git a/t/spawn.t b/t/spawn.t index 2e4091574e1be4bcbafd91ff851f0938f60557ef..c31c4f193a4fd2379df35252cec28aab425dc754 100644 --- a/t/spawn.t +++ b/t/spawn.t @@ -31,7 +31,7 @@ { my ($r, $w); pipe $r, $w or die "pipe failed: $!"; my $pid = spawn(['sh', '-c', 'echo $HELLO'], - { 'HELLO' => 'world' }, { 1 => fileno($w) }); + { 'HELLO' => 'world' }, { 1 => $w }); close $w or die "close pipe[1] failed: $!"; is(<$r>, "world\n", 'read stdout of spawned from pipe'); is(waitpid($pid, 0), $pid, 'waitpid succeeds on spawned process');