Add fdopen_pipe()

5 files changed, 230 insertions, 132 deletions

diff --git a/butl/fdstream b/butl/fdstream
index 3fb52f7..3ed104f 100644
--- a/butl/fdstream
+++ b/butl/fdstream
@@ -30,7 +30,7 @@ namespace butl
   class LIBBUTL_EXPORT auto_fd
   {
   public:
-    auto_fd (nullfd_t = nullfd): fd_ (-1) {}
+    auto_fd (nullfd_t = nullfd) noexcept: fd_ (-1) {}
 
     explicit
     auto_fd (int fd) noexcept: fd_ (fd) {}
@@ -41,17 +41,22 @@ namespace butl
     auto_fd (const auto_fd&) = delete;
     auto_fd& operator= (const auto_fd&) = delete;
 
-    ~auto_fd () noexcept {reset ();}
+    ~auto_fd () noexcept;
 
     int
     get () const noexcept {return fd_;}
 
-    int
-    release () noexcept;
-
     void
     reset (int fd = -1) noexcept;
 
+    int
+    release () noexcept
+    {
+      int r (fd_);
+      fd_ = -1;
+      return r;
+    }
+
     // Close an open file descriptor. Throw ios::failure on the underlying OS
     // error. Reset the descriptor to -1 whether the exception is thrown or
     // not.
@@ -151,7 +156,10 @@ namespace butl
   //
   // The text/binary flags have the same semantics as those in std::fstream.
   // Specifically, this is a noop for POSIX systems where the two modes are
-  // the same.
+  // the same. On Windows, when reading in the text mode the sequence of 0xD,
+  // 0xA characters is translated into the single OxA character and 0x1A is
+  // interpreted as EOF. When writing in the text mode the OxA character is
+  // translated into the 0xD, 0xA sequence.
   //
   // The skip flag instructs the stream to skip to the end before closing the
   // file descriptor. This is primarily useful when working with pipes where
@@ -505,8 +513,17 @@ namespace butl
   LIBBUTL_EXPORT auto_fd
   fddup (int fd);
 
-  // Set the translation mode for the file descriptor. Return the previous
-  // mode on success, throw ios::failure otherwise.
+  // Set the translation mode for the file descriptor. Throw invalid_argument
+  // for an invalid combination of flags. Return the previous mode on success,
+  // throw ios::failure otherwise.
+  //
+  // The text and binary flags are mutually exclusive on Windows. Due to
+  // implementation details at least one of them should be specified. On POSIX
+  // system the two modes are the same and so no check is performed.
+  //
+  // The blocking and non-blocking flags are mutually exclusive on POSIX
+  // system. Non-blocking mode is not supported on Windows and so the blocking
+  // mode is assumed regardless of the flags.
   //
   LIBBUTL_EXPORT fdstream_mode
   fdmode (int, fdstream_mode);
@@ -558,6 +575,24 @@ namespace butl
   LIBBUTL_EXPORT int
   fdnull (bool temp = false) noexcept;
 #endif
+
+  struct fdpipe
+  {
+    auto_fd in;
+    auto_fd out;
+  };
+
+  // Create a pipe. Throw ios::failure on the underlying OS error. By default
+  // both ends of the pipe are opened in the text mode. Pass the binary flag
+  // to instead open them in the binary mode. Passing a mode other than none
+  // or binary is illegal.
+  //
+  // Note that on Windows both ends of the created pipe are not inheritable.
+  // In particular, the process class that uses fdpipe underneath makes the
+  // appropriate end of pipe (the one being passed to the child) inheritable.
+  //
+  LIBBUTL_EXPORT fdpipe
+  fdopen_pipe (fdopen_mode = fdopen_mode::none);
 }
 
 #include <butl/fdstream.ixx>
diff --git a/butl/fdstream.cxx b/butl/fdstream.cxx
index 16801ed..6ebfaf2 100644
--- a/butl/fdstream.cxx
+++ b/butl/fdstream.cxx
@@ -6,14 +6,14 @@
 
 #ifndef _WIN32
 #  include <fcntl.h>     // open(), O_*, fcntl()
