common: Change calling convention for gnupg_spawn_process.
authorWerner Koch <wk@gnupg.org>
Mon, 28 Sep 2015 16:10:21 +0000 (18:10 +0200)
committerWerner Koch <wk@gnupg.org>
Mon, 28 Sep 2015 16:40:38 +0000 (18:40 +0200)
* common/exechelp.h (GNUPG_SPAWN_NONBLOCK): New.
(GNUPG_SPAWN_RUN_ASFW, GNUPG_SPAWN_DETACHED): Macro to replace the
numbers.
* common/exechelp.h (gnupg_spawn_process): Change function to not take
an optional stream for stdin but to return one.
* common/exechelp-posix.c (gnupg_spawn_process): Implement change.
(create_pipe_and_estream): Add args outbound and nonblock.
* common/exechelp-w32.c (gnupg_spawn_process): Implement change.
--

In 2.1 this function is only used at one place and the stdin parameter
is not used.  Thus this change is trivial for the callers but along
with estream's new es_poll it is overall simpler to use.

Note that the Windows version has not been tested.

Signed-off-by: Werner Koch <wk@gnupg.org>
common/exechelp-posix.c
common/exechelp-w32.c
common/exechelp-w32ce.c
common/exechelp.h

index 8a2b3b9..2bf2592 100644 (file)
@@ -313,6 +313,7 @@ gnupg_create_outbound_pipe (int filedes[2])
 
 static gpg_error_t
 create_pipe_and_estream (int filedes[2], estream_t *r_fp,
+                         int outbound, int nonblock,
                          gpg_err_source_t errsource)
 {
   gpg_error_t err;
@@ -326,7 +327,10 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp,
       return err;
     }
 
-  *r_fp = es_fdopen (filedes[0], "r");
+  if (outbound)
+    *r_fp = es_fdopen (filedes[0], nonblock? "r,nonblock" : "r");
+  else
+    *r_fp = es_fdopen (filedes[1], nonblock? "w,nonblock" : "w");
   if (!*r_fp)
     {
       err = gpg_err_make (errsource, gpg_err_code_from_syserror ());
@@ -347,53 +351,70 @@ gpg_error_t
 gnupg_spawn_process (const char *pgmname, const char *argv[],
                      gpg_err_source_t errsource,
                      void (*preexec)(void), unsigned int flags,
-                     estream_t infp,
+                     estream_t *r_infp,
                      estream_t *r_outfp,
                      estream_t *r_errfp,
                      pid_t *pid)
 {
   gpg_error_t err;
-  int infd = -1;
+  int inpipe[2] = {-1, -1};
   int outpipe[2] = {-1, -1};
   int errpipe[2] = {-1, -1};
+  estream_t infp = NULL;
   estream_t outfp = NULL;
   estream_t errfp = NULL;
+  int nonblock = !!(flags & GNUPG_SPAWN_NONBLOCK);
 
-  (void)flags; /* Currently not used.  */
-
+  if (r_infp)
+    *r_infp = NULL;
   if (r_outfp)
     *r_outfp = NULL;
   if (r_errfp)
     *r_errfp = NULL;
   *pid = (pid_t)(-1); /* Always required.  */
 
-  if (infp)
+  if (r_infp)
     {
-      es_fflush (infp);
-      es_rewind (infp);
-      infd = es_fileno (infp);
-      if (infd == -1)
-        return gpg_err_make (errsource, GPG_ERR_INV_VALUE);
+      err = create_pipe_and_estream (inpipe, &infp, 0, nonblock, errsource);
+      if (err)
+        return err;
     }
 
   if (r_outfp)
     {
-      err = create_pipe_and_estream (outpipe, &outfp, errsource);
+      err = create_pipe_and_estream (outpipe, &outfp, 1, nonblock, errsource);
       if (err)
-        return err;
+        {
+          if (infp)
+            es_fclose (infp);
+          else if (inpipe[1] != -1)
+            close (inpipe[1]);
+          if (inpipe[0] != -1)
+            close (inpipe[0]);
+
+          return err;
+        }
     }
 
   if (r_errfp)
     {
-      err = create_pipe_and_estream (errpipe, &errfp, errsource);
+      err = create_pipe_and_estream (errpipe, &errfp, 1, nonblock, errsource);
       if (err)
         {
+          if (infp)
+            es_fclose (infp);
+          else if (inpipe[1] != -1)
+            close (inpipe[1]);
+          if (inpipe[0] != -1)
+            close (inpipe[0]);
+
           if (outfp)
             es_fclose (outfp);
           else if (outpipe[0] != -1)
             close (outpipe[0]);
           if (outpipe[1] != -1)
             close (outpipe[1]);
+
           return err;
         }
     }
@@ -405,6 +426,13 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
       err = gpg_err_make (errsource, gpg_err_code_from_syserror ());
       log_error (_("error forking process: %s\n"), gpg_strerror (err));
 
+      if (infp)
+        es_fclose (infp);
+      else if (inpipe[1] != -1)
+        close (inpipe[1]);
+      if (inpipe[0] != -1)
+        close (inpipe[0]);
+
       if (outfp)
         es_fclose (outfp);
       else if (outpipe[0] != -1)
@@ -427,16 +455,20 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
       gcry_control (GCRYCTL_TERM_SECMEM);
       es_fclose (outfp);
       es_fclose (errfp);
-      do_exec (pgmname, argv, infd, outpipe[1], errpipe[1], preexec);
+      do_exec (pgmname, argv, inpipe[0], outpipe[1], errpipe[1], preexec);
       /*NOTREACHED*/
     }
 
   /* This is the parent. */
+  if (inpipe[0] != -1)
+    close (inpipe[0]);
   if (outpipe[1] != -1)
     close (outpipe[1]);
   if (errpipe[1] != -1)
     close (errpipe[1]);
 
+  if (r_infp)
+    *r_infp = infp;
   if (r_outfp)
     *r_outfp = outfp;
   if (r_errfp)
index 05e9e10..ba3b357 100644 (file)
@@ -346,7 +346,7 @@ gpg_error_t
 gnupg_spawn_process (const char *pgmname, const char *argv[],
                      gpg_err_source_t errsource,
                      void (*preexec)(void), unsigned int flags,
-                     estream_t infp,
+                     estream_t *r_infp,
                      estream_t *r_outfp,
                      estream_t *r_errfp,
                      pid_t *pid)
@@ -363,9 +363,10 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
   STARTUPINFO si;
   int cr_flags;
   char *cmdline;
-  HANDLE inhandle = INVALID_HANDLE_VALUE;
+  HANDLE inpipe[2]  = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE};
   HANDLE outpipe[2] = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE};
   HANDLE errpipe[2] = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE};
