Make File::close() public.

test/gtest-extra-test.cc

@@ -171,13 +171,8 @@ bool IsClosedInternal(int fd) {
 #endif
 
 TEST(FileTest, OpenFileInCtor) {
-  int fd = 0;
-  {
-    File f(".travis.yml", File::RDONLY);
-    fd = f.get();
-    ASSERT_TRUE(IsOpen(fd));
-  }
-  EXPECT_CLOSED(fd);
+  File f(".travis.yml", File::RDONLY);
+  ASSERT_TRUE(IsOpen(f.get()));
 }
 
 TEST(FileTest, OpenFileError) {
@@ -245,20 +240,38 @@ TEST(FileTest, CloseFileInDtor) {
     File f(".travis.yml", File::RDONLY);
     fd = f.get();
   }
-  FILE *f = fdopen(fd, "r");
-  int error_code = errno;
-  if (f)
-    fclose(f);
-  EXPECT_TRUE(f == 0);
-  EXPECT_EQ(EBADF, error_code);
+  EXPECT_CLOSED(fd);
 }
 
-TEST(FileTest, CloseError) {
-  File *fd = new File(".travis.yml", File::RDONLY);
-  EXPECT_STDERR(close(fd->get()); delete fd,
+TEST(FileTest, DtorCloseError) {
+  File *f = new File(".travis.yml", File::RDONLY);
+  // The close function must be called inside EXPECT_STDERR, otherwise
+  // the system may allocate freed file descriptor when redirecting the
+  // output in EXPECT_STDERR.
+  EXPECT_STDERR(close(f->get()); delete f,
     FormatSystemErrorMessage(EBADF, "cannot close file") + "\n");
 }
 
+TEST(FileTest, Close) {
+  File f(".travis.yml", File::RDONLY);
+  int fd = f.get();
+  f.close();
+  EXPECT_EQ(-1, f.get());
+  EXPECT_CLOSED(fd);
+}
+
+TEST(FileTest, CloseError) {
+  File *f = new File(".travis.yml", File::RDONLY);
+  fmt::SystemError error("", 0);
+  std::string message = FormatSystemErrorMessage(EBADF, "cannot close file");
+  EXPECT_STDERR(
+    close(f->get());
+    try { f->close(); } catch (const fmt::SystemError &e) { error = e; }
+    delete f,
+    message + "\n");
+  EXPECT_EQ(message, error.what());
+}
+
 // Attempts to read count characters from the file.
 std::string Read(File &f, std::size_t count) {
   std::string buffer(count, '\0');
@@ -287,17 +300,15 @@ TEST(FileTest, ReadError) {
 
 TEST(FileTest, Write) {
   const char MESSAGE[] = "test";
-  File read_end;
-  {
-    File write_end;
-    File::pipe(read_end, write_end);
-    enum { SIZE = sizeof(MESSAGE) - 1 };
-    std::streamsize offset = 0, count = 0;
-    do {
-      count = write_end.write(MESSAGE + offset, SIZE - offset);
-      offset += count;
-    } while (offset < SIZE && count != 0);
-  }
+  File read_end, write_end;
+  File::pipe(read_end, write_end);
+  enum { SIZE = sizeof(MESSAGE) - 1 };
+  std::streamsize offset = 0, count = 0;
+  do {
+    count = write_end.write(MESSAGE + offset, SIZE - offset);
+    offset += count;
+  } while (offset < SIZE && count != 0);
+  write_end = File();  // Close file.
   EXPECT_READ(read_end, MESSAGE);
 }
 
@@ -355,7 +366,7 @@ TEST(FileTest, Pipe) {
   File::pipe(read_end, write_end);
   EXPECT_NE(-1, read_end.get());
   EXPECT_NE(-1, write_end.get());
-  // TODO: try writing to write_fd and reading from read_fd
+  // TODO: try writing to write_end and reading from read_end
 }
 
 // TODO: test pipe

test/gtest-extra.cc

@@ -63,13 +63,21 @@ File::File(const char *path, int oflag) {
     fmt::ThrowSystemError(errno, "cannot open file {}") << path;
 }
 
+File::~File() {
+  // Don't need to retry close in case of EINTR.
+  // See http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html
+  if (fd_ != -1 && ::FMT_POSIX(close(fd_)) != 0)
+    fmt::ReportSystemError(errno, "cannot close file");
+}
+
 void File::close() {
   if (fd_ == -1)
     return;
   // Don't need to retry close in case of EINTR.
   // See http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html
   if (::FMT_POSIX(close(fd_)) != 0)
-    fmt::ReportSystemError(errno, "cannot close file");
+    fmt::ThrowSystemError(errno, "cannot close file");
+  fd_ = -1;
 }
 
 std::streamsize File::read(void *buffer, std::size_t count) {

test/gtest-extra.h

@@ -108,9 +108,6 @@ class File {
  private:
   int fd_;  // File descriptor.
 
-  // Closes the file if its descriptor is not -1.
-  void close();
-
   // Constructs a File object with a given descriptor.
   explicit File(int fd) : fd_(fd) {}
 
@@ -188,12 +185,15 @@ class File {
 #endif
 
   // Destroys the object closing the file it represents if any.
-  ~File() { close(); }
+  ~File();
 
   // Returns the file descriptor.
-  // TODO: rename
+  // TODO: rename to descriptor
   int get() const FMT_NOEXCEPT(true) { return fd_; }
 
+  // Closes the file if its descriptor is not -1.
+  void close();
+
   // Attempts to read count bytes from the file into the specified buffer.
   std::streamsize read(void *buffer, std::size_t count);