Made File::release() return the released file descriptor.

Summary: It's convenient to get back the fd, similar to unique_ptr::release().

Test Plan: unittest

Reviewed By: tudorb@fb.com

FB internal diff: D1080426

diff --git a/folly/File.cpp b/folly/File.cpp
index 56d30b0c8ae7718f40bb4814dc0901e46e6cfe9e..98f935cc87362a51eef36d59dc379a096a48439c 100644 (file)
--- a/folly/File.cpp
+++ b/folly/File.cpp
@@ -37,8 +37,10 @@ File::File()
 
 File::File(int fd, bool ownsFd)
   : fd_(fd)
-  , ownsFd_(ownsFd)
-{}
+  , ownsFd_(ownsFd) {
+  CHECK_GE(fd, -1) << "fd must be -1 or non-negative";
+  CHECK(fd != -1 || !ownsFd) << "cannot own -1";
+}
 
 File::File(const char* name, int flags, mode_t mode)
   : fd_(::open(name, flags, mode))
@@ -53,7 +55,6 @@ File::File(const char* name, int flags, mode_t mode)
 File::File(File&& other)
   : fd_(other.fd_)
   , ownsFd_(other.ownsFd_) {
-
   other.release();
 }
 
@@ -79,9 +80,11 @@ File::~File() {
   return File(fd, true);
 }
 
-void File::release() {
+int File::release() {
+  int released = fd_;
   fd_ = -1;
   ownsFd_ = false;
+  return released;
 }
 
 void File::swap(File& other) {
@@ -95,7 +98,7 @@ void swap(File& a, File& b) {
 }
 
 File File::dup() const {
-  if (fd_ >= 0) {
+  if (fd_ != -1) {
     int fd = ::dup(fd_);
     checkUnixError(fd, "dup() failed");
 

diff --git a/folly/File.h b/folly/File.h
index 506181580844857efde6957b1ac102236b18a1f2..ec4a208579b7dfda778fcb620c9b94855ead1bcc 100644 (file)
--- a/folly/File.h
+++ b/folly/File.h
@@ -64,7 +64,7 @@ class File {
    * Returns 'true' iff the file was successfully opened.
    */
   explicit operator bool() const {
-    return fd_ >= 0;
+    return fd_ != -1;
   }
 
   /**
@@ -85,9 +85,10 @@ class File {
   bool closeNoThrow();
 
   /**
-   * Releases the file descriptor; no longer owned by this File.
+   * Returns and releases the file descriptor; no longer owned by this File.
+   * Returns -1 if the File object didn't wrap a file.
    */
-  void release();
+  int release();
 
   /**
    * Swap this File with another.

diff --git a/folly/test/FileTest.cpp b/folly/test/FileTest.cpp
index 3b5acaaa8008f2f4a880e40fb1495f2a8aaf9b7e..43eb5a1516d2f41410d96e5984f234e8df9cc165 100644 (file)
--- a/folly/test/FileTest.cpp
+++ b/folly/test/FileTest.cpp
@@ -91,6 +91,12 @@ TEST(File, OwnsFd) {
   ::close(p[0]);
 }
 
+TEST(File, Release) {
+  File in(STDOUT_FILENO, false);
+  CHECK_EQ(STDOUT_FILENO, in.release());
+  CHECK_EQ(-1, in.release());
+}
+
 #define EXPECT_CONTAINS(haystack, needle) \
   EXPECT_NE(::std::string::npos, ::folly::StringPiece(haystack).find(needle)) \
     << "Haystack: '" << haystack << "'\nNeedle: '" << needle << "'";