+  estream_t infp = NULL;
   estream_t outfp = NULL;
   estream_t errfp = NULL;
   HANDLE nullhd[3] = {INVALID_HANDLE_VALUE,
@@ -374,6 +375,8 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
   int i;
   es_syshd_t syshd;
 
+  if (r_infp)
+    *r_infp = NULL;
   if (r_outfp)
     *r_outfp = NULL;
   if (r_errfp)
@@ -382,29 +385,26 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
 
   if (infp)
     {
-      es_fflush (infp);
-      es_rewind (infp);
-      es_syshd (infp, &syshd);
-      switch (syshd.type)
+      if (create_inheritable_pipe (inpipe, 0))
         {
-        case ES_SYSHD_FD:
-          inhandle = (HANDLE)_get_osfhandle (syshd.u.fd);
-          break;
-        case ES_SYSHD_SOCK:
-          inhandle = (HANDLE)_get_osfhandle (syshd.u.sock);
-          break;
-        case ES_SYSHD_HANDLE:
-          inhandle = syshd.u.handle;
-          break;
-        default:
-          inhandle = INVALID_HANDLE_VALUE;
-          break;
+          err = gpg_err_make (errsource, GPG_ERR_GENERAL);
+          log_error (_("error creating a pipe: %s\n"), gpg_strerror (err));
+          return err;
+        }
+
+      syshd.type = ES_SYSHD_HANDLE;
+      syshd.u.handle = inpipe[1];
+      infp = es_sysopen (&syshd, "w");
+      if (!infp)
+        {
+          err = gpg_err_make (errsource, gpg_err_code_from_syserror ());
+          log_error (_("error creating a stream for a pipe: %s\n"),
+                     gpg_strerror (err));
+          CloseHandle (inpipe[0]);
+          CloseHandle (inpipe[1]);
+          inpipe[0] = inpipe[1] = INVALID_HANDLE_VALUE;
+          return err;
         }
-      if (inhandle == INVALID_HANDLE_VALUE)
-        return gpg_err_make (errsource, GPG_ERR_INV_VALUE);
-      /* FIXME: In case we can't get a system handle (e.g. due to
-         es_fopencookie we should create a piper and a feeder
-         thread.  */
     }
 
   if (r_outfp)
@@ -427,6 +427,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
           CloseHandle (outpipe[0]);
           CloseHandle (outpipe[1]);
           outpipe[0] = outpipe[1] = INVALID_HANDLE_VALUE;
+          if (infp)
+            es_fclose (infp);
+          else if (inpipe[1] != INVALID_HANDLE_VALUE)
+            CloseHandle (outpipe[1]);
+          if (inpipe[0] != INVALID_HANDLE_VALUE)
+            CloseHandle (inpipe[0]);
           return err;
         }
     }