-#  include <unistd.h>    // close(), read(), write(), lseek(), dup(), ssize_t,
-                         // STD*_FILENO
+#  include <unistd.h>    // close(), read(), write(), lseek(), dup(), pipe(),
+                         // ssize_t, STD*_FILENO
 #  include <sys/uio.h>   // writev(), iovec
 #  include <sys/stat.h>  // S_I*
 #  include <sys/types.h> // off_t
 #else
 #  include <io.h>       // _close(), _read(), _write(), _setmode(), _sopen(),
-                        // _lseek(), _dup()
+                        // _lseek(), _dup(), _pipe()
 #  include <share.h>    // _SH_DENYNO
 #  include <stdio.h>    // _fileno(), stdin, stdout, stderr
 #  include <fcntl.h>    // _O_*
@@ -816,6 +816,18 @@ namespace butl
     return fdmode (STDERR_FILENO, m);
   }
 
+  fdpipe
+  fdopen_pipe (fdopen_mode m)
+  {
+    assert (m == fdopen_mode::none || m == fdopen_mode::binary);
+
+    int pd[2];
+    if (pipe (pd) == -1)
+      throw_ios_failure (errno);
+
+    return {auto_fd (pd[0]), auto_fd (pd[1])};
+  }
+
 #else
 
   bool
@@ -914,5 +926,19 @@ namespace butl
     return fdmode (fd, m);
   }
 
+  fdpipe
+  fdopen_pipe (fdopen_mode m)
+  {
+    assert (m == fdopen_mode::none || m == fdopen_mode::binary);
+
+    int pd[2];
+    if (_pipe (
+          pd,
+          64 * 1024, // Set buffer size to 64K.
+          _O_NOINHERIT | (m == fdopen_mode::none ? _O_TEXT : _O_BINARY)) == -1)
+      throw_ios_failure (errno);
+
+    return {auto_fd (pd[0]), auto_fd (pd[1])};
+  }
 #endif
 }
