Make fdnull() to return auto_fd

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 <bool v>
-  static inline typename enable_if<v>::type
+  [[noreturn]] static inline typename enable_if<v>::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 <bool v>
-  static inline typename enable_if<!v>::type
+  [[noreturn]] static inline typename enable_if<!v>::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<const system_error*> (&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<const system_error*> (&e));
 
-      return fd;
+        throw process_error (se != nullptr
+                             ? se->code ().value ()
+                             : EIO);
+      }
     };
 
     // If we are asked to open null (-2) then open "half-pipe".