Switch [String]FileWriter to use new file_io.h functions/types

Also add ScopedFileHandle as cross-platform version of ScopedFD/ScopedFileHANDLE.

R=mark@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/815053004

tools/generate_dump.cc

@@ -152,8 +152,8 @@ int GenerateDumpMain(int argc, char* argv[]) {
 
     FileWriter file_writer;
     if (!file_writer.Open(base::FilePath(options.dump_path),
-                          O_WRONLY | O_CREAT | O_TRUNC,
-                          0644)) {
+                          FileWriteMode::kTruncateOrCreate,
+                          true)) {
       return EXIT_FAILURE;
     }
 

util/file/file_io.h

@@ -19,8 +19,11 @@
 
 #include "build/build_config.h"
 
-#if defined(OS_WIN)
+#if defined(OS_POSIX)
+#include "base/files/scoped_file.h"
+#elif defined(OS_WIN)
 #include <windows.h>
+#include "util/win/scoped_handle.h"
 #endif
 
 namespace base {
@@ -37,10 +40,14 @@ using FileHandle = int;
 //! \brief Platform-specific alias for a position in an open file.
 using FileOffset = off_t;
 
+//! \brief Scoped wrapper of a FileHandle.
+using ScopedFileHandle = base::ScopedFD;
+
 #elif defined(OS_WIN)
 
 using FileHandle = HANDLE;
 using FileOffset = LONGLONG;
+using ScopedFileHandle = ScopedFileHANDLE;
 
 #endif
 
@@ -152,8 +159,7 @@ void CheckedReadFileAtEOF(FileHandle file);
 //!
 //! \return The newly opened FileHandle, or an invalid FileHandle on failure.
 //!
-//! \sa ScopedFD
-//! \sa ScopedFileHANDLE
+//! \sa ScopedFileHandle
 FileHandle LoggingOpenFileForRead(const base::FilePath& path);
 
 //! \brief Wraps `open()` or `CreateFile()`, creating a file for output. Logs
@@ -168,8 +174,7 @@ FileHandle LoggingOpenFileForRead(const base::FilePath& path);
 //! \return The newly opened FileHandle, or an invalid FileHandle on failure.
 //!
 //! \sa FileWriteMode
-//! \sa ScopedFD
-//! \sa ScopedFileHANDLE
+//! \sa ScopedFileHandle
 FileHandle LoggingOpenFileForWrite(const base::FilePath& path,
                                    FileWriteMode write_mode,
                                    bool world_readable);

util/file/file_writer.cc

@@ -19,49 +19,49 @@
 #include <limits.h>
 
 #include "base/logging.h"
+
+#if defined(OS_POSIX)
+#include <sys/uio.h>
+#include <unistd.h>
 #include "base/posix/eintr_wrapper.h"
-#include "util/file/file_io.h"
+#endif  // OS_POSIX
 
 namespace crashpad {
 
+#if defined(OS_POSIX)
 // Ensure type compatibility between WritableIoVec and iovec.
 static_assert(sizeof(WritableIoVec) == sizeof(iovec), "WritableIoVec size");
 static_assert(offsetof(WritableIoVec, iov_base) == offsetof(iovec, iov_base),
               "WritableIoVec base offset");
 static_assert(offsetof(WritableIoVec, iov_len) == offsetof(iovec, iov_len),
               "WritableIoVec len offset");
+#endif  // OS_POSIX
 
-FileWriter::FileWriter() : fd_() {
+FileWriter::FileWriter() : file_() {
 }
 
 FileWriter::~FileWriter() {
 }
 
-bool FileWriter::Open(const base::FilePath& path, int oflag, mode_t mode) {
-  CHECK(!fd_.is_valid());
-
-  DCHECK((oflag & O_WRONLY) || (oflag & O_RDWR));
-
-  fd_.reset(HANDLE_EINTR(open(path.value().c_str(), oflag, mode)));
-  if (!fd_.is_valid()) {
-    PLOG(ERROR) << "open " << path.value();
-    return false;
-  }
-
-  return true;
+bool FileWriter::Open(const base::FilePath& path,
+                      FileWriteMode write_mode,
+                      bool world_readable) {
+  CHECK(!file_.is_valid());
+  file_.reset(LoggingOpenFileForWrite(path, write_mode, world_readable));
+  return file_.is_valid();
 }
 
 void FileWriter::Close() {
-  CHECK(fd_.is_valid());
+  CHECK(file_.is_valid());
 
-  fd_.reset();
+  file_.reset();
 }
 
 bool FileWriter::Write(const void* data, size_t size) {
-  DCHECK(fd_.is_valid());
+  DCHECK(file_.is_valid());
 
   // TODO(mark): Write no more than SSIZE_MAX bytes in a single call.
-  ssize_t written = WriteFile(fd_.get(), data, size);
+  ssize_t written = WriteFile(file_.get(), data, size);
   if (written < 0) {
     PLOG(ERROR) << "write";
     return false;
@@ -74,7 +74,9 @@ bool FileWriter::Write(const void* data, size_t size) {
 }
 
 bool FileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
-  DCHECK(fd_.is_valid());
+  DCHECK(file_.is_valid());
+
+#if defined(OS_POSIX)
 
   ssize_t size = 0;
   for (const WritableIoVec& iov : *iovecs) {
@@ -94,7 +96,8 @@ bool FileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
   while (size > 0) {
     size_t writev_iovec_count =
         std::min(remaining_iovecs, implicit_cast<size_t>(IOV_MAX));
-    ssize_t written = HANDLE_EINTR(writev(fd_.get(), iov, writev_iovec_count));
+    ssize_t written =
+        HANDLE_EINTR(writev(file_.get(), iov, writev_iovec_count));
     if (written < 0) {
       PLOG(ERROR) << "writev";
       return false;
@@ -128,6 +131,15 @@ bool FileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
 
   DCHECK_EQ(remaining_iovecs, 0u);
 
+#else  // !OS_POSIX
+
+  for (const WritableIoVec& iov : *iovecs) {
+    if (!Write(iov.iov_base, iov.iov_len))
+      return false;
+  }
+
+#endif  // OS_POSIX
+
 #ifndef NDEBUG
   // The interface says that |iovecs| is not sacred, so scramble it to make sure
   // that nobody depends on it.
@@ -137,15 +149,9 @@ bool FileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
   return true;
 }
 
-off_t FileWriter::Seek(off_t offset, int whence) {
-  DCHECK(fd_.is_valid());
-
-  off_t rv = lseek(fd_.get(), offset, whence);
-  if (rv < 0) {
-    PLOG(ERROR) << "lseek";
-  }
-
-  return rv;
+FileOffset FileWriter::Seek(FileOffset offset, int whence) {
+  DCHECK(file_.is_valid());
+  return LoggingSeekFile(file_.get(), offset, whence);
 }
 
 }  // namespace crashpad

util/file/file_writer.h

@@ -17,15 +17,14 @@
 
 #include <fcntl.h>
 #include <stddef.h>
-#include <sys/uio.h>
-#include <unistd.h>
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
-#include "base/files/scoped_file.h"
+#include "build/build_config.h"
+#include "util/file/file_io.h"
 
 namespace crashpad {
 
@@ -43,21 +42,21 @@ struct WritableIoVec {
   size_t iov_len;
 };
 
-//! \brief An interface to write to files and other file-like objects with POSIX
-//!     semantics.
+//! \brief An interface to write to files and other file-like objects with
+//!     semantics matching the underlying platform (POSIX or Windows).
 class FileWriterInterface {
  public:
-  //! \brief Wraps `write()` or provides an alternate implementation with
-  //!     identical semantics. This method will write the entire buffer,
-  //!     continuing after a short write or after being interrupted.
+  //! \brief Wraps WriteFile(), or provides an implementation with identical
+  //!     semantics.
   //!
   //! \return `true` if the operation succeeded, `false` if it failed, with an
   //!     error message logged.
   virtual bool Write(const void* data, size_t size) = 0;
 
-  //! \brief Wraps `writev()` or provides an alternate implementation with
-  //!     identical semantics. This method will write the entire buffer,
-  //!     continuing after a short write or after being interrupted.
+  //! \brief Wraps `writev()` on POSIX or provides an alternate implementation
+  //!     with identical semantics. This method will write entire buffers,
+  //!     continuing after a short write or after being interrupted. On
+  //!     non-POSIX this is a simple wrapper around Write().
   //!
   //! \return `true` if the operation succeeded, `false` if it failed, with an
   //!     error message logged.
@@ -65,34 +64,36 @@ class FileWriterInterface {
   //! \note The contents of \a iovecs are undefined when this method returns.
   virtual bool WriteIoVec(std::vector<WritableIoVec>* iovecs) = 0;
 
-  //! \brief Wraps `lseek()` or provides an alternate implementation with
-  //!     identical semantics.
+  //! \brief Wraps LoggingFileSeek() or provides an alternate implementation
+  //!     with identical semantics.
   //!
-  //! \return The return value of `lseek()`. `-1` on failure, with an error
-  //!     message logged.
-  virtual off_t Seek(off_t offset, int whence) = 0;
+  //! \return The return value of LoggingFileSeek(). `-1` on failure,
+  //!     with an error message logged.
+  virtual FileOffset Seek(FileOffset offset, int whence) = 0;
 
  protected:
   ~FileWriterInterface() {}
 };
 
-//! \brief A file writer implementation that wraps traditional POSIX file
+//! \brief A file writer implementation that wraps traditional system file
 //!     operations on files accessed through the filesystem.
 class FileWriter : public FileWriterInterface {
  public:
   FileWriter();
   ~FileWriter();
 
-  //! \brief Wraps `open()`.
+  //! \brief Wraps LoggingOpenFileForWrite().
   //!
   //! \return `true` if the operation succeeded, `false` if it failed, with an
   //!     error message logged.
   //!
   //! \note After a successful call, this method cannot be called again until
   //!     after Close().
-  bool Open(const base::FilePath& path, int oflag, mode_t mode);
+  bool Open(const base::FilePath& path,
+            FileWriteMode write_mode,
+            bool world_readable);
 
-  //! \brief Wraps `close().`
+  //! \brief Wraps CheckedCloseHandle().
   //!
   //! \note It is only valid to call this method on an object that has had a
   //!     successful Open() that has not yet been matched by a subsequent call
@@ -117,10 +118,10 @@ class FileWriter : public FileWriterInterface {
   //!
   //! \note It is only valid to call this method between a successful Open() and
   //!     a Close().
-  off_t Seek(off_t offset, int whence) override;
+  FileOffset Seek(FileOffset offset, int whence) override;
 
  private:
-  base::ScopedFD fd_;
+  ScopedFileHandle file_;
 
   DISALLOW_COPY_AND_ASSIGN(FileWriter);
 };

util/file/string_file_writer.cc

@@ -87,7 +87,7 @@ bool StringFileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
   return true;
 }
 
-off_t StringFileWriter::Seek(off_t offset, int whence) {
+FileOffset StringFileWriter::Seek(FileOffset offset, int whence) {
   DCHECK(offset_.IsValid());
 
   size_t base_offset;
@@ -110,21 +110,22 @@ off_t StringFileWriter::Seek(off_t offset, int whence) {
       return -1;
   }
 
-  off_t base_offset_offt;
-  if (!AssignIfInRange(&base_offset_offt, base_offset)) {
-    LOG(ERROR) << "Seek(): base_offset " << base_offset << " invalid for off_t";
+  FileOffset base_offset_fileoffset;
+  if (!AssignIfInRange(&base_offset_fileoffset, base_offset)) {
+    LOG(ERROR) << "Seek(): base_offset " << base_offset
+               << " invalid for FileOffset";
     return -1;
   }
-  base::CheckedNumeric<off_t> new_offset(base_offset_offt);
+  base::CheckedNumeric<FileOffset> new_offset(base_offset_fileoffset);
   new_offset += offset;
   if (!new_offset.IsValid()) {
     LOG(ERROR) << "Seek(): new_offset invalid";
     return -1;
   }
-  off_t new_offset_offt = new_offset.ValueOrDie();
+  FileOffset new_offset_fileoffset = new_offset.ValueOrDie();
   size_t new_offset_sizet;
-  if (!AssignIfInRange(&new_offset_sizet, new_offset_offt)) {
-    LOG(ERROR) << "Seek(): new_offset " << new_offset_offt
+  if (!AssignIfInRange(&new_offset_sizet, new_offset_fileoffset)) {
+    LOG(ERROR) << "Seek(): new_offset " << new_offset_fileoffset
                << " invalid for size_t";
     return -1;
   }

util/file/string_file_writer.h

@@ -47,7 +47,7 @@ class StringFileWriter : public FileWriterInterface {
   // FileWriterInterface:
   bool Write(const void* data, size_t size) override;
   bool WriteIoVec(std::vector<WritableIoVec>* iovecs) override;
-  off_t Seek(off_t offset, int whence) override;
+  FileOffset Seek(FileOffset offset, int whence) override;
 
  private:
   //! \brief The virtual file’s contents.

util/win/scoped_handle.cc

@@ -15,12 +15,13 @@
 #include "util/win/scoped_handle.h"
 
 #include "base/logging.h"
+#include "util/file/file_io.h"
 
 namespace crashpad {
 namespace internal {
 
 void ScopedFileHANDLECloseTraits::Free(HANDLE handle) {
-  PCHECK(CloseHandle(handle));
+  CheckedCloseFile(handle);
 }
 
 void ScopedKernelHANDLECloseTraits::Free(HANDLE handle) {