* [PATCH] spamcheck/spamc: pass GLOB handles instead of FD numbers
@ 2019-12-31 22:34 7% Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2019-12-31 22:34 UTC (permalink / raw)
To: meta
The spawn() interface improvements[1] propagate to popen_rd,
too, so we can avoid weird dances to keep the GLOB handle
references live and just pass the handle around.
[1] commit 267371b1273b518215939e817e53733584b68af7
("spawn: allow passing GLOB handles for redirects")
---
lib/PublicInbox/Spamcheck/Spamc.pm | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index e34748c6..bb288b16 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -7,7 +7,7 @@ use strict;
use warnings;
use PublicInbox::Spawn qw(popen_rd spawn);
use IO::Handle;
-use Fcntl qw(:DEFAULT SEEK_SET);
+use Fcntl qw(SEEK_SET);
sub new {
my ($class) = @_;
@@ -21,9 +21,7 @@ sub new {
sub spamcheck {
my ($self, $msg, $out) = @_;
- my $tmp;
- my $fd = _msg_to_fd($self, $msg, \$tmp);
- my $rdr = { 0 => $fd };
+ 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;
@@ -57,10 +55,9 @@ sub spamlearn {
sub _learn {
my ($self, $msg, $rdr, $field) = @_;
$rdr ||= {};
+ $rdr->{0} = _msg_to_fh($self, $msg);
$rdr->{1} ||= $self->_devnull;
$rdr->{2} ||= $self->_devnull;
- my $tmp;
- $rdr->{0} = _msg_to_fd($self, $msg, \$tmp);
my $pid = spawn($self->{$field}, undef, $rdr);
waitpid($pid, 0);
!$?;
@@ -68,20 +65,18 @@ sub _learn {
sub _devnull {
my ($self) = @_;
- my $fd = $self->{-devnullfd};
- return $fd if defined $fd;
- open my $fh, '+>', '/dev/null' or
+ $self->{-devnull} //= do {
+ open my $fh, '+>', '/dev/null' or
die "failed to open /dev/null: $!";
- $self->{-devnull} = $fh;
- $self->{-devnullfd} = fileno($fh);
+ $fh
+ }
}
-sub _msg_to_fd {
- my ($self, $msg, $tmpref) = @_;
- my $fd;
+sub _msg_to_fh {
+ my ($self, $msg) = @_;
if (my $ref = ref($msg)) {
- my $fileno = eval { fileno($msg) };
- return $fileno if defined $fileno;
+ my $fd = eval { fileno($msg) };
+ return $msg if defined($fd) && $fd >= 0;
open(my $tmpfh, '+>', undef) or die "failed to open: $!";
$tmpfh->autoflush(1);
@@ -89,9 +84,8 @@ sub _msg_to_fd {
print $tmpfh $$msg or die "failed to print: $!";
sysseek($tmpfh, 0, SEEK_SET) or
die "sysseek(fh) failed: $!";
- $$tmpref = $tmpfh;
- return fileno($tmpfh);
+ return $tmpfh;
}
$msg;
}
^ permalink raw reply related [relevance 7%]
* [PATCH] spawn: allow passing GLOB handles for redirects
@ 2019-12-29 12:51 4% Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2019-12-29 12:51 UTC (permalink / raw)
To: meta
We can save callers the trouble of {-hold} and {-dev_null}
refs as well as the trouble of calling fileno().
---
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 | 42 ++++++++++++++++++-------------
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 +-
16 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm
index 32a9f65e..5a3554cf 100644
--- a/lib/PublicInbox/Admin.pm
+++ b/lib/PublicInbox/Admin.pm
@@ -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 @_ };
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index af3a5712..8d587469 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -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: $!");
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index b7640d42..8dd27a75 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -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 {
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 46de09c4..34f7d122 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -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);
diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm
index 17a43060..03666646 100644
--- a/lib/PublicInbox/SolverGit.pm
+++ b/lib/PublicInbox/SolverGit.pm
@@ -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);
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index c64812dd..6d42d5bc 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -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;
}
diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm
index 29b13371..2ac02c56 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 @@ 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";
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index b0b1f4d9..532cbee6 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -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: $!";
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 77b3bde4..c614e20c 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -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);
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 9f897dad..544242a3 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -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 6cfadd08..89a276bf 100644
--- 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);
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 4ef1618a..4ed34934 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -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') {
diff --git a/t/import.t b/t/import.t
index 3cf7e2d2..cfbe501b 100644
--- a/t/import.t
+++ b/t/import.t
@@ -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);
diff --git a/t/qspawn.t b/t/qspawn.t
index 8bc88e0e..ab3764b8 100644
--- a/t/qspawn.t
+++ b/t/qspawn.t
@@ -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 ($) {
diff --git a/t/solver_git.t b/t/solver_git.t
index 0c272d77..55746994 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -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;
diff --git a/t/spawn.t b/t/spawn.t
index 2e409157..c31c4f19 100644
--- 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');
^ permalink raw reply related [relevance 4%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-12-29 12:51 4% [PATCH] spawn: allow passing GLOB handles for redirects Eric Wong
2019-12-31 22:34 7% [PATCH] spamcheck/spamc: pass GLOB handles instead of FD numbers 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).