@@ -457,6 +463,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
             CloseHandle (outpipe[0]);
           if (outpipe[1] != INVALID_HANDLE_VALUE)
             CloseHandle (outpipe[1]);
+          if (infp)
+            es_fclose (infp);
+          else if (inpipe[1] != INVALID_HANDLE_VALUE)
+            CloseHandle (outpipe[1]);
+          if (inpipe[0] != INVALID_HANDLE_VALUE)
+            CloseHandle (inpipe[0]);
           return err;
         }
     }
@@ -471,7 +483,7 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
   if (err)
     return err;
 
-  if (inhandle != INVALID_HANDLE_VALUE)
+  if (inpipe[0] != INVALID_HANDLE_VALUE)
     nullhd[0] = w32_open_null (0);
   if (outpipe[1] != INVALID_HANDLE_VALUE)
     nullhd[1] = w32_open_null (0);
@@ -486,12 +498,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
   si.cb = sizeof (si);
   si.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
   si.wShowWindow = DEBUG_W32_SPAWN? SW_SHOW : SW_MINIMIZE;
-  si.hStdInput  =   inhandle == INVALID_HANDLE_VALUE? nullhd[0] : inhandle;
+  si.hStdInput  = inpipe[0]  == INVALID_HANDLE_VALUE? nullhd[0] : inpipe[0];
   si.hStdOutput = outpipe[1] == INVALID_HANDLE_VALUE? nullhd[1] : outpipe[1];
   si.hStdError  = errpipe[1] == INVALID_HANDLE_VALUE? nullhd[2] : errpipe[1];
 
   cr_flags = (CREATE_DEFAULT_ERROR_MODE
-              | ((flags & 128)? DETACHED_PROCESS : 0)
+              | ((flags & GNUPG_SPAWN_DETACHED)? DETACHED_PROCESS : 0)
               | GetPriorityClass (GetCurrentProcess ())
               | CREATE_SUSPENDED);
 /*   log_debug ("CreateProcess, path='%s' cmdline='%s'\n", pgmname, cmdline); */
