Helper utility to construct, returns an Expected<..>

Summary:
folly::File's throwing constructor results in many try-catches in the callsites
or bugs where the exception is not caught.

This is a helper method to return an Expected with system_error wrapped into an
exception_wrapper.

Reviewed By: yfeldblum

Differential Revision: D4995702

fbshipit-source-id: be0e22b37c21c35bf157ada598916b05dfd32631

diff --git a/folly/File.cpp b/folly/File.cpp
index 4df3274eb86c3f77fa3280abb49303bbee9da9f0..be44ce572925cb614a26d54c2b86fe86f5566e4a 100644 (file)
--- a/folly/File.cpp
+++ b/folly/File.cpp
@@ -31,14 +31,9 @@
 
 namespace folly {
 
 
 namespace folly {
 

-File::File()
-  : fd_(-1)
-  , ownsFd_(false)
-{}
-
-File::File(int fd, bool ownsFd)
-  : fd_(fd)
-  , ownsFd_(ownsFd) {
+File::File() noexcept : fd_(-1), ownsFd_(false) {}
+
+File::File(int fd, bool ownsFd) noexcept : fd_(fd), ownsFd_(ownsFd) {

   CHECK_GE(fd, -1) << "fd must be -1 or non-negative";
   CHECK(fd != -1 || !ownsFd) << "cannot own -1";
 }
   CHECK_GE(fd, -1) << "fd must be -1 or non-negative";
   CHECK(fd != -1 || !ownsFd) << "cannot own -1";
 }

diff --git a/folly/File.h b/folly/File.h
index e936ca87a8a76bf7da010b7c7f27c9abb19e3365..af6620960b6e3bb36e354a6f4417a9392f9ca25d 100644 (file)
--- a/folly/File.h
+++ b/folly/File.h
@@ -21,7 +21,10 @@
 #include <sys/types.h>
 
 #include <string>
 #include <sys/types.h>
 
 #include <string>

+#include <system_error>

 
 

+#include <folly/ExceptionWrapper.h>
+#include <folly/Expected.h>

 #include <folly/Portability.h>
 #include <folly/Range.h>
 #include <folly/portability/Unistd.h>
 #include <folly/Portability.h>
 #include <folly/Range.h>
 #include <folly/portability/Unistd.h>


@@ -36,13 +39,13 @@ class File {
   /**
    * Creates an empty File object, for late initialization.
    */
   /**
    * Creates an empty File object, for late initialization.
    */

-  File();
+  File() noexcept;

 
   /**
    * Create a File object from an existing file descriptor.
    * Takes ownership of the file descriptor if ownsFd is true.
    */
 
   /**
    * Create a File object from an existing file descriptor.
    * Takes ownership of the file descriptor if ownsFd is true.
    */

-  explicit File(int fd, bool ownsFd = false);
+  explicit File(int fd, bool ownsFd = false) noexcept;

 
   /**
    * Open and create a file object.  Throws on error.
 
   /**
    * Open and create a file object.  Throws on error.


@@ -52,6 +55,20 @@ class File {
       const std::string& name, int flags = O_RDONLY, mode_t mode = 0666);
   explicit File(StringPiece name, int flags = O_RDONLY, mode_t mode = 0666);
 
       const std::string& name, int flags = O_RDONLY, mode_t mode = 0666);
   explicit File(StringPiece name, int flags = O_RDONLY, mode_t mode = 0666);
 

+  /**
+   * All the constructors that are not noexcept can throw std::system_error.
+   * This is a helper method to use folly::Expected to chain a file open event
+   * to something else you want to do with the open fd.
+   */
+  template <typename... Args>
+  static Expected<File, exception_wrapper> makeFile(Args&&... args) noexcept {
+    try {
+      return File(std::forward<Args>(args)...);
+    } catch (const std::system_error& se) {
+      return makeUnexpected(exception_wrapper(std::current_exception(), se));
+    }
+  }
+

   ~File();
 
   /**
   ~File();
 
   /**

diff --git a/folly/test/FileTest.cpp b/folly/test/FileTest.cpp
index 73ccf0e39718f46f7b3829cd8a96e09bb4e793a1..7aec67ba97e81b7c45505814b7a6c9496e5979be 100644 (file)
--- a/folly/test/FileTest.cpp
+++ b/folly/test/FileTest.cpp
@@ -135,3 +135,13 @@ TEST(File, Truthy) {
     EXPECT_TRUE(false);
   }
 }
     EXPECT_TRUE(false);
   }
 }

+
+TEST(File, HelperCtor) {
+  File::makeFile(StringPiece("/etc/hosts")).then([](File&& f) {
+    char buf = 'x';
+    EXPECT_NE(-1, f.fd());
+    EXPECT_EQ(1, ::read(f.fd(), &buf, 1));
+    f.close();
+    EXPECT_EQ(-1, f.fd());
+  });
+}