From b8526f8f0cb24c457da8c88877e516943e3e1902 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 24 Apr 2017 13:51:56 +0300 Subject: Make fdnull() to return auto_fd --- butl/curl.cxx | 8 ++++---- butl/fdstream | 17 ++++++----------- butl/fdstream.cxx | 55 +++++++++++++++++++++++++++++++++++-------------------- butl/process.cxx | 46 ++++++++++++++++++++++++++++++++++++---------- 4 files changed, 81 insertions(+), 45 deletions(-) diff --git a/butl/curl.cxx b/butl/curl.cxx index 4951b52..12a1d54 100644 --- a/butl/curl.cxx +++ b/butl/curl.cxx @@ -25,7 +25,7 @@ namespace butl case ftp_get: case http_get: { - d.pipe.in.reset (fdnull ()); // /dev/null + d.pipe.in = fdnull (); // /dev/null return d.pipe.in.get (); } } @@ -60,7 +60,7 @@ namespace butl out.open (move (d.pipe.out)); } else - d.pipe.in.reset (fdnull ()); // /dev/null + d.pipe.in = fdnull (); // /dev/null return d.pipe.in.get (); } @@ -85,7 +85,7 @@ namespace butl case ftp_put: case http_post: // May or may not produce output. { - d.pipe.out.reset (fdnull ()); + d.pipe.out = fdnull (); return d.pipe.out.get (); // /dev/null } } @@ -113,7 +113,7 @@ namespace butl { d.options.push_back ("-o"); d.options.push_back (f.string ().c_str ()); - d.pipe.out.reset (fdnull ()); // /dev/null + d.pipe.out = fdnull (); // /dev/null } return d.pipe.out.get (); diff --git a/butl/fdstream b/butl/fdstream index afc3ef6..df39808 100644 --- a/butl/fdstream +++ b/butl/fdstream @@ -575,9 +575,8 @@ namespace butl // Open the null device (e.g., /dev/null) that discards all data written to // it and provides no data for read operations (i.e., yelds EOF on read). - // Return file descriptor on success, set errno and return -1 otherwise. - // Note that it's the caller's responsibility to close the returned file - // descriptor. + // Return an auto_fd that holds its file descriptor on success and throwing + // ios::failure otherwise. // // On Windows the null device is NUL and writing anything substantial to it // (like redirecting a process' output) is extremely slow, as in, an order @@ -593,18 +592,14 @@ namespace butl // and read from the descriptor. // // Note that on POSIX the FD_CLOEXEC flag is set for the file descriptor to - // prevent its leakage into child processes. - // - // @@ 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. + // prevent its leakage into child processes. On Windows, for the same + // purpose, the _O_NOINHERIT flag is set. // #ifndef _WIN32 - LIBBUTL_EXPORT int + LIBBUTL_EXPORT auto_fd fdnull () noexcept; #else - LIBBUTL_EXPORT int + LIBBUTL_EXPORT auto_fd fdnull (bool temp = false) noexcept; #endif diff --git a/butl/fdstream.cxx b/butl/fdstream.cxx index 19ef6c5..68168f3 100644 --- a/butl/fdstream.cxx +++ b/butl/fdstream.cxx @@ -47,7 +47,7 @@ namespace butl // throw_ios_failure // template - static inline typename enable_if::type + [[noreturn]] static inline typename enable_if::type throw_ios_failure (error_code e, const char* m) { // The idea here is to make an error code to be saved into failure @@ -65,7 +65,7 @@ namespace butl } template - static inline typename enable_if::type + [[noreturn]] static inline typename enable_if::type throw_ios_failure (error_code e, const char* m) { throw ios_base::failure (m != nullptr ? m : e.message ().c_str ()); @@ -73,7 +73,7 @@ namespace butl // Throw system_error with generic_category. // - static inline void + [[noreturn]] static inline void throw_ios_failure (int errno_code, const char* m = nullptr) { error_code ec (errno_code, generic_category ()); @@ -810,10 +810,15 @@ namespace butl return close (fd) == 0; } - int + auto_fd fdnull () noexcept { - return open ("/dev/null", O_RDWR | O_CLOEXEC); + int fd (open ("/dev/null", O_RDWR | O_CLOEXEC)); + + if (fd == -1) + throw_ios_failure (errno); + + return auto_fd (fd); } fdstream_mode @@ -955,13 +960,20 @@ namespace butl return _close (fd) == 0; } - int + auto_fd fdnull (bool temp) noexcept { // No need to translate \r\n before sending it to void. // if (!temp) - return _sopen ("nul", _O_RDWR | _O_BINARY | _O_NOINHERIT, _SH_DENYNO); + { + int fd (_sopen ("nul", _O_RDWR | _O_BINARY | _O_NOINHERIT, _SH_DENYNO)); + + if (fd == -1) + throw_ios_failure (errno); + + return auto_fd (fd); + } try { @@ -969,20 +981,24 @@ namespace butl // the temporary file that avoid any allocations and exceptions. // path p (path::temp_path ("null")); // Can throw. - return _sopen (p.string ().c_str (), - (_O_CREAT | - _O_RDWR | - _O_BINARY | // Don't translate. - _O_TEMPORARY | // Remove on close. - _O_SHORT_LIVED | // Don't flush to disk. - _O_NOINHERIT), // Don't inherit by child process. - _SH_DENYNO, - _S_IREAD | _S_IWRITE); + int fd (_sopen (p.string ().c_str (), + (_O_CREAT | + _O_RDWR | + _O_BINARY | // Don't translate. + _O_TEMPORARY | // Remove on close. + _O_SHORT_LIVED | // Don't flush to disk. + _O_NOINHERIT), // Don't inherit by child process. + _SH_DENYNO, + _S_IREAD | _S_IWRITE)); + + if (fd == -1) + throw_ios_failure (errno); + + return auto_fd (fd); } catch (const bad_alloc&) { - errno = ENOMEM; - return -1; + throw_ios_failure (ENOMEM); } catch (const system_error& e) { @@ -990,8 +1006,7 @@ namespace butl // assert (e.code ().category () == generic_category ()); - errno = e.code ().value (); - return -1; + throw_ios_failure (e.code ().value ()); } } diff --git a/butl/process.cxx b/butl/process.cxx index 850ba9b..c427aa8 100644 --- a/butl/process.cxx +++ b/butl/process.cxx @@ -286,13 +286,26 @@ namespace butl } }; - auto open_null = [&fail] () -> auto_fd + auto open_null = [] () -> auto_fd { - auto_fd fd (fdnull ()); - if (fd.get () == -1) - fail (false); + try + { + return fdnull (); + } + catch (const ios_base::failure& e) + { + // 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. Lets use + // EIO in this case. This is a temporary code after all. + // + const system_error* se (dynamic_cast (&e)); - return fd; + throw process_error (se != nullptr + ? se->code ().value () + : EIO); + } }; // If we are asked to open null (-2) then open "half-pipe". @@ -940,17 +953,30 @@ namespace butl throw process_error (m == nullptr ? last_error_msg () : m); }; - auto open_null = [&fail] () -> auto_fd + auto open_null = [] () -> auto_fd { // 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 (); + try + { + return fdnull (true); + } + catch (const ios_base::failure& e) + { + // 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. Lets use + // EIO in this case. This is a temporary code after all. + // + const system_error* se (dynamic_cast (&e)); - return fd; + throw process_error (se != nullptr + ? se->code ().value () + : EIO); + } }; // If we are asked to open null (-2) then open "half-pipe". -- cgit v1.1