@@ -509,6 +521,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
     {
       log_error ("CreateProcess failed: %s\n", w32_strerror (-1));
       xfree (cmdline);
+      if (infp)
+        es_fclose (infp);
+      else if (inpipe[1] != INVALID_HANDLE_VALUE)
+        CloseHandle (outpipe[1]);
+      if (inpipe[0] != INVALID_HANDLE_VALUE)
+        CloseHandle (inpipe[0]);
       if (outfp)
         es_fclose (outfp);
       else if (outpipe[0] != INVALID_HANDLE_VALUE)
@@ -532,6 +550,8 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
       CloseHandle (nullhd[i]);
 
   /* Close the inherited ends of the pipes.  */
+  if (inpipe[0] != INVALID_HANDLE_VALUE)
+    CloseHandle (inpipe[0]);
   if (outpipe[1] != INVALID_HANDLE_VALUE)
     CloseHandle (outpipe[1]);
   if (errpipe[1] != INVALID_HANDLE_VALUE)
@@ -546,13 +566,15 @@ gnupg_spawn_process (const char *pgmname, const char *argv[],
   /* Fixme: For unknown reasons AllowSetForegroundWindow returns an
      invalid argument error if we pass it the correct processID.  As a
      workaround we use -1 (ASFW_ANY).  */
-  if ( (flags & 64) )
+  if ((flags & GNUPG_SPAWN_RUN_ASFW))
     gnupg_allow_set_foregound_window ((pid_t)(-1)/*pi.dwProcessId*/);
 
   /* Process has been created suspended; resume it now. */
   ResumeThread (pi.hThread);
   CloseHandle (pi.hThread);
 
+  if (r_infp)
+    *r_infp = infp;
   if (r_outfp)
     *r_outfp = outfp;
   if (r_errfp)
index cca55c8..975386a 100644 (file)
@@ -502,7 +502,7 @@ gpg_error_t
 gnupg_spawn_process (const char *pgmname, const char *argv[],
                      gpg_err_source_t errsource,
                      void (*preexec)(void), unsigned int flags,
-                     estream_t infp,
+                     estream_t *r_infp,
                      estream_t *r_outfp,
                      estream_t *r_errfp,
                      pid_t *pid)
index a9a9ca3..a146d89 100644 (file)
@@ -59,17 +59,22 @@ gpg_error_t gnupg_create_inbound_pipe (int filedes[2]);
    inheritable.  */
 gpg_error_t gnupg_create_outbound_pipe (int filedes[2]);
 
+#define GNUPG_SPAWN_NONBLOCK   16
+#define GNUPG_SPAWN_RUN_ASFW   64
+#define GNUPG_SPAWN_DETACHED  128
 
-/* Fork and exec the PGMNAME.  If INFP is NULL connect /dev/null to
-   stdin of the new process; if it is not NULL connect the file
-   descriptor retrieved from INFP to stdin.  If R_OUTFP is NULL
-   connect stdout of the new process to /dev/null; if it is not NULL
-   store the address of a pointer to a new estream there.  If R_ERRFP
-   is NULL connect stderr of the new process to /dev/null; if it is
-   not NULL store the address of a pointer to a new estream there.  On
-   success the pid of the new process is stored at PID.  On error -1
-   is stored at PID and if R_OUTFP or R_ERRFP are not NULL, NULL is
-   stored there.
+
+/* Fork and exec the program PGMNAME.
+
+   If R_INFP is NULL connect stdin of the new process to /dev/null; if
+   it is not NULL store the address of a pointer to a new estream
+   there. If R_OUTFP is NULL connect stdout of the new process to
+   /dev/null; if it is not NULL store the address of a pointer to a
+   new estream there.  If R_ERRFP is NULL connect stderr of the new
+   process to /dev/null; if it is not NULL store the address of a
+   pointer to a new estream there.  On success the pid of the new
+   process is stored at PID.  On error -1 is stored at PID and if
+   R_OUTFP or R_ERRFP are not NULL, NULL is stored there.
 
    The arguments for the process are expected in the NULL terminated
    array ARGV.  The program name itself should not be included there.
@@ -81,12 +86,21 @@ gpg_error_t gnupg_create_outbound_pipe (int filedes[2]);
 
    FLAGS is a bit vector:
 
-   Bit 7: If set the process will be started as a background process.
+   GNUPG_SPAWN_NONBLOCK
+          If set the two output streams are created in non-blocking
+          mode and the input stream is switched to non-blocking mode.
+          This is merely a convenience feature because the caller
+          could do the same with gpgrt_set_nonblock.  Does not yet
+          work for Windows.
+
+   GNUPG_SPAWN_DETACHED
+          If set the process will be started as a background process.
           This flag is only useful under W32 (but not W32CE) systems,
           so that no new console is created and pops up a console
           window when starting the server.  Does not work on W32CE.
 
-   Bit 6: On W32 (but not on W32CE) run AllowSetForegroundWindow for
+   GNUPG_SPAWN_RUN_ASFW
+          On W32 (but not on W32CE) run AllowSetForegroundWindow for
           the child.  Note that due to unknown problems this actually
           allows SetForegroundWindow for all childs of this process.
 
@@ -95,7 +109,7 @@ gpg_error_t
 gnupg_spawn_process (const char *pgmname, const char *argv[],
                      gpg_err_source_t errsource,
                      void (*preexec)(void), unsigned int flags,
-                     estream_t infp,
+                     estream_t *r_infp,
                      estream_t *r_outfp,
                      estream_t *r_errfp,
                      pid_t *pid);