diff --git a/butl/fdstream.ixx b/butl/fdstream.ixx
index 5d38d36..e511718 100644
--- a/butl/fdstream.ixx
+++ b/butl/fdstream.ixx
@@ -8,6 +8,15 @@ namespace butl
 {
   // auto_fd
   //
+  inline void auto_fd::
+  reset (int fd) noexcept
+  {
+    if (fd_ >= 0)
+      fdclose (fd_); // Don't check for an error as not much we can do here.
+
+    fd_ = fd;
+  }
+
   inline auto_fd& auto_fd::
   operator= (auto_fd&& fd) noexcept
   {
@@ -15,21 +24,10 @@ namespace butl
     return *this;
   }
 
-  inline int auto_fd::
-  release () noexcept
+  inline auto_fd::
+  ~auto_fd () noexcept
   {
-    int r (fd_);
-    fd_ = -1;
-    return r;
-  }
-
-  inline void auto_fd::
-  reset (int fd) noexcept
-  {
-    if (fd_ >= 0)
-      fdclose (fd_); // Don't check for an error as not much we can do here.
-
-    fd_ = fd;
+    reset ();
   }
 
   // ifdstream
diff --git a/butl/process.cxx b/butl/process.cxx
index 6b515b7..41645e4 100644
--- a/butl/process.cxx
+++ b/butl/process.cxx
@@ -12,8 +12,7 @@
 #else
 #  include <butl/win32-utility>
 
-#  include <io.h>        // _open_osfhandle(), _get_osfhandle(), _close()
-#  include <fcntl.h>     // _O_TEXT
+#  include <io.h>        // _get_osfhandle(), _close()
 #  include <stdlib.h>    // _MAX_PATH, getenv()
 #  include <sys/types.h> // stat
 #  include <sys/stat.h>  // stat(), S_IS*
@@ -34,6 +33,7 @@
 
 #include <errno.h>
 
+#include <ios>     // ios_base::failure
 #include <cassert>
 #include <cstddef> // size_t
 #include <cstring> // strlen(), strchr()
@@ -227,49 +227,56 @@ namespace butl
            const process_path& pp, const char* args[],
            int in, int out, int err)
   {
-    using pipe = auto_fd[2];
-
-    pipe out_fd;
-    pipe in_ofd;
-    pipe in_efd;
+    fdpipe out_fd;
+    fdpipe in_ofd;
+    fdpipe in_efd;
 
     auto fail = [](bool child) {throw process_error (errno, child);};
 
-    auto create_pipe = [&fail](pipe& p)
+    auto open_pipe = [] () -> fdpipe
     {
-      int pd[2];
-      if (::pipe (pd) == -1)
-        fail (false);
-
-      p[0].reset (pd[0]);
-      p[1].reset (pd[1]);
+      try
+      {
+        return fdopen_pipe ();
+      }
+      catch (const ios_base::failure&)
+      {
+        // Translate to process_error.
+        //
+        // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+        // from system_error and so we cannot recover the errno value. On the
+        // other hand the only possible values are EMFILE and ENFILE. Lets use
+        // EMFILE as the more probable. This is a temporary code after all.
+        //
+        throw process_error (EMFILE, false);
+      }
     };
 
-    auto create_null = [&fail](auto_fd& n)
+    auto open_null = [&fail] () -> auto_fd
     {
-      int fd (fdnull ());
-      if (fd == -1)
+      auto_fd fd (fdnull ());
+      if (fd.get () == -1)
         fail (false);
 
-      n.reset (fd);
+      return fd;
     };
 
     // If we are asked to open null (-2) then open "half-pipe".
     //
     if (in == -1)
-      create_pipe (out_fd);
+      out_fd = open_pipe ();
     else if (in == -2)
-      create_null (out_fd[0]);
+      out_fd.in = open_null ();
 
     if (out == -1)
-      create_pipe (in_ofd);
+      in_ofd = open_pipe ();
     else if (out == -2)
-      create_null (in_ofd[1]);
+      in_ofd.out = open_null ();
 
     if (err == -1)
-      create_pipe (in_efd);
+      in_efd = open_pipe ();
     else if (err == -2)
-      create_null (in_efd[1]);
+      in_efd.out = open_null ();
 
     handle = fork ();
 
@@ -284,17 +291,17 @@ namespace butl
       // to the standard stream descriptor (read end for STDIN_FILENO, write
       // end otherwise). Close the the pipe afterwards.
       //
-      auto duplicate = [&fail](int sd, int fd, pipe& pd)
+      auto duplicate = [&fail](int sd, int fd, fdpipe& pd)
       {
         if (fd == -1 || fd == -2)
-          fd = pd[sd == STDIN_FILENO ? 0 : 1].get ();
+          fd = (sd == STDIN_FILENO ? pd.in : pd.out).get ();
 
         assert (fd > -1);
         if (dup2 (fd, sd) == -1)
           fail (true);
 
-        pd[0].reset (); // Close.
-        pd[1].reset (); // Close.
+        pd.in.reset ();  // Silently close.
+        pd.out.reset (); // Silently close.
       };
 
       if (in != STDIN_FILENO)
@@ -333,9 +340,9 @@ namespace butl
 
     assert (handle != 0); // Shouldn't get here unless in the parent process.
 
-    this->out_fd = move (out_fd[1]);
-    this->in_ofd = move (in_ofd[0]);
-    this->in_efd = move (in_efd[0]);
+    this->out_fd = move (out_fd.out);
+    this->in_ofd = move (in_ofd.in);
+    this->in_efd = move (in_efd.in);
   }
 
   bool process::
