]> Sergey Matveev's repositories - public-inbox.git/commitdiff
spawn: allow passing GLOB handles for redirects
authorEric Wong <e@80x24.org>
Sun, 29 Dec 2019 12:51:18 +0000 (12:51 +0000)
committerEric Wong <e@80x24.org>
Mon, 30 Dec 2019 00:06:23 +0000 (00:06 +0000)
We can save callers the trouble of {-hold} and {-dev_null}
refs as well as the trouble of calling fileno().

16 files changed:
lib/PublicInbox/Admin.pm
lib/PublicInbox/Git.pm
lib/PublicInbox/GitHTTPBackend.pm
lib/PublicInbox/Import.pm
lib/PublicInbox/SolverGit.pm
lib/PublicInbox/Spawn.pm
lib/PublicInbox/SpawnPP.pm
lib/PublicInbox/TestCommon.pm
lib/PublicInbox/V2Writable.pm
lib/PublicInbox/Xapcmd.pm
t/git.t
t/httpd-corner.t
t/import.t
t/qspawn.t
t/solver_git.t
t/spawn.t

index 32a9f65edfb7f5a6b808ce5bb0489cd8089d1be9..5a3554cfe460c9efb8189f93422fc90a9a5efe9b 100644 (file)
@@ -241,8 +241,7 @@ sub progress_prepare ($) {
        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 @_ };
index af3a5712cf79dec86c90e4f92eb9a2df87a3683e..8d587469f3eb5828ca6dbb4756f54a66db1cc1b7 100644 (file)
@@ -114,12 +114,12 @@ sub _bidi_pipe {
 
        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: $!");
index b7640d42a02aecde1c3643e3be7e7177b370df7c..8dd27a75e6c625205789b3989e1eee5217872a48 100644 (file)
@@ -164,7 +164,7 @@ sub input_prepare {
                err($env, "error seeking temporary file: $!");
                return;
        }
-       { 0 => fileno($in), -hold => $in };
+       { 0 => $in };
 }
 
 sub parse_cgi_headers {
index 46de09c40792c511b835de6bded17b8c8173a1a9..34f7d122af67526306dc6feebce30f2c0d9e1d9b 100644 (file)
@@ -66,7 +66,7 @@ sub gfi_start {
        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);
index 17a430606dfc64678583fd85a16b217a399213d8..036666469ee1750b0886d676c27575d4c9665fbc 100644 (file)
@@ -259,7 +259,7 @@ sub prepare_index ($) {
        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);
index c64812ddf4a3fb01e6bb13f3c1489316a78693c1..6d42d5bc70c39318865848013d2f7e96b77acbf8 100644 (file)
@@ -79,20 +79,14 @@ static void xerr(const char *msg)
        _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 @@ int pi_fork_exec(int in, int out, int err,
        pid = vfork();
        if (pid == 0) {
                int sig;
-               I32 i, max;
-
-               REDIR(in, 0);
-               REDIR(out, 1);
-               REDIR(err, 2);
+               I32 i, child_fd, max = av_len(redir);
+
+               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 @@ sub spawn ($;$$) {
        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 @@ sub spawn ($;$$) {
                }
                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;
 }
 
index 29b1337103f59c164a6bd1c2fe79ea2d125cc743..2ac02c564c76e36b24353f1b32d85e3677c0c9ad 100644 (file)
@@ -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 @@ sub pi_fork_exec ($$$$$$) {
                        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: $!";
+               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 ($out != 1) {
-                       dup2($out, 1) or die "dup2 failed for stdout: $!";
-               }
-               if ($err != 2) {
-                       dup2($err, 2) or die "dup2 failed for stderr: $!";
-               }
-
                if ($ENV{MOD_PERL}) {
                        exec which('env'), '-i', @$env, @$cmd;
                        die "exec env -i ... $cmd->[0] failed: $!\n";
index b0b1f4d9e17de11b50837f0d9b8b27d2939195f3..532cbee676b2ac5b5184ddf24df2ed417c757bf0 100644 (file)
@@ -194,7 +194,7 @@ sub run_script ($;$$) {
                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: $!";
index 77b3bde4d7b35939ba633ae961c671489d5f3146..c614e20c382518e6d257b59871b4eb109d839869 100644 (file)
@@ -473,7 +473,7 @@ sub git_hash_raw ($$) {
 
        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);
index 9f897dad05f31d098c68183891aace44d1bc2deb..544242a36663594d489aa4a54bfa13e77e30cdd6 100644 (file)
@@ -286,7 +286,7 @@ sub compact ($$) {
                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 (file)
--- a/t/git.t
+++ b/t/git.t
@@ -92,7 +92,7 @@ if ('alternates reloaded') {
        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);
index 4ef1618a60f171bb5abe1c746994671f3ae443df..4ed34934e500649470ba411629764afbe286f044 100644 (file)
@@ -266,7 +266,7 @@ SKIP: {
        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') {
index 3cf7e2d2e4ea4e1e6168c7caae797ba945645785..cfbe501b102bca5061856ac52609529ed650c37e 100644 (file)
@@ -44,7 +44,7 @@ if ($v2) {
        $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);
index 8bc88e0e0469f439c78cb31482955ee64ace56b1..ab3764b87baaeb9358389477106baf0d46508189 100644 (file)
@@ -10,6 +10,11 @@ use_ok 'PublicInbox::Qspawn';
        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 ($) {
index 0c272d777c752ce9a10b3a003f009d1c0dbc5f4a..55746994d9f94cc887d2b5c71f46315f73cb9445 100644 (file)
@@ -127,7 +127,7 @@ SKIP: {
        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;
index 2e4091574e1be4bcbafd91ff851f0938f60557ef..c31c4f193a4fd2379df35252cec28aab425dc754 100644 (file)
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -31,7 +31,7 @@ use PublicInbox::Spawn qw(which spawn popen_rd);
        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');