common,kbx,tests: Clean up the PIPE function API.

* common/call-gpg.c (_gpg_encrypt, _gpg_decrypt): Simply, use
gnupg_create_pipe.
* tests/gpgscm/ffi.c (do_inbound_pipe): Likewise.
* common/exechelp.h (gnupg_create_inbound_pipe): Use gnupg_fd_t
for native pipe descriptor and don't expose other end of pipe.
(gnupg_create_outbound_pipe): Ditto.
* common/exechelp-posix.c (create_pipe_and_estream): Clean up.
(gnupg_create_inbound_pipe): Fail if R_FD or R_FP is NULL.
(gnupg_create_outbound_pipe: Ditto.
* common/exechelp-w32.c (create_pipe_and_estream): Clean up.
(gnupg_create_inbound_pipe): Fail if R_FD or R_FP is NULL.
(gnupg_create_outbound_pipe: Ditto.
(gnupg_create_pipe): Move the code from original
create_pipe_and_estream to call _open_osfhandle.
* common/exectool.c (gnupg_exec_tool_stream): Follow the change of
API.
* kbx/kbx-client-util.c (prepare_data_pipe): Likewise.

--

GnuPG-bug-id: 7194
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

common/call-gpg.c

@@ -29,6 +29,7 @@
 #include <time.h>
 
 #include "call-gpg.h"
+#include "sysutils.h"
 #include "exechelp.h"
 #include "i18n.h"
 #include "logging.h"
@@ -428,9 +429,9 @@ _gpg_encrypt (ctrl_t ctrl,
   assert ((reader_mb == NULL) != (cipher_stream == NULL));
 
   /* Create two pipes.  */
-  err = gnupg_create_outbound_pipe (outbound_fds, NULL, 0);
+  err = gnupg_create_pipe (outbound_fds);
   if (!err)
-    err = gnupg_create_inbound_pipe (inbound_fds, NULL, 0);
+    err = gnupg_create_pipe (inbound_fds);
   if (err)
     {
       log_error (_("error creating a pipe: %s\n"), gpg_strerror (err));
@@ -612,9 +613,9 @@ _gpg_decrypt (ctrl_t ctrl,
   assert ((reader_mb == NULL) != (plain_stream == NULL));
 
   /* Create two pipes.  */
-  err = gnupg_create_outbound_pipe (outbound_fds, NULL, 0);
+  err = gnupg_create_pipe (outbound_fds);
   if (!err)
-    err = gnupg_create_inbound_pipe (inbound_fds, NULL, 0);
+    err = gnupg_create_pipe (inbound_fds);
   if (err)
     {
       log_error (_("error creating a pipe: %s\n"), gpg_strerror (err));

common/exechelp-posix.c

@@ -283,24 +283,31 @@ do_create_pipe (int filedes[2])
 
 
 static gpg_error_t
-create_pipe_and_estream (int filedes[2], estream_t *r_fp,
+create_pipe_and_estream (gnupg_fd_t *r_fd, estream_t *r_fp,
                          int outbound, int nonblock)
 {
   gpg_error_t err;
+  int filedes[2];
 
   if (pipe (filedes) == -1)
     {
       err = my_error_from_syserror ();
       log_error (_("error creating a pipe: %s\n"), gpg_strerror (err));
-      filedes[0] = filedes[1] = -1;
+      *r_fd = -1;
       *r_fp = NULL;
       return err;
     }
 
   if (!outbound)
+    {
+      *r_fd = filedes[1];
       *r_fp = es_fdopen (filedes[0], nonblock? "r,nonblock" : "r");
+    }
   else
+    {
+      *r_fd = filedes[0];
       *r_fp = es_fdopen (filedes[1], nonblock? "w,nonblock" : "w");
+    }
   if (!*r_fp)
     {
       err = my_error_from_syserror ();
@@ -308,7 +315,7 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp,
                  gpg_strerror (err));
       close (filedes[0]);
       close (filedes[1]);
-      filedes[0] = filedes[1] = -1;
+      *r_fd = -1;
       return err;
     }
   return 0;
@@ -316,28 +323,28 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp,
 
 
 /* Portable function to create a pipe.  Under Windows the write end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
+   inheritable.  Pipe is created and the read end is stored at R_FD.
-   read end and stored at R_FP.  */
+   An estream is created for the write end and stored at R_FP.  */
 gpg_error_t
-gnupg_create_inbound_pipe (int filedes[2], estream_t *r_fp, int nonblock)
+gnupg_create_inbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock)
 {
-  if (r_fp)
+  if (!r_fd || !r_fp)
-    return create_pipe_and_estream (filedes, r_fp, 0, nonblock);
+    gpg_error (GPG_ERR_INV_ARG);
-  else
+
-    return do_create_pipe (filedes);
+  return create_pipe_and_estream (r_fd, r_fp, 0, nonblock);
 }
 
 
 /* Portable function to create a pipe.  Under Windows the read end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
+   inheritable.  Pipe is created and the write end is stored at R_FD.
-   write end and stored at R_FP.  */
+   An estream is created for the write end and stored at R_FP.  */
 gpg_error_t
-gnupg_create_outbound_pipe (int filedes[2], estream_t *r_fp, int nonblock)
+gnupg_create_outbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock)
 {
-  if (r_fp)
+  if (!r_fd || !r_fp)
-    return create_pipe_and_estream (filedes, r_fp, 1, nonblock);
+    gpg_error (GPG_ERR_INV_ARG);
-  else
+
-    return do_create_pipe (filedes);
+  return create_pipe_and_estream (r_fd, r_fp, 1, nonblock);
 }
 
 

common/exechelp-w32.c

@@ -223,22 +223,94 @@ create_inheritable_pipe (HANDLE filedes[2], int flags)
 
 
 static gpg_error_t
-create_pipe_and_estream (int filedes[2], int flags,
+create_pipe_and_estream (gnupg_fd_t *r_fd, int flags,
                          estream_t *r_fp, int outbound, int nonblock)
 {
   gpg_error_t err = 0;
-  HANDLE fds[2];
   es_syshd_t syshd;
+  gnupg_fd_t fds[2];
 
-  filedes[0] = filedes[1] = -1;
+  if (create_inheritable_pipe (fds, flags) < 0)
-  err = my_error (GPG_ERR_GENERAL);
-  if (!create_inheritable_pipe (fds, flags))
     {
+      err = my_error_from_syserror ();
+      log_error (_("error creating a pipe: %s\n"), gpg_strerror (err));
+      *r_fd = GNUPG_INVALID_FD;
+      *r_fp = NULL;
+      return err;
+    }
+
+  syshd.type = ES_SYSHD_HANDLE;
+  if (!outbound)
+    {
+      syshd.u.handle = fds[0];
+      *r_fd = fds[1];
+      *r_fp = es_sysopen (&syshd, nonblock? "r,nonblock" : "r");
+    }
+  else
+    {
+      syshd.u.handle = fds[1];
+      *r_fd = fds[0];
+      *r_fp = es_sysopen (&syshd, nonblock? "w,nonblock" : "w");
+    }
+  if (!*r_fp)
+    {
+      err = my_error_from_syserror ();
+      log_error (_("error creating a stream for a pipe: %s\n"),
+                 gpg_strerror (err));
+      CloseHandle (fds[0]);
+      CloseHandle (fds[1]);
+      *r_fd = GNUPG_INVALID_FD;
+      return err;
+    }
+
+  return 0;
+}
+
+/* Portable function to create a pipe.  Under Windows the write end is
+   inheritable.  Pipe is created and the read end is stored at R_FD.
+   An estream is created for the write end and stored at R_FP.  */
+gpg_error_t
+gnupg_create_inbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock)
+{
+  if (!r_fd || !r_fp)
+    gpg_error (GPG_ERR_INV_ARG);
+
+  return create_pipe_and_estream (r_fd, INHERIT_WRITE, r_fp, 0, nonblock);
+}
+
+
+/* Portable function to create a pipe.  Under Windows the read end is
+   inheritable.  Pipe is created and the write end is stored at R_FD.
+   An estream is created for the write end and stored at R_FP.  */
+gpg_error_t
+gnupg_create_outbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock)
+{
+  if (!r_fd || !r_fp)
+    gpg_error (GPG_ERR_INV_ARG);
+
+  return create_pipe_and_estream (r_fd, INHERIT_READ, r_fp, 1, nonblock);
+}
+
+
+/* Portable function to create a pipe.  Under Windows both ends are
+   inheritable.  */
+gpg_error_t
+gnupg_create_pipe (int filedes[2])
+{
+  gnupg_fd_t fds[2];
+  gpg_error_t err = 0;
+
+  if (create_inheritable_pipe (fds, INHERIT_BOTH) < 0)
+    return my_error_from_syserror ();
+
   filedes[0] = _open_osfhandle (handle_to_fd (fds[0]), O_RDONLY);
   if (filedes[0] == -1)
     {
       log_error ("failed to translate osfhandle %p\n", fds[0]);
+      CloseHandle (fds[0]);
       CloseHandle (fds[1]);
+      filedes[1] = -1;
+      err = my_error (GPG_ERR_GENERAL);
     }
   else
     {
@@ -249,71 +321,15 @@ create_pipe_and_estream (int filedes[2], int flags,
           close (filedes[0]);
           filedes[0] = -1;
           CloseHandle (fds[1]);
+          err = my_error (GPG_ERR_GENERAL);
         }
       else
         err = 0;
     }
-    }
-
-  if (! err && r_fp)
-    {
-      syshd.type = ES_SYSHD_HANDLE;
-      if (!outbound)
-        {
-          syshd.u.handle = fds[0];
-          *r_fp = es_sysopen (&syshd, nonblock? "r,nonblock" : "r");
-        }
-      else
-        {
-          syshd.u.handle = fds[1];
-          *r_fp = es_sysopen (&syshd, nonblock? "w,nonblock" : "w");
-        }
-      if (!*r_fp)
-        {
-          err = my_error_from_syserror ();
-          log_error (_("error creating a stream for a pipe: %s\n"),
-                     gpg_strerror (err));
-          close (filedes[0]);
-          close (filedes[1]);
-          filedes[0] = filedes[1] = -1;
-          return err;
-        }
-    }
 
   return err;
 }
 
-/* Portable function to create a pipe.  Under Windows the write end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
-   read end and stored at R_FP.  */
-gpg_error_t
-gnupg_create_inbound_pipe (int filedes[2], estream_t *r_fp, int nonblock)
-{
-  return create_pipe_and_estream (filedes, INHERIT_WRITE,
-                                  r_fp, 0, nonblock);
-}
-
-
-/* Portable function to create a pipe.  Under Windows the read end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
-   write end and stored at R_FP.  */
-gpg_error_t
-gnupg_create_outbound_pipe (int filedes[2], estream_t *r_fp, int nonblock)
-{
-  return create_pipe_and_estream (filedes, INHERIT_READ,
-                                  r_fp, 1, nonblock);
-}
-
-
-/* Portable function to create a pipe.  Under Windows both ends are
-   inheritable.  */
-gpg_error_t
-gnupg_create_pipe (int filedes[2])
-{
-  return create_pipe_and_estream (filedes, INHERIT_BOTH,
-                                  NULL, 0, 0);
-}
-
 
 /* Close the end of a pipe.  */
 void

common/exechelp.h

@@ -54,15 +54,15 @@ int *get_all_open_fds (void);
 
 
 /* Portable function to create a pipe.  Under Windows the write end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
+   inheritable.  Pipe is created and the read end is stored at R_FD.
-   write end and stored at R_FP.  */
+   An estream is created for the write end and stored at R_FP.  */
-gpg_error_t gnupg_create_inbound_pipe (int filedes[2],
+gpg_error_t gnupg_create_inbound_pipe (gnupg_fd_t *r_fd,
                                        estream_t *r_fp, int nonblock);
 
 /* Portable function to create a pipe.  Under Windows the read end is
-   inheritable.  If R_FP is not NULL, an estream is created for the
+   inheritable.  Pipe is created and the write end is stored at R_FD.
-   write end and stored at R_FP.  */
+   An estream is created for the write end and stored at R_FP.  */
-gpg_error_t gnupg_create_outbound_pipe (int filedes[2],
+gpg_error_t gnupg_create_outbound_pipe (gnupg_fd_t *r_fd,
                                         estream_t *r_fp, int nonblock);
 
 /* Portable function to create a pipe.  Under Windows both ends are

common/exectool.c

@@ -331,7 +331,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[],
 #else
   int exceptclose[2];
 #endif
-  int extrapipe[2] = {-1, -1};
+  gnupg_fd_t extrapipe;
   char extrafdbuf[20];
   const char *argsave = NULL;
   int argsaveidx;
@@ -387,7 +387,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[],
 
   if (inextra)
     {
-      err = gnupg_create_outbound_pipe (extrapipe, &extrafp, 1);
+      err = gnupg_create_outbound_pipe (&extrapipe, &extrafp, 1);
       if (err)
         {
           log_error ("error creating outbound pipe for extra fp: %s\n",
@@ -395,11 +395,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[],
           goto leave;
         }
       /* Do not close in child. */
-#ifdef HAVE_W32_SYSTEM
+      exceptclose[i] = extrapipe;
-      exceptclose[i] = (HANDLE)_get_osfhandle (extrapipe[0]);
-#else
-      exceptclose[i] = extrapipe[0];
-#endif
       /* Now find the argument marker and replace by the pipe's fd.
          Yeah, that is an ugly non-thread safe hack but it safes us to
          create a copy of the array.  */
@@ -424,11 +420,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[],
       i++;
     }
 
-#ifdef HAVE_W32_SYSTEM
+  exceptclose[i] = GNUPG_INVALID_FD;
-    exceptclose[i] = INVALID_HANDLE_VALUE;
-#else
-    exceptclose[i] = -1;
-#endif
 
   err = gpgrt_spawn_actions_new (&act);
   if (err)
@@ -447,8 +439,12 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[],
                               | GPGRT_PROCESS_STDERR_PIPE), act, &proc);
   gpgrt_process_get_streams (proc, GPGRT_PROCESS_STREAM_NONBLOCK,
                              input? &infp : NULL, &outfp, &errfp);
-  if (extrapipe[0] != -1)
+  if (extrapipe != GNUPG_INVALID_FD)
-    close (extrapipe[0]);
+#ifdef HAVE_W32_SYSTEM
+    CloseHandle (extrapipe);
+#else
+    close (extrapipe);
+#endif
   if (argsave)
     argv[argsaveidx] = argsave;
   if (err)

kbx/kbx-client-util.c

@@ -30,6 +30,7 @@
 #include "../common/membuf.h"
 #include "../common/i18n.h"
 #include "../common/asshelp.h"
+#include "../common/sysutils.h"
 #include "../common/exechelp.h"
 #include "../common/sysutils.h"
 #include "../common/host2net.h"
@@ -102,7 +103,7 @@ prepare_data_pipe (kbx_client_data_t kcd)
 {
   gpg_error_t err;
   int rc;
-  int inpipe[2];
+  gnupg_fd_t inpipe;
   estream_t infp;
   npth_attr_t tattr;
 
@@ -111,24 +112,29 @@ prepare_data_pipe (kbx_client_data_t kcd)
   kcd->datalen = 0;
   kcd->dataerr = 0;
 
-  err = gnupg_create_inbound_pipe (inpipe, &infp, 0);
+  err = gnupg_create_inbound_pipe (&inpipe, &infp, 0);
   if (err)
     {
       log_error ("error creating inbound pipe: %s\n", gpg_strerror (err));
       return err;  /* That should not happen.  */
     }
 
-#ifdef HAVE_W32_SYSTEM
+  err = assuan_sendfd (kcd->ctx, inpipe);
-  err = assuan_sendfd (kcd->ctx, (HANDLE)_get_osfhandle (inpipe[1]));
-#else
-  err = assuan_sendfd (kcd->ctx, inpipe[1]);
-#endif
   if (err)
     {
+#ifdef HAVE_W32_SYSTEM
+      log_error ("sending fd %p to keyboxd: %s <%s>\n",
+                 inpipe, gpg_strerror (err), gpg_strsource (err));
+#else
       log_error ("sending fd %d to keyboxd: %s <%s>\n",
-                 inpipe[1], gpg_strerror (err), gpg_strsource (err));
+                 inpipe, gpg_strerror (err), gpg_strsource (err));
+#endif
       es_fclose (infp);
-      gnupg_close_pipe (inpipe[1]);
+#ifdef HAVE_W32_SYSTEM
+      CloseHandle (inpipe);
+#else
+      close (inpipe);
+#endif
       return err;
     }
 
@@ -142,7 +148,11 @@ prepare_data_pipe (kbx_client_data_t kcd)
       return err;
     }
 
-  close (inpipe[1]);
+#ifdef HAVE_W32_SYSTEM
+  CloseHandle (inpipe);
+#else
+  close (inpipe);
+#endif
   kcd->fp = infp;
 
   rc = npth_attr_init (&tattr);

tests/gpgscm/ffi.c

@@ -1241,7 +1241,7 @@ do_inbound_pipe (scheme *sc, pointer args)
   FFI_PROLOG ();
   int filedes[2];
   FFI_ARGS_DONE_OR_RETURN (sc, args);
-  err = gnupg_create_inbound_pipe (filedes, NULL, 0);
+  err = gnupg_create_pipe (filedes);
 #define IMC(A, B)                                                       \
   _cons (sc, sc->vptr->mk_integer (sc, (unsigned long) (A)), (B), 1)
   FFI_RETURN_POINTER (sc, IMC (filedes[0],
@@ -1255,7 +1255,7 @@ do_outbound_pipe (scheme *sc, pointer args)
   FFI_PROLOG ();
   int filedes[2];
   FFI_ARGS_DONE_OR_RETURN (sc, args);
-  err = gnupg_create_outbound_pipe (filedes, NULL, 0);
+  err = gnupg_create_pipe (filedes);
 #define IMC(A, B)                                                       \
   _cons (sc, sc->vptr->mk_integer (sc, (unsigned long) (A)), (B), 1)
   FFI_RETURN_POINTER (sc, IMC (filedes[0],