From 834f0df3850c2b115e2febbf5b6bdafbe88651e2 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 11 Nov 2016 11:33:21 +0300 Subject: Add fdopen_pipe() --- butl/fdstream | 51 ++++++++++-- butl/fdstream.cxx | 32 +++++++- butl/fdstream.ixx | 26 +++---- butl/process.cxx | 192 ++++++++++++++++++++++------------------------ tests/fdstream/driver.cxx | 61 +++++++++++++-- 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 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 // open(), O_*, fcntl() -# include // close(), read(), write(), lseek(), dup(), ssize_t, - // STD*_FILENO +# include // close(), read(), write(), lseek(), dup(), pipe(), + // ssize_t, STD*_FILENO # include // writev(), iovec # include // S_I* # include // off_t #else # include // _close(), _read(), _write(), _setmode(), _sopen(), - // _lseek(), _dup() + // _lseek(), _dup(), _pipe() # include // _SH_DENYNO # include // _fileno(), stdin, stdout, stderr # include // _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 -# include // _open_osfhandle(), _get_osfhandle(), _close() -# include // _O_TEXT +# include // _get_osfhandle(), _close() # include // _MAX_PATH, getenv() # include // stat # include // stat(), S_IS* @@ -34,6 +33,7 @@ #include +#include // ios_base::failure #include #include // size_t #include // 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 (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 @@ -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); -- cgit v1.1