@@ -616,37 +623,65 @@ namespace butl
            const process_path& pp, const char* args[],
            int in, int out, int err)
   {
-    using pipe = auto_handle[2];
-
-    pipe out_h;
-    pipe in_oh;
-    pipe in_eh;
+    fdpipe out_fd;
+    fdpipe in_ofd;
+    fdpipe in_efd;
 
-    SECURITY_ATTRIBUTES sa;
-    sa.nLength = sizeof (SECURITY_ATTRIBUTES);
-    sa.bInheritHandle = true;
-    sa.lpSecurityDescriptor = 0;
+    auto open_pipe = [] () -> fdpipe
+    {
+      try
+      {
+        return fdopen_pipe ();
+      }
+      catch (const ios_base::failure&)
+      {
+        // Translate to process_error.
+        //
+        // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+        // from system_error and so we cannot recover the errno value. On the
+        // other hand the only possible values are EMFILE and ENFILE. Lets use
+        // EMFILE as the more probable. Also let's make no distinction for VC.
+        // This is a temporary code after all.
+        //
+        throw process_error (EMFILE);
+      }
+    };
 
     auto fail = [](const char* m = nullptr)
     {
       throw process_error (m == nullptr ? last_error_msg () : m);
     };
 
-    // Create a pipe and clear the inherit flag on the parent side.
-    //
-    auto create_pipe = [&sa, &fail](pipe& p, int parent)
+    auto open_null = [&fail] () -> auto_fd
     {
-      HANDLE ph[2];
-      if (!CreatePipe (&ph[0], &ph[1], &sa, 0))
+      // Note that we are using a faster, temporary file-based emulation of
+      // NUL since we have no way of making sure the child buffers things
+      // properly (and by default they seem no to).
+      //
+      auto_fd fd (fdnull (true));
+      if (fd.get () == -1)
         fail ();
 
-      p[0].reset (ph[0]);
-      p[1].reset (ph[1]);
-
-      if (!SetHandleInformation (p[parent].get (), HANDLE_FLAG_INHERIT, 0))
-        fail ();
+      return fd;
     };
 
+    // If we are asked to open null (-2) then open "half-pipe".
+    //
+    if (in == -1)
+      out_fd = open_pipe ();
+    else if (in == -2)
+      out_fd.in = open_null ();
+
+    if (out == -1)
+      in_ofd = open_pipe ();
+    else if (out == -2)
+      in_ofd.out = open_null ();
+
+    if (err == -1)
+      in_efd = open_pipe ();
+    else if (err == -2)
+      in_efd.out = open_null ();
+
     // Resolve file descriptor to HANDLE and make sure it is inherited. Note
     // that the handle is closed either when CloseHandle() is called for it or
     // when _close() is called for the associated file descriptor. Make sure
@@ -672,35 +707,6 @@ namespace butl
       return h;
     };
 
-    auto create_null = [&get_osfhandle, &fail](auto_handle& n)
-    {
-      // Note that we are using a faster, temporary file-based emulation of
-      // NUL since we have no way of making sure the child buffers things
-      // properly (and by default they seem no to).
-      //
-      auto_fd fd (fdnull (true));
-      if (fd.get () == -1)
-        fail ();
-
-      n.reset (get_osfhandle (fd.get ()));
-      fd.release (); // Not to close the handle twice.
-    };
-
-    if (in == -1)
-      create_pipe (out_h, 1);
-    else if (in == -2)
-      create_null (out_h[0]);
-
-    if (out == -1)
-      create_pipe (in_oh, 0);
-    else if (out == -2)
-      create_null (in_oh[1]);
-
-    if (err == -1)
-      create_pipe (in_eh, 0);
-    else if (err == -2)
-      create_null (in_eh[1]);
-
     // Create the process.
     //
 
@@ -738,7 +744,6 @@ namespace butl
     //
     STARTUPINFO si;
     PROCESS_INFORMATION pi;
-
     memset (&si, 0, sizeof (STARTUPINFO));
     memset (&pi, 0, sizeof (PROCESS_INFORMATION));
 
@@ -746,19 +751,19 @@ namespace butl
     si.dwFlags |= STARTF_USESTDHANDLES;
 
     si.hStdInput = in == -1 || in == -2
-      ? out_h[0].get ()
+      ? get_osfhandle (out_fd.in.get ())
       : in == STDIN_FILENO
         ? GetStdHandle (STD_INPUT_HANDLE)
         : get_osfhandle (in);
 
     si.hStdOutput = out == -1 || out == -2
-      ? in_oh[1].get ()
+      ? get_osfhandle (in_ofd.out.get ())
       : out == STDOUT_FILENO
         ? GetStdHandle (STD_OUTPUT_HANDLE)
         : get_osfhandle (out);
 
     si.hStdError = err == -1 || err == -2
