On Sun, Mar 17, 2013 at 08:15:32PM +0800, Daniel Hartwig wrote: > On 17 March 2013 19:56, Serafeim Zanikolas <s...@debian.org> wrote: > > On Sun, Mar 17, 2013 at 02:14:50PM +0800, Daniel Hartwig wrote: > >> The data can be passed through an open fd, similar to dpkg --status-fd > >> argument. Then there are no issues due to filesystems global > >> namespace and it removes the fs as an unrequired middle-man. > > > > Sure, that'd be an improvement. Would you make apt pass the fd number to > > apt-listbugs in the command line? > > or just using a well known env. variable that will also work with > substituation in the command line.
Attached the updated patches for apt and apt-listbugs, which implement Daniel's proposal of using an fd rather than a fifo. The HookFifoFilename option is replaced by: DPkgs::Tools::Options::/usr/sbin/apt-listbugs::HookAvoidStdin "yes"; The default value of Pipes[0] (the fd to be inherited and read by apt-listbugs) was normally 3, which seems to be FD_CLOEXEC'd somewhere before the exec to apt-listbugs (apt-listbugs was failing with EBADF), so I'm using instead 20 as a minimum. -- Every great idea is worthless without someone to do the work. --Neil Williams
diff --git a/apt-listbugs b/apt-listbugs index 58d67cb..b387947 100755 --- a/apt-listbugs +++ b/apt-listbugs @@ -289,7 +289,19 @@ when "apt" puts if $DEBUG puts "Pre-Install-Pkgs hook info:" if $DEBUG state=1 - STDIN.each { |pkg| + apt_hook_fd = ENV["AptHookFd"] + if apt_hook_fd.nil? + $stderr.puts sprintf(_("E: AptHookFd is undefined")) + exit(1) + end + begin + apt_hook_stream = IO.open(apt_hook_fd, 'r') + rescue Errno::ENOENT + $stderr.puts sprintf(_("W: Cannot open file descriptor %s"), apt_hook_fd) + exit(1) + end + + apt_hook_stream.each { |pkg| pkg=pkg.rstrip case state when 1 @@ -353,6 +365,7 @@ when "apt" end end } + apt_hook_stream.close puts if $DEBUG when "list", "rss" ARGV.each { |pkg|
diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 6cb8bc6..dabf48c 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -328,8 +328,9 @@ bool pkgDPkgPM::SendV2Pkgs(FILE *F) // DPkgPM::RunScriptsWithPkgs - Run scripts with package names on stdin /*{{{*/ // --------------------------------------------------------------------- /* This looks for a list of scripts to run from the configuration file - each one is run and is fed on standard input a list of all .deb files - that are due to be installed. */ + each one is run and is fed on standard input (or another fd, if + HookAvoidStdin is set) a list of all .deb files that are due to be + installed. */ bool pkgDPkgPM::RunScriptsWithPkgs(const char *Cnf) { Configuration::Item const *Opts = _config->Tree(Cnf); @@ -352,10 +353,14 @@ bool pkgDPkgPM::RunScriptsWithPkgs(const char *Cnf) unsigned int Version = _config->FindI(OptSec+"::Version",1); + // Feed subprocess via stdin, unless HookAvoidStdin is true + bool const UseStdin = !_config->FindB(OptSec+"::HookAvoidStdin",false); + // Create the pipes int Pipes[2]; if (pipe(Pipes) != 0) return _error->Errno("pipe","Failed to create IPC pipe to subprocess"); + if (UseStdin == true) SetCloseExec(Pipes[0],true); SetCloseExec(Pipes[1],true); @@ -364,7 +369,21 @@ bool pkgDPkgPM::RunScriptsWithPkgs(const char *Cnf) if (Process == 0) { // Setup the FDs + if (UseStdin == true) dup2(Pipes[0],STDIN_FILENO); + else + { + // small fd numbers (<5) are closed down the process tree, so use + // a higher number instead + int ChildReadFd = fcntl(Pipes[0], F_DUPFD, 20); + if (ChildReadFd < 0) + return _error->Errno("fcntl","Failed to duplicate file descriptor"); + char AptHookEnv[80]; + if (sprintf(AptHookEnv, "AptHookFd=%d", ChildReadFd) < 0) + return _error->Errno("sprintf","Failed to contruct environment variable value"); + if (putenv(AptHookEnv) != 0) + return _error->Errno("putenv","Failed to set environment variable AptHookFd"); + } SetCloseExec(STDOUT_FILENO,false); SetCloseExec(STDIN_FILENO,false); SetCloseExec(STDERR_FILENO,false);