diff options
Diffstat (limited to 'butl')
| -rw-r--r-- | butl/fdstream | 121 | ||||
| -rw-r--r-- | butl/fdstream.cxx | 63 | ||||
| -rw-r--r-- | butl/fdstream.ixx | 64 | ||||
| -rw-r--r-- | butl/optional | 4 | ||||
| -rw-r--r-- | butl/pager.cxx | 5 | ||||
| -rw-r--r-- | butl/process | 14 | ||||
| -rw-r--r-- | butl/process.cxx | 92 | ||||
| -rw-r--r-- | butl/process.ixx | 12 |
8 files changed, 188 insertions, 187 deletions
diff --git a/butl/fdstream b/butl/fdstream index 2d7fae5..9c7274d 100644 --- a/butl/fdstream +++ b/butl/fdstream @@ -8,6 +8,7 @@ #include <string> #include <istream> #include <ostream> +#include <utility> // move() #include <cstdint> // uint16_t #include <butl/export> @@ -17,6 +18,45 @@ namespace butl { + // RAII type for file descriptors. Note that failure to close the descriptor + // is silently ignored by both the destructor and reset() (thought we could + // probably provide the close() function that throws). + // + // The descriptor can be negative. Such a descriptor is treated as unopened + // and is not closed. + // + struct nullfd_t {constexpr explicit nullfd_t (int) {}}; + constexpr const nullfd_t nullfd (-1); + + class auto_fd + { + public: + auto_fd (nullfd_t = nullfd): fd_ (-1) {} + + explicit + auto_fd (int fd) noexcept: fd_ (fd) {} + + auto_fd (auto_fd&& fd) noexcept: fd_ (fd.release ()) {} + auto_fd& operator= (auto_fd&&) noexcept; + + auto_fd (const auto_fd&) = delete; + auto_fd& operator= (const auto_fd&) = delete; + + ~auto_fd () noexcept {reset ();} + + int + get () const noexcept {return fd_;} + + int + release () noexcept; + + void + reset (int fd = -1) noexcept; + + private: + int fd_; + }; + // An [io]fstream that can be initialized with a file descriptor in addition // to a file name and that also by default enables exceptions on badbit and // failbit. So instead of a dance like this: @@ -41,31 +81,26 @@ namespace butl // - after catching an exception caused by badbit the stream is no longer // used // - not movable, though can be easily supported - // - passing to constructor -1 file descriptor is valid and results in the - // creation of an unopened object + // - passing to constructor auto_fd with a negative file descriptor is valid + // and results in the creation of an unopened object // class LIBBUTL_EXPORT fdbuf: public std::basic_streambuf<char> { public: - virtual - ~fdbuf (); fdbuf () = default; - fdbuf (int fd) {if (fd != -1) open (fd);} - - fdbuf (const fdbuf&) = delete; - fdbuf& operator= (const fdbuf&) = delete; + fdbuf (auto_fd&&); void close (); void - open (int fd); + open (auto_fd&&); bool - is_open () const {return fd_ != -1;} + is_open () const {return fd_.get () >= 0;} int - fd () const {return fd_;} + fd () const {return fd_.get ();} public: using int_type = std::basic_streambuf<char>::int_type; @@ -101,7 +136,7 @@ namespace butl save (); private: - int fd_ = -1; + auto_fd fd_; char buf_[8192]; bool non_blocking_ = false; }; @@ -164,8 +199,8 @@ namespace butl { protected: fdstream_base () = default; - fdstream_base (int fd): buf_ (fd) {} - fdstream_base (int, fdstream_mode); + fdstream_base (auto_fd&& fd): buf_ (std::move (fd)) {} + fdstream_base (auto_fd&&, fdstream_mode); public: int @@ -191,18 +226,13 @@ namespace butl // iofdstream constructors and open() functions that take file path as a // const std::string& or const char* may throw the invalid_path exception. // - // Passing -1 as a file descriptor is valid and results in the creation of - // an unopened object. + // Passing auto_fd with a negative file descriptor is valid and results in + // the creation of an unopened object. // // Also note that open() and close() functions can be successfully called // for an opened and unopened objects respectively. That is in contrast with // iofstream that sets failbit in such cases. // - // @@ Need to make sure performance is on par with fstream on both - // Linux and Windows. - // - // @@ Do we need to increase default buffer size? Make it customizable? - // Wonder what it is in libstdc++ and MSVC? // Note that ifdstream destructor will close an open file descriptor but // will ignore any errors. To detect such errors, call close() explicitly. @@ -219,8 +249,8 @@ namespace butl // { // // In case of exception, skip and close input after output. // // - // ifdstream is (pr.in_ofd, fdstream_mode::skip); - // ofdstream os (pr.out_fd); + // ifdstream is (move (pr.in_ofd), fdstream_mode::skip); + // ofdstream os (move (pr.out_fd)); // // // Write. // @@ -268,17 +298,14 @@ namespace butl class LIBBUTL_EXPORT ifdstream: public fdstream_base, public std::istream { public: - // Create an unopened object with iostate = badbit | failbit (we cannot - // have the iostate as an argument since it clashes with int fd) To create - // an unopened object with non-default exception mask one can do: - // - // ifdstream (-1, ...); + // Create an unopened object. // - ifdstream (); + explicit + ifdstream (iostate e = badbit | failbit); explicit - ifdstream (int fd, iostate e = badbit | failbit); - ifdstream (int fd, fdstream_mode m, iostate e = badbit | failbit); + ifdstream (auto_fd&&, iostate e = badbit | failbit); + ifdstream (auto_fd&&, fdstream_mode m, iostate e = badbit | failbit); explicit ifdstream (const char*, @@ -328,7 +355,7 @@ namespace butl open (const path&, fdopen_mode); void - open (int fd) {buf_.open (fd); clear ();} + open (auto_fd&& fd) {buf_.open (std::move (fd)); clear ();} void close (); bool is_open () const {return buf_.is_open ();} @@ -348,17 +375,14 @@ namespace butl class LIBBUTL_EXPORT ofdstream: public fdstream_base, public std::ostream { public: - // Create an unopened object with iostate = badbit | failbit (we cannot - // have the iostate as an argument since it clashes with int fd). To create - // an unopened object with non-default exception mask one can do: - // - // ofdstream (-1, ...); + // Create an unopened object. // - ofdstream (); + explicit + ofdstream (iostate e = badbit | failbit); explicit - ofdstream (int fd, iostate e = badbit | failbit); - ofdstream (int fd, fdstream_mode m, iostate e = badbit | failbit); + ofdstream (auto_fd&&, iostate e = badbit | failbit); + ofdstream (auto_fd&&, fdstream_mode m, iostate e = badbit | failbit); explicit ofdstream (const char*, @@ -408,7 +432,7 @@ namespace butl open (const path&, fdopen_mode); void - open (int fd) {buf_.open (fd); clear ();} + open (auto_fd&& fd) {buf_.open (std::move (fd)); clear ();} void close () {if (is_open ()) flush (); buf_.close ();} bool is_open () const {return buf_.is_open ();} @@ -432,8 +456,8 @@ namespace butl LIBBUTL_EXPORT ifdstream& getline (ifdstream&, std::string&, char delim = '\n'); - // Open a file returning the file descriptor on success and throwing - // ios:failure otherwise. + // Open a file returning an auto_fd that holds its file descriptor on + // success and throwing ios::failure otherwise. // // The mode argument should have at least one of the in or out flags set. // The append and truncate flags are meaningless in the absense of the out @@ -448,21 +472,21 @@ namespace butl // process' umask, so effective permissions are permissions & ~umask. On // Windows permissions other than ru and wu are unlikelly to have effect. // - LIBBUTL_EXPORT int + LIBBUTL_EXPORT auto_fd fdopen (const char*, fdopen_mode, permissions = permissions::ru | permissions::wu | permissions::rg | permissions::wg | permissions::ro | permissions::wo); - LIBBUTL_EXPORT int + LIBBUTL_EXPORT auto_fd fdopen (const std::string&, fdopen_mode, permissions = permissions::ru | permissions::wu | permissions::rg | permissions::wg | permissions::ro | permissions::wo); - LIBBUTL_EXPORT int + LIBBUTL_EXPORT auto_fd fdopen (const path&, fdopen_mode, permissions = permissions::ru | permissions::wu | @@ -510,6 +534,11 @@ namespace butl // closed. One difference, however, would be if you were to both write to // and read from the descriptor. // + // @@ You may have noticed that this interface doesn't match fdopen() (it + // does not throw and return int instead of auto_fd). The reason for this + // is process and its need to translate everything to process_error). + // Once we solve that we can clean this up as well. + // #ifndef _WIN32 LIBBUTL_EXPORT int fdnull () noexcept; diff --git a/butl/fdstream.cxx b/butl/fdstream.cxx index 109b041..dbd7767 100644 --- a/butl/fdstream.cxx +++ b/butl/fdstream.cxx @@ -71,19 +71,19 @@ namespace butl // fdbuf // fdbuf:: - ~fdbuf () + fdbuf (auto_fd&& fd) { - if (is_open ()) - fdclose (fd_); // Don't check for an error as not much we can do here. + if (fd.get () >= 0) + open (move (fd)); } void fdbuf:: - open (int fd) + open (auto_fd&& fd) { close (); #ifndef _WIN32 - int flags (fcntl (fd, F_GETFL)); + int flags (fcntl (fd.get (), F_GETFL)); if (flags == -1) throw_ios_failure (errno); @@ -91,21 +91,22 @@ namespace butl non_blocking_ = (flags & O_NONBLOCK) == O_NONBLOCK; #endif - fd_ = fd; setg (buf_, buf_, buf_); setp (buf_, buf_ + sizeof (buf_) - 1); // Keep space for overflow's char. + + fd_ = move (fd); } void fdbuf:: close () { - if (is_open ()) - { - if (!fdclose (fd_)) - throw_ios_failure (errno); - - fd_ = -1; - } + // Before we invented auto_fd into fdstreams we keept fdbuf opened on + // faulty close attempt. Now fdbuf is always closed by close() function. + // This semantics change seems to be the right one as there is no reason to + // expect fdclose() to succeed after it has already failed once. + // + if (is_open () && !fdclose (fd_.release ())) + throw_ios_failure (errno); } streamsize fdbuf:: @@ -122,7 +123,7 @@ namespace butl #ifndef _WIN32 if (non_blocking_) { - ssize_t n (read (fd_, buf_, sizeof (buf_))); + ssize_t n (read (fd_.get (), buf_, sizeof (buf_))); if (n == -1) { @@ -173,9 +174,9 @@ namespace butl assert (!non_blocking_); #ifndef _WIN32 - ssize_t n (read (fd_, buf_, sizeof (buf_))); + ssize_t n (read (fd_.get (), buf_, sizeof (buf_))); #else - int n (_read (fd_, buf_, sizeof (buf_))); + int n (_read (fd_.get (), buf_, sizeof (buf_))); #endif if (n == -1) @@ -242,9 +243,9 @@ namespace butl // expected). This is in contrast with VC's _write() and POSIX's write(). // #ifndef _WIN32 - ssize_t m (write (fd_, buf_, n)); + ssize_t m (write (fd_.get (), buf_, n)); #else - int m (_write (fd_, buf_, n)); + int m (_write (fd_.get (), buf_, n)); #endif if (m == -1) @@ -296,10 +297,10 @@ namespace butl // Write both buffered and new data with a single system call. // iovec iov[2] = {{pbase (), bn}, {const_cast<char*> (s), n}}; - r = writev (fd_, iov, 2); + r = writev (fd_.get (), iov, 2); } else - r = write (fd_, s, n); + r = write (fd_.get (), s, n); if (r == -1) throw_ios_failure (errno); @@ -342,7 +343,7 @@ namespace butl // Flush the buffer. // size_t wn (bn + an); - int r (wn > 0 ? _write (fd_, buf_, wn) : 0); + int r (wn > 0 ? _write (fd_.get (), buf_, wn) : 0); if (r == -1) throw_ios_failure (errno); @@ -378,7 +379,7 @@ namespace butl // The data tail doesn't fit the buffer so write it to the file. // - r = _write (fd_, s, n); + r = _write (fd_.get (), s, n); if (r == -1) throw_ios_failure (errno); @@ -393,15 +394,15 @@ namespace butl return (m & flag) == flag; } - inline static int - mode (int fd, fdstream_mode m) + inline static auto_fd + mode (auto_fd fd, fdstream_mode m) { - if (fd != -1 && + if (fd.get () >= 0 && (flag (m, fdstream_mode::text) || flag (m, fdstream_mode::binary) || flag (m, fdstream_mode::blocking) || flag (m, fdstream_mode::non_blocking))) - fdmode (fd, m); + fdmode (fd.get (), m); return fd; } @@ -409,8 +410,8 @@ namespace butl // fdstream_base // fdstream_base:: - fdstream_base (int fd, fdstream_mode m) - : fdstream_base (mode (fd, m)) // Delegate. + fdstream_base (auto_fd&& fd, fdstream_mode m) + : fdstream_base (mode (move (fd), m)) // Delegate. { } @@ -582,7 +583,7 @@ namespace butl // Utility functions // - int + auto_fd fdopen (const char* f, fdopen_mode m, permissions p) { mode_t pf (S_IREAD | S_IWRITE | S_IEXEC); @@ -714,7 +715,7 @@ namespace butl } } - return fd; + return auto_fd (fd); } #ifndef _WIN32 @@ -793,7 +794,7 @@ namespace butl int fdnull (bool temp) noexcept { - // No need to translate /r/n before sending it to void. + // No need to translate \r\n before sending it to void. // if (!temp) return _sopen ("nul", _O_RDWR | _O_BINARY, _SH_DENYNO); diff --git a/butl/fdstream.ixx b/butl/fdstream.ixx index c7c329b..5d38d36 100644 --- a/butl/fdstream.ixx +++ b/butl/fdstream.ixx @@ -6,26 +6,51 @@ namespace butl { - // ifdstream + // auto_fd // - inline ifdstream:: - ifdstream () - : std::istream (&buf_) + inline auto_fd& auto_fd:: + operator= (auto_fd&& fd) noexcept + { + reset (fd.release ()); + return *this; + } + + inline int auto_fd:: + release () noexcept { - exceptions (badbit | failbit); + 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; + } + + // ifdstream + // inline ifdstream:: - ifdstream (int fd, iostate e) - : fdstream_base (fd), std::istream (&buf_) + ifdstream (auto_fd&& fd, iostate e) + : fdstream_base (std::move (fd)), std::istream (&buf_) { assert (e & badbit); exceptions (e); } inline ifdstream:: - ifdstream (int fd, fdstream_mode m, iostate e) - : fdstream_base (fd, m), + ifdstream (iostate e) + : ifdstream (auto_fd (), e) // Delegate. + { + } + + inline ifdstream:: + ifdstream (auto_fd&& fd, fdstream_mode m, iostate e) + : fdstream_base (std::move (fd), m), std::istream (&buf_), skip_ ((m & fdstream_mode::skip) == fdstream_mode::skip) { @@ -84,23 +109,22 @@ namespace butl // ofdstream // inline ofdstream:: - ofdstream () - : std::ostream (&buf_) + ofdstream (auto_fd&& fd, iostate e) + : fdstream_base (std::move (fd)), std::ostream (&buf_) { - exceptions (badbit | failbit); + assert (e & badbit); + exceptions (e); } inline ofdstream:: - ofdstream (int fd, iostate e) - : fdstream_base (fd), std::ostream (&buf_) + ofdstream (iostate e) + : ofdstream (auto_fd (), e) // Delegate. { - assert (e & badbit); - exceptions (e); } inline ofdstream:: - ofdstream (int fd, fdstream_mode m, iostate e) - : fdstream_base (fd, m), std::ostream (&buf_) + ofdstream (auto_fd&& fd, fdstream_mode m, iostate e) + : fdstream_base (std::move (fd), m), std::ostream (&buf_) { assert (e & badbit); exceptions (e); @@ -156,13 +180,13 @@ namespace butl // fdopen() // - inline int + inline auto_fd fdopen (const std::string& f, fdopen_mode m, permissions p) { return fdopen (f.c_str (), m, p); } - inline int + inline auto_fd fdopen (const path& f, fdopen_mode m, permissions p) { return fdopen (f.string (), m, p); diff --git a/butl/optional b/butl/optional index 0ae7e7b..21d686c 100644 --- a/butl/optional +++ b/butl/optional @@ -11,8 +11,8 @@ namespace butl { // Simple optional class template while waiting for std::optional. // - struct nullopt_t {constexpr nullopt_t (int) {}}; - constexpr const nullopt_t nullopt = 1; + struct nullopt_t {constexpr explicit nullopt_t (int) {}}; + constexpr const nullopt_t nullopt (1); template <typename T> class optional diff --git a/butl/pager.cxx b/butl/pager.cxx index b154a5c..a03582c 100644 --- a/butl/pager.cxx +++ b/butl/pager.cxx @@ -15,6 +15,7 @@ #endif #include <cstring> // strchr() +#include <utility> // move() #include <system_error> #include <butl/fdstream> // fdclose() @@ -134,13 +135,13 @@ namespace butl bool r; if (p_.try_wait (r)) { - fdclose (p_.out_fd); + p_.out_fd.reset (); if (pager != nullptr) throw system_error (ECHILD, system_category ()); } else - os_.open (p_.out_fd); + os_.open (move (p_.out_fd)); } catch (const process_error& e) { diff --git a/butl/process b/butl/process index c69c306..e98e367 100644 --- a/butl/process +++ b/butl/process @@ -17,6 +17,7 @@ #include <butl/path> #include <butl/export> #include <butl/optional> +#include <butl/fdstream> // auto_fd namespace butl { @@ -134,7 +135,7 @@ namespace butl // the child process. 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. Use the _setmode() function to + // translated into the 0xD, 0xA sequence. Use the fdmode() function to // change the mode, if required. // // Instead of passing -1, -2 or the default value, you can also pass your @@ -216,8 +217,6 @@ namespace butl // Create an empty or "already terminated" process. By default the // termination status is abnormal but you can change that. // - // @@ Need to audit all calls (__attribute__((deprecated))). - // explicit process (optional<status_type> status = nullopt); @@ -295,9 +294,12 @@ namespace butl handle_type handle; optional<status_type> status; // Absence means terminated abnormally. - int out_fd; // Write to this fd to send to the new process' stdin. - int in_ofd; // Read from this fd to receive from the new process' stdout. - int in_efd; // Read from this fd to receive from the new process' stderr. + // Use the following file descriptors to communicate with the new process's + // standard streams. + // + auto_fd out_fd; // Write to it to send to stdin. + auto_fd in_ofd; // Read from it to receive from stdout. + auto_fd in_efd; // Read from it to receive from stderr. }; } diff --git a/butl/process.cxx b/butl/process.cxx index 3af42cf..d8b10be 100644 --- a/butl/process.cxx +++ b/butl/process.cxx @@ -37,10 +37,11 @@ #include <cassert> #include <cstddef> // size_t #include <cstring> // strlen(), strchr() +#include <utility> // move() #include <ostream> #include <butl/utility> // casecmp() -#include <butl/fdstream> // fdnull(), fdclose() +#include <butl/fdstream> // fdnull() using namespace std; @@ -50,49 +51,6 @@ using namespace butl::win32; namespace butl { - class auto_fd - { - public: - explicit - auto_fd (int fd = -1) noexcept: fd_ (fd) {} - - auto_fd (const auto_fd&) = delete; - auto_fd& operator= (const auto_fd&) = delete; - - ~auto_fd () noexcept {reset ();} - - int - get () const noexcept {return fd_;} - - int - release () noexcept - { - int r (fd_); - fd_ = -1; - return r; - } - - void - reset (int fd = -1) noexcept - { - if (fd_ != -1) - { - bool r (fdclose (fd_)); - - // The valid file descriptor that has no IO operations being - // performed on it should close successfully, unless something is - // severely damaged. - // - assert (r); - } - - fd_ = fd; - } - - private: - int fd_; - }; - static process_path path_search (const char*, const dir_path&); @@ -158,6 +116,16 @@ namespace butl } while (*p != nullptr); } + process:: + process (const char* cwd, + const process_path& pp, const char* args[], + process& in, int out, int err) + : process (cwd, pp, args, in.in_ofd.get (), out, err) + { + assert (in.in_ofd.get () != -1); // Should be a pipe. + in.in_ofd.reset (); // Close it on our side. + } + #ifndef _WIN32 static process_path @@ -365,19 +333,9 @@ namespace butl assert (handle != 0); // Shouldn't get here unless in the parent process. - this->out_fd = out_fd[1].release (); - this->in_ofd = in_ofd[0].release (); - this->in_efd = in_efd[0].release (); - } - - process:: - process (const char* cwd, - const process_path& pp, const char* args[], - process& in, int out, int err) - : process (cwd, pp, args, in.in_ofd, out, err) - { - assert (in.in_ofd != -1); // Should be a pipe. - close (in.in_ofd); // Close it on our side. + this->out_fd = move (out_fd[1]); + this->in_ofd = move (in_ofd[0]); + this->in_efd = move (in_efd[0]); } bool process:: @@ -849,13 +807,9 @@ namespace butl return fd; }; - auto_fd out_fd (in == -1 ? open_osfhandle (out_h[1]) : -1); - auto_fd in_ofd (out == -1 ? open_osfhandle (in_oh[0]) : -1); - auto_fd in_efd (err == -1 ? open_osfhandle (in_eh[0]) : -1); - - this->out_fd = out_fd.release (); - this->in_ofd = in_ofd.release (); - this->in_efd = in_efd.release (); + 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->handle = process.release (); @@ -864,16 +818,6 @@ namespace butl assert (this->handle != 0 && this->handle != INVALID_HANDLE_VALUE); } - process:: - process (const char* cwd, - const process_path& pp, const char* args[], - process& in, int out, int err) - : process (cwd, pp, args, in.in_ofd, out, err) - { - assert (in.in_ofd != -1); // Should be a pipe. - _close (in.in_ofd); // Close it on our side. - } - bool process:: wait (bool ie) { diff --git a/butl/process.ixx b/butl/process.ixx index 6d4dd2f..1bc259c 100644 --- a/butl/process.ixx +++ b/butl/process.ixx @@ -139,9 +139,9 @@ namespace butl process (process&& p) : handle (p.handle), status (p.status), - out_fd (p.out_fd), - in_ofd (p.in_ofd), - in_efd (p.in_efd) + out_fd (std::move (p.out_fd)), + in_ofd (std::move (p.in_ofd)), + in_efd (std::move (p.in_efd)) { p.handle = 0; } @@ -156,9 +156,9 @@ namespace butl handle = p.handle; status = std::move (p.status); - out_fd = p.out_fd; - in_ofd = p.in_ofd; - in_efd = p.in_efd; + out_fd = std::move (p.out_fd); + in_ofd = std::move (p.in_ofd); + in_efd = std::move (p.in_efd); p.handle = 0; } |