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);

Reply via email to