From: Soren Lassen <soren@fb.com>
Date: Tue, 3 Dec 2013 18:40:05 +0000 (-0800)
Subject: Made File::release() return the released file descriptor.
X-Git-Tag: v0.22.0~776
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=cb394cb5128a84d55c52e6ad8c6f8a5a0db00e32;p=folly.git

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 56d30b0c..98f935cc 100644
--- 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 50618158..ec4a2085 100644
--- 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 3b5acaaa..43eb5a15 100644
--- 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 << "'";