-      ? in_eh[1].get ()
+      ? get_osfhandle (in_efd.out.get ())
       : err == STDERR_FILENO
         ? GetStdHandle (STD_ERROR_HANDLE)
         : get_osfhandle (err);
@@ -792,24 +797,9 @@ namespace butl
     auto_handle (pi.hThread).reset (); // Close.
     auto_handle process (pi.hProcess);
 
-    // Convert file handles to file descriptors. Note that the handle is
-    // closed when _close() is called for the returned file descriptor.
-    //
-    auto open_osfhandle = [&fail](auto_handle& h) -> int
-    {
-      int fd (
-        _open_osfhandle (reinterpret_cast<intptr_t> (h.get ()), _O_TEXT));
-
-      if (fd == -1)
-        fail ("unable to convert file handle to file descriptor");
-
-      h.release ();
-      return fd;
-    };
-
-    this->out_fd.reset (in  == -1 ? open_osfhandle (out_h[1]) : -1);
-    this->in_ofd.reset (out == -1 ? open_osfhandle (in_oh[0]) : -1);
-    this->in_efd.reset (err == -1 ? open_osfhandle (in_eh[0]) : -1);
+    this->out_fd = move (out_fd.out);
+    this->in_ofd = move (in_ofd.in);
+    this->in_efd = move (in_efd.in);
 
     this->handle = process.release ();
 
diff --git a/tests/fdstream/driver.cxx b/tests/fdstream/driver.cxx
index 4d72284..61e6ae6 100644
--- a/tests/fdstream/driver.cxx
+++ b/tests/fdstream/driver.cxx
@@ -29,25 +29,39 @@ using namespace butl;
 
 static const string text1 ("ABCDEF\nXYZ");
 static const string text2 ("12");            // Keep shorter than text1.
+
+// Windows text mode write-translated form of text1.
+//
 static const string text3 ("ABCDEF\r\nXYZ");
 
 static string
+from_stream (ifdstream& is)
+{
+  string s;
+  getline (is, s, '\0');
+  is.close (); // Not to miss failed close of the underlying file descriptor.
+  return s;
+}
+
+static string
 from_file (const path& f, fdopen_mode m = fdopen_mode::none)
 {
   ifdstream ifs (f, m, ifdstream::badbit);
+  return from_stream (ifs);
+}
 
-  string s;
-  getline (ifs, s, '\0');
-  ifs.close (); // Not to miss failed close of the underlying file descriptor.
-  return s;
+static void
+to_stream (ofdstream& os, const string& s)
+{
+  os << s;
+  os.close ();
 }
 
 static void
 to_file (const path& f, const string& s, fdopen_mode m = fdopen_mode::none)
 {
   ofdstream ofs (f, m);
-  ofs << s;
-  ofs.close ();
+  to_stream (ofs, s);
 }
 
 template <typename S, typename T>
@@ -449,6 +463,41 @@ main (int argc, const char* argv[])
 
 #endif
 
+  // Test pipes.
+  //
+  // Here we rely on buffering being always enabled for pipes.
+  //
+  {
+    fdpipe pipe (fdopen_pipe ());
+    ofdstream os (move (pipe.out));
+    ifdstream is (move (pipe.in));
+    to_stream (os, text1);
+    assert (from_stream (is) == text1);
+  }
+
+#ifdef _WIN32
+
+  // Test opening a pipe in the text mode.
+  //
+  {
+    fdpipe pipe (fdopen_pipe ());
+    ofdstream os (move (pipe.out));
+    ifdstream is (move (pipe.in), fdstream_mode::binary);
+    to_stream (os, text1);
+    assert (from_stream (is) == text3);
+  }
+
+  // Test opening a pipe in the binary mode.
+  //
+  {
+    fdpipe pipe (fdopen_pipe (fdopen_mode::binary));
+    ofdstream os (move (pipe.out), fdstream_mode::text);
+    ifdstream is (move (pipe.in));
+    to_stream (os, text1);
+    assert (from_stream (is) == text3);
+  }
+
+#endif
   // Compare fdstream and fstream operations performance.
   //
   duration fwd (0);