[Libguestfs] [PATCH 2/2] daemon: Remove redundant fork in commandrvf

[Matthew Booth]

Currently the code is doing a redundant fork when passed the
COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN flag. The additional sub-process does a
chroot() which has no effect because all file handles are already open at that
point, then simply copies its input to its output.

This change simply replaces the above with a dup2 of the passed file handle to
STDIN of the command process.
---
 daemon/guestfsd.c | 89 +++++--------------------------------------------------
 1 file changed, 7 insertions(+), 82 deletions(-)

diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 4b3dd5f..7e83a2a 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -789,8 +789,8 @@ commandrvf (char **stdoutput, char **stderror, int flags,
   size_t so_size = 0, se_size = 0;
   int so_fd[2], se_fd[2];
   int flag_copy_stdin = flags & COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN;
-  int stdin_fd[2] = { -1, -1 };
-  pid_t pid, stdin_pid = -1;
+  int flag_copy_fd = flags & COMMAND_FLAG_FD_MASK;
+  pid_t pid = -1;
   int r, quit, i;
   fd_set rset, rset2;
   char buf[256];
@@ -820,13 +820,6 @@ commandrvf (char **stdoutput, char **stderror, int flags,
     abort ();
   }
 
-  if (flag_copy_stdin) {
-    if (pipe (stdin_fd) == -1) {
-      error (0, errno, "pipe");
-      abort ();
-    }
-  }
-
   pid = fork ();
   if (pid == -1) {
     error (0, errno, "fork");
@@ -838,9 +831,7 @@ commandrvf (char **stdoutput, char **stderror, int flags,
     signal (SIGPIPE, SIG_DFL);
     close (0);
     if (flag_copy_stdin) {
-      dup2 (stdin_fd[PIPE_READ], STDIN_FILENO);
-      close (stdin_fd[PIPE_READ]);
-      close (stdin_fd[PIPE_WRITE]);
+      dup2 (flag_copy_fd, STDIN_FILENO);
     } else {
       /* Set stdin to /dev/null (ignore failure) */
       ignore_value (open ("/dev/null", O_RDONLY|O_CLOEXEC));
@@ -860,60 +851,6 @@ commandrvf (char **stdoutput, char **stderror, int flags,
     _exit (EXIT_FAILURE);
   }
 
-  if (flag_copy_stdin) {
-    int fd = flags & COMMAND_FLAG_FD_MASK;
-
-    stdin_pid = fork ();
-    if (stdin_pid == -1) {
-      error (0, errno, "fork");
-      abort ();
-    }
-
-    if (stdin_pid == 0) {       /* Child process copying stdin. */
-      close (so_fd[PIPE_READ]);
-      close (so_fd[PIPE_WRITE]);
-      close (se_fd[PIPE_READ]);
-      close (se_fd[PIPE_WRITE]);
-
-      close (STDOUT_FILENO);
-      dup2 (stdin_fd[PIPE_WRITE], STDOUT_FILENO);
-      close (stdin_fd[PIPE_READ]);
-      close (stdin_fd[PIPE_WRITE]);
-
-      if (chroot (sysroot) == -1) {
-        perror ("chroot");
-        _exit (EXIT_FAILURE);
-      }
-
-      ssize_t n;
-      char buffer[BUFSIZ];
-      while ((n = read (fd, buffer, sizeof buffer)) > 0) {
-        if (xwrite (STDOUT_FILENO, buffer, n) == -1)
-          /* EPIPE error indicates the command process has exited
-           * early.  If the command process fails that will be caught
-           * by the daemon, and if not, then it's not an error.
-           */
-          _exit (errno == EPIPE ? EXIT_SUCCESS : EXIT_FAILURE);
-      }
-
-      if (n == -1) {
-        perror ("read");
-        _exit (EXIT_FAILURE);
-      }
-
-      if (close (fd) == -1) {
-        perror ("close");
-        _exit (EXIT_FAILURE);
-      }
-
-      _exit (EXIT_SUCCESS);
-    }
-
-    close (fd);
-    close (stdin_fd[PIPE_READ]);
-    close (stdin_fd[PIPE_WRITE]);
-  }
-
   /* Parent process. */
   close (so_fd[PIPE_WRITE]);
   close (se_fd[PIPE_WRITE]);
@@ -950,8 +887,8 @@ commandrvf (char **stdoutput, char **stderror, int flags,
       }
       close (so_fd[PIPE_READ]);
       close (se_fd[PIPE_READ]);
+      if (flag_copy_stdin) close (flag_copy_fd);
       waitpid (pid, NULL, 0);
-      if (stdin_pid >= 0) waitpid (stdin_pid, NULL, 0);
       return -1;
     }
 
@@ -1033,21 +970,9 @@ commandrvf (char **stdoutput, char **stderror, int flags,
     }
   }
 
-  if (flag_copy_stdin) {
-    /* Check copy process didn't fail. */
-    if (waitpid (stdin_pid, &r, 0) != stdin_pid) {
-      perror ("waitpid");
-      kill (pid, 9);
-      waitpid (pid, NULL, 0);
-      return -1;
-    }
-
-    if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
-      fprintf (stderr, "failed copying from input file, see earlier messages (r = %d)\n", r);
-      kill (pid, 9);
-      waitpid (pid, NULL, 0);
-      return -1;
-    }
+  if (flag_copy_stdin && close (flag_copy_fd) == -1) {
+    perror ("close");
+    return -1;
   }
 
   /* Get the exit status of the command. */
-- 
1.7.11.7