Add BufferedFile. OutputRedirector -> OutputRedirect.

test/gtest-extra-test.cc

@@ -27,6 +27,7 @@
 
 #include "gtest-extra.h"
 
+#include <cstdio>
 #include <cstring>
 #include <algorithm>
 #include <stdexcept>
@@ -44,6 +45,26 @@ std::string FormatSystemErrorMessage(int error_code, fmt::StringRef message) {
   EXPECT_THROW_MSG(statement, fmt::SystemError, \
       FormatSystemErrorMessage(error_code, message))
 
+// Checks if the file is open by reading one character from it.
+bool IsOpen(int fd) {
+  char buffer;
+  return FMT_POSIX(read(fd, &buffer, 1)) == 1;
+}
+
+bool IsClosedInternal(int fd) {
+  char buffer;
+  std::streamsize result = FMT_POSIX(read(fd, &buffer, 1));
+  return result == -1 && errno == EBADF;
+}
+
+#ifndef _WIN32
+// Checks if the file is closed.
+# define EXPECT_CLOSED(fd) EXPECT_TRUE(IsClosedInternal(fd))
+#else
+// Reading from a closed file causes death on Windows.
+# define EXPECT_CLOSED(fd) EXPECT_DEATH(IsClosedInternal(fd), "")
+#endif
+
 #ifndef _WIN32
 # define EXPECT_SYSTEM_ERROR_OR_DEATH(statement, error_code, message) \
     EXPECT_SYSTEM_ERROR(statement, error_code, message)
@@ -154,30 +175,94 @@ TEST(ErrorCodeTest, Ctor) {
   EXPECT_EQ(42, ErrorCode(42).get());
 }
 
-TEST(FileTest, DefaultCtor) {
-  File f;
-  EXPECT_EQ(-1, f.descriptor());
+TEST(BufferedFileTest, DefaultCtor) {
+  BufferedFile f;
+  EXPECT_TRUE(f.get() == 0);
 }
 
-// Checks if the file is open by reading one character from it.
-bool IsOpen(int fd) {
-  char buffer;
-  return FMT_POSIX(read(fd, &buffer, 1)) == 1;
+BufferedFile OpenFile(const char *name, FILE **fp = 0) {
+  BufferedFile f = File(".travis.yml", File::RDONLY).fdopen("r");
+  if (fp)
+    *fp = f.get();
+  return f;
 }
 
-bool IsClosedInternal(int fd) {
-  char buffer;
-  std::streamsize result = FMT_POSIX(read(fd, &buffer, 1));
-  return result == -1 && errno == EBADF;
+TEST(BufferedFileTest, MoveCtor) {
+  BufferedFile bf = OpenFile(".travis.yml");
+  FILE *fp = bf.get();
+  EXPECT_TRUE(fp != 0);
+  BufferedFile bf2(std::move(bf));
+  EXPECT_EQ(fp, bf2.get());
+  EXPECT_TRUE(bf.get() == 0);
 }
 
+TEST(BufferedFileTest, MoveAssignment) {
+  BufferedFile bf = OpenFile(".travis.yml");
+  FILE *fp = bf.get();
+  EXPECT_TRUE(fp != 0);
+  BufferedFile bf2;
+  bf2 = std::move(bf);
+  EXPECT_EQ(fp, bf2.get());
+  EXPECT_TRUE(bf.get() == 0);
+}
+
+TEST(BufferedFileTest, MoveAssignmentClosesFile) {
+  BufferedFile bf = OpenFile(".travis.yml");
+  BufferedFile bf2 = OpenFile("CMakeLists.txt");
+  int old_fd = fileno(bf2.get());
+  bf2 = std::move(bf);
+  EXPECT_CLOSED(old_fd);
+}
+
+TEST(BufferedFileTest, MoveFromTemporaryInCtor) {
+  FILE *fp = 0;
+  BufferedFile f(OpenFile(".travis.yml", &fp));
+  EXPECT_EQ(fp, f.get());
+}
+
+TEST(BufferedFileTest, MoveFromTemporaryInAssignment) {
+  FILE *fp = 0;
+  BufferedFile f;
+  f = OpenFile(".travis.yml", &fp);
+  EXPECT_EQ(fp, f.get());
+}
+
+TEST(BufferedFileTest, MoveFromTemporaryInAssignmentClosesFile) {
+  BufferedFile f = OpenFile(".travis.yml");
+  int old_fd = fileno(f.get());
+  f = OpenFile(".travis.yml");
+  EXPECT_CLOSED(old_fd);
+}
+
+TEST(BufferedFileTest, CloseFileInDtor) {
+  int fd = 0;
+  {
+    BufferedFile f = OpenFile(".travis.yml");
+    fd = fileno(f.get());
+  }
+  EXPECT_CLOSED(fd);
+}
+
+TEST(BufferedFileTest, DtorCloseError) {
+  BufferedFile *f = new BufferedFile(OpenFile(".travis.yml"));
 #ifndef _WIN32
-// Checks if the file is closed.
-# define EXPECT_CLOSED(fd) EXPECT_TRUE(IsClosedInternal(fd))
+  // The close function must be called inside EXPECT_STDERR, otherwise
+  // the system may recycle closed file descriptor when redirecting the
+  // output in EXPECT_STDERR and the second close will break output
+  // redirection.
+  EXPECT_STDERR(close(fileno(f->get())); delete f,
+    FormatSystemErrorMessage(EBADF, "cannot close file") + "\n");
 #else
-// Reading from a closed file causes death on Windows.
-# define EXPECT_CLOSED(fd) EXPECT_DEATH(IsClosedInternal(fd), "")
+  FMT_POSIX(close(fileno(f->get())));
+  // Closing file twice causes death on Windows.
+  EXPECT_DEATH(delete f, "");
 #endif
+}
+
+TEST(FileTest, DefaultCtor) {
+  File f;
+  EXPECT_EQ(-1, f.descriptor());
+}
 
 TEST(FileTest, OpenFileInCtor) {
   File f(".travis.yml", File::RDONLY);
@@ -277,27 +362,22 @@ TEST(FileTest, Close) {
 }
 
 TEST(FileTest, CloseError) {
-  File *f = new File(".travis.yml", File::RDONLY);
+  File f(".travis.yml", File::RDONLY);
 #ifndef _WIN32
-  fmt::SystemError error("", 0);
-  std::string message = FormatSystemErrorMessage(EBADF, "cannot close file");
   // The close function must be called inside EXPECT_STDERR, otherwise
   // the system may recycle closed file descriptor when redirecting the
   // output in EXPECT_STDERR and the second close will break output
   // redirection.
-  EXPECT_STDERR(
-    close(f->descriptor());
-    try { f->close(); } catch (const fmt::SystemError &e) { error = e; }
-    delete f,
-    message + "\n");
-  EXPECT_EQ(message, error.what());
+  close(f.descriptor());
+  EXPECT_SYSTEM_ERROR(f.close(), EBADF, "cannot close file");
+  EXPECT_EQ(-1, f.descriptor());
 #else
+  // Open other before closing f or the descriptor may be recycled.
   File other(".travis.yml", File::RDONLY);
-  close(f->descriptor());
+  close(f.descriptor());
   // Closing file twice causes death on Windows.
-  EXPECT_DEATH(f->close(), "");
-  other.dup2(f->descriptor());  // "undo" close or delete will fail
-  delete f;
+  EXPECT_DEATH(f.close(), "");
+  other.dup2(f.descriptor());  // "undo" close or dtor will fail
 #endif
 }
 
@@ -412,10 +492,25 @@ TEST(FileTest, Pipe) {
   EXPECT_READ(read_end, "test");
 }
 
-// TODO: compile both with C++11 & C++98 mode
-#endif
+TEST(OutputRedirectTest, ScopedRedirect) {
+  File read_end, write_end;
+  File::pipe(read_end, write_end);
+  {
+    BufferedFile file(write_end.fdopen("w"));
+    std::fprintf(file.get(), "[[[");
+    {
+      OutputRedirect redir(file.get());
+      std::fprintf(file.get(), "censored");
+    }
+    std::fprintf(file.get(), "]]]");
+  }
+  EXPECT_READ(read_end, "[[[]]]");
+}
 
-// TODO: test OutputRedirector
+// TODO: test OutputRedirect
 // TODO: test EXPECT_STDOUT and EXPECT_STDERR
 
+// TODO: compile both with C++11 & C++98 mode
+#endif
+
 }  // namespace

test/gtest-extra.cc

@@ -51,6 +51,20 @@
     result = (expression); \
   } while (result == -1 && errno == EINTR)
 
+BufferedFile::~BufferedFile() FMT_NOEXCEPT(true) {
+  if (file_ && std::fclose(file_) != 0)
+    fmt::ReportSystemError(errno, "cannot close file");
+}
+
+void BufferedFile::close() {
+  if (!file_)
+    return;
+  int result = std::fclose(file_);
+  file_ = 0;
+  if (result != 0)
+    fmt::ThrowSystemError(errno, "cannot close file");
+}
+
 File::File(const char *path, int oflag) {
   int mode = S_IRUSR | S_IWUSR;
 #ifdef _WIN32
@@ -73,11 +87,12 @@ File::~File() FMT_NOEXCEPT(true) {
 void File::close() {
   if (fd_ == -1)
     return;
-  // Don't need to retry close in case of EINTR.
+  // Don't 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::ThrowSystemError(errno, "cannot close file");
+  int result = ::FMT_POSIX(close(fd_));
   fd_ = -1;
+  if (result != 0)
+    fmt::ThrowSystemError(errno, "cannot close file");
 }
 
 std::streamsize File::read(void *buffer, std::size_t count) {
@@ -142,7 +157,13 @@ void File::pipe(File &read_end, File &write_end) {
   write_end = File(fds[1]);
 }
 
-OutputRedirector::OutputRedirector(FILE *file) : file_(file) {
+BufferedFile File::fdopen(const char *mode) {
+  BufferedFile f(::fdopen(fd_, mode));
+  fd_ = -1;
+  return f;
+}
+
+OutputRedirect::OutputRedirect(FILE *file) : file_(file) {
   if (std::fflush(file) != 0)
     fmt::ThrowSystemError(errno, "cannot flush stream");
   int fd = FMT_POSIX(fileno(file));
@@ -155,7 +176,7 @@ OutputRedirector::OutputRedirector(FILE *file) : file_(file) {
   write_end.dup2(fd);
 }
 
-OutputRedirector::~OutputRedirector() FMT_NOEXCEPT(true) {
+OutputRedirect::~OutputRedirect() FMT_NOEXCEPT(true) {
   try {
     Restore();
   } catch (const std::exception &e) {
@@ -163,14 +184,14 @@ OutputRedirector::~OutputRedirector() FMT_NOEXCEPT(true) {
   }
 }
 
-void OutputRedirector::Restore() {
+void OutputRedirect::Restore() {
   if (std::fflush(file_) != 0)
     fmt::ThrowSystemError(errno, "cannot flush stream");
   // Restore the original file.
   original_.dup2(FMT_POSIX(fileno(file_)));
 }
 
-std::string OutputRedirector::Read() {
+std::string OutputRedirect::Read() {
   // Restore output.
   Restore();
 

test/gtest-extra.h

@@ -103,7 +103,91 @@ class ErrorCode {
   int get() const FMT_NOEXCEPT(true) { return value_; }
 };
 
+// A buffered file.
+class BufferedFile {
+ private:
+  FILE *file_;
+
+  friend class File;
+
+  explicit BufferedFile(FILE *f) : file_(f) {}
+
+  void close();
+
+ public:
+  // Constructs a BufferedFile object which doesn't represent any file.
+  BufferedFile() FMT_NOEXCEPT(true) : file_(0) {}
+
+  // Destroys the object closing the file it represents if any.
+  ~BufferedFile() FMT_NOEXCEPT(true);
+
+#if !FMT_USE_RVALUE_REFERENCES
+  // Emulate a move constructor and a move assignment operator if rvalue
+  // references are not supported.
+
+ private:
+  // A proxy object to emulate a move constructor.
+  // It is private to make it impossible call operator Proxy directly.
+  struct Proxy {
+    FILE *file;
+  };
+
+ public:
+  // A "move constructor" for moving from a temporary.
+  BufferedFile(Proxy p) FMT_NOEXCEPT(true) : file_(p.file) {}
+
+  // A "move constructor" for for moving from an lvalue.
+  BufferedFile(BufferedFile &f) FMT_NOEXCEPT(true) : file_(f.file_) {
+    f.file_ = 0;
+  }
+
+  // A "move assignment operator" for moving from a temporary.
+  BufferedFile &operator=(Proxy p) {
+    close();
+    file_ = p.file;
+    return *this;
+  }
+
+  // A "move assignment operator" for moving from an lvalue.
+  BufferedFile &operator=(BufferedFile &other) {
+    close();
+    file_ = other.file_;
+    other.file_ = 0;
+    return *this;
+  }
+
+  // Returns a proxy object for moving from a temporary:
+  //   BufferedFile file = BufferedFile(...);
+  operator Proxy() FMT_NOEXCEPT(true) {
+    Proxy p = {file_};
+    file_ = 0;
+    return p;
+  }
+#else
+ private:
+  GTEST_DISALLOW_COPY_AND_ASSIGN_(BufferedFile);
+
+ public:
+  BufferedFile(BufferedFile &&other) FMT_NOEXCEPT(true) : file_(other.file_) {
+    other.file_ = 0;
+  }
+
+  BufferedFile& operator=(BufferedFile &&other) {
+    close();
+    file_ = other.file_;
+    other.file_ = 0;
+    return *this;
+  }
+#endif
+
+  FILE *get() const { return file_; }
+};
+
 // A file.
+// Methods that are not declared with FMT_NOEXCEPT(true) may throw
+// fmt::SystemError in case of failure. Note that some errors such as
+// closing the file multiple times will cause a crash on Windows rather
+// than an exception.
 class File {
  private:
   int fd_;  // File descriptor.
@@ -123,7 +207,6 @@ class File {
   File() FMT_NOEXCEPT(true) : fd_(-1) {}
 
   // Opens a file and constructs a File object representing this file.
-  // Throws fmt::SystemError on error.
   File(const char *path, int oflag);
 
 #if !FMT_USE_RVALUE_REFERENCES
@@ -138,22 +221,22 @@ class File {
   };
 
  public:
-  // A "move" constructor for moving from a temporary.
+  // A "move constructor" for moving from a temporary.
   File(Proxy p) FMT_NOEXCEPT(true) : fd_(p.fd) {}
 
-  // A "move" constructor for for moving from an lvalue.
+  // A "move constructor" for for moving from an lvalue.
   File(File &other) FMT_NOEXCEPT(true) : fd_(other.fd_) {
     other.fd_ = -1;
   }
 
-  // A "move" assignment operator for moving from a temporary.
+  // A "move assignment operator" for moving from a temporary.
   File &operator=(Proxy p) {
     close();
     fd_ = p.fd;
     return *this;
   }
 
-  // A "move" assignment operator for moving from an lvalue.
+  // A "move assignment operator" for moving from an lvalue.
   File &operator=(File &other) {
     close();
     fd_ = other.fd_;
@@ -201,11 +284,11 @@ class File {
   std::streamsize write(const void *buffer, std::size_t count);
 
   // Duplicates a file descriptor with the dup function and returns
-  // the duplicate as a file object. Throws fmt::SystemError on error.
+  // the duplicate as a file object.
   static File dup(int fd);
 
   // Makes fd be the copy of this file descriptor, closing fd first if
-  // necessary. Throws fmt::SystemError on error.
+  // necessary.
   void dup2(int fd);
 
   // Makes fd be the copy of this file descriptor, closing fd first if
@@ -213,32 +296,35 @@ class File {
   void dup2(int fd, ErrorCode &ec) FMT_NOEXCEPT(true);
 
   // Creates a pipe setting up read_end and write_end file objects for reading
-  // and writing respectively. Throws fmt::SystemError on error.
+  // and writing respectively.
   static void pipe(File &read_end, File &write_end);
+
+  BufferedFile fdopen(const char *mode);
 };
 
 #if !FMT_USE_RVALUE_REFERENCES
 namespace std {
 // For compatibility with C++98.
+inline BufferedFile &move(BufferedFile &f) { return f; }
 inline File &move(File &f) { return f; }
 }
 #endif
 
 // Captures file output by redirecting it to a pipe.
 // The output it can handle is limited by the pipe capacity.
-class OutputRedirector {
+class OutputRedirect {
  private:
   FILE *file_;
   File original_;  // Original file passed to redirector.
   File read_end_;  // Read end of the pipe where the output is redirected.
 
-  GTEST_DISALLOW_COPY_AND_ASSIGN_(OutputRedirector);
+  GTEST_DISALLOW_COPY_AND_ASSIGN_(OutputRedirect);
 
   void Restore();
 
  public:
-  explicit OutputRedirector(FILE *file);
-  ~OutputRedirector() FMT_NOEXCEPT(true);
+  explicit OutputRedirect(FILE *file);
+  ~OutputRedirect() FMT_NOEXCEPT(true);
 
   // Restores the original file, reads output from the pipe into a string
   // and returns it.
@@ -250,7 +336,7 @@ class OutputRedirector {
   if (::testing::AssertionResult gtest_ar = ::testing::AssertionSuccess()) { \
     std::string output; \
     { \
-      OutputRedirector redir(file); \
+      OutputRedirect redir(file); \
       GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \
       output = redir.Read(); \
     } \