Accept longer Settings::Data structs

This attempts to be somewhat forward-compatible with upcoming additions
to the Data struct, most importantly to not lose the client ID if we
ever need to downgrade / read data from a future crashpad version.

Bug: 42310127
Change-Id: Ic3914fdd8460f4f41e5bb523d5c52361767880dd
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5915193
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
Commit-Queue: Peter Boström <pbos@chromium.org>

client/crash_report_database_test.cc

@@ -979,6 +979,50 @@ TEST_F(CrashReportDatabaseTest, GetReportSize_RightSizeWithAttachments) {
                 sizeof(attachment_2_data));
 }
 
+TEST_F(CrashReportDatabaseTest, InitializeFromLargerFileRetainsClientId) {
+  // Initialize the database for the first time, creating it.
+  ASSERT_TRUE(db());
+
+  const base::FilePath settings_path =
+      path().Append(FILE_PATH_LITERAL("settings.dat"));
+  EXPECT_FALSE(FileExists(settings_path));
+
+  Settings* settings = db()->GetSettings();
+  ASSERT_TRUE(settings);
+  EXPECT_TRUE(FileExists(settings_path));
+
+  UUID client_id;
+  ASSERT_TRUE(settings->GetClientID(&client_id));
+  EXPECT_NE(client_id, UUID());
+
+  // Close and reopen the database at the same path.
+  ResetDatabase();
+  EXPECT_FALSE(db());
+  EXPECT_TRUE(FileExists(settings_path));
+
+  // Append some data, to ensure that we can open settings even if a future
+  // version has added additional field to Settings (forwards compatible).
+  FileWriter settings_writer;
+  ASSERT_TRUE(settings_writer.Open(
+      settings_path, FileWriteMode::kReuseOrFail, FilePermissions::kOwnerOnly));
+  ASSERT_NE(settings_writer.Seek(0, SEEK_END), 0u);
+  constexpr uint64_t extra_garbage = 0xBADF00D;
+  ASSERT_TRUE(settings_writer.Write(&extra_garbage, sizeof(extra_garbage)));
+  settings_writer.Close();
+
+  auto db = CrashReportDatabase::InitializeWithoutCreating(path());
+  ASSERT_TRUE(db);
+
+  settings = db->GetSettings();
+  ASSERT_TRUE(settings);
+
+  // Make sure that the reopened settings retained the original client id and
+  // wasn't recreated.
+  UUID reopened_client_id;
+  ASSERT_TRUE(settings->GetClientID(&reopened_client_id));
+  EXPECT_EQ(client_id, reopened_client_id);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

client/settings.cc

@@ -15,6 +15,7 @@
 #include "client/settings.h"
 
 #include <stdint.h>
+#include <string.h>
 
 #include <limits>
 
@@ -117,6 +118,10 @@ void ScopedLockedFileHandleTraits::Free(FileHandle handle) {
 
 struct Settings::Data {
   static constexpr uint32_t kSettingsMagic = 'CPds';
+
+  // Version number only used for incompatible changes to Data. Do not change
+  // this when adding additional fields at the end. Modifying `kSettingsVersion`
+  // will wipe away the entire struct when reading from other versions.
   static constexpr uint32_t kSettingsVersion = 1;
 
   enum Options : uint32_t {
@@ -389,16 +394,33 @@ Settings::ScopedLockedFileHandle Settings::OpenForWritingAndReadSettings(
 bool Settings::ReadSettings(FileHandle handle,
                             Data* out_data,
                             bool log_read_error) {
-  if (LoggingSeekFile(handle, 0, SEEK_SET) != 0)
+  if (LoggingSeekFile(handle, 0, SEEK_SET) != 0) {
     return false;
+  }
 
-  bool read_result =
-      log_read_error
-          ? LoggingReadFileExactly(handle, out_data, sizeof(*out_data))
-          : ReadFileExactly(handle, out_data, sizeof(*out_data));
+  // This clears `out_data` so that any bytes not read from disk are zero
+  // initialized. This is expected when reading from an older settings file with
+  // fewer fields.
+  memset(out_data, 0, sizeof(*out_data));
 
-  if (!read_result)
+  const FileOperationResult read_result =
+      log_read_error ? LoggingReadFileUntil(handle, out_data, sizeof(*out_data))
+                     : ReadFileUntil(handle, out_data, sizeof(*out_data));
+
+  if (read_result <= 0) {
     return false;
+  }
+
+  // Newer versions of crashpad may add fields to Data, but all versions have
+  // the data members up to `client_id`. Do not attempt to understand a smaller
+  // struct read.
+  const size_t min_size =
+      offsetof(Data, client_id) + sizeof(out_data->client_id);
+  if (static_cast<size_t>(read_result) < min_size) {
+    LOG(ERROR) << "Settings file too small: minimum " << min_size
+               << ", observed " << read_result;
+    return false;
+  }
 
   if (out_data->magic != Data::kSettingsMagic) {
     LOG(ERROR) << "Settings magic is not " << Data::kSettingsMagic;

util/file/file_io.cc

@@ -14,6 +14,8 @@
 
 #include "util/file/file_io.h"
 
+#include <functional>
+
 #include "base/check_op.h"
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
@@ -22,29 +24,17 @@ namespace crashpad {
 
 namespace {
 
-class FileIOReadExactly final : public internal::ReadExactlyInternal {
- public:
-  explicit FileIOReadExactly(FileHandle file)
-      : ReadExactlyInternal(), file_(file) {}
-
-  FileIOReadExactly(const FileIOReadExactly&) = delete;
-  FileIOReadExactly& operator=(const FileIOReadExactly&) = delete;
-
-  ~FileIOReadExactly() {}
-
- private:
-  // ReadExactlyInternal:
-  FileOperationResult Read(void* buffer, size_t size, bool can_log) override {
-    FileOperationResult rv = ReadFile(file_, buffer, size);
-    if (rv < 0) {
-      PLOG_IF(ERROR, can_log) << internal::kNativeReadFunctionName;
-      return -1;
-    }
-    return rv;
+FileOperationResult FileIORead(FileHandle file,
+                               bool can_log,
+                               void* buffer,
+                               size_t size) {
+  FileOperationResult rv = ReadFile(file, buffer, size);
+  if (rv < 0) {
+    PLOG_IF(ERROR, can_log) << internal::kNativeReadFunctionName;
+    return -1;
   }
-
-  FileHandle file_;
-};
+  return rv;
+}
 
 class FileIOWriteAll final : public internal::WriteAllInternal {
  public:
@@ -64,19 +54,21 @@ class FileIOWriteAll final : public internal::WriteAllInternal {
   FileHandle file_;
 };
 
-}  // namespace
-
-namespace internal {
-
-bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) {
+FileOperationResult ReadUntil(
+    std::function<FileOperationResult(void*, size_t)> read_function,
+    void* buffer,
+    size_t size) {
+  // Ensure bytes read fit within int32_t::max to make sure that they also fit
+  // into FileOperationResult on all platforms.
+  DCHECK_LE(size, size_t{std::numeric_limits<int32_t>::max()});
   uintptr_t buffer_int = reinterpret_cast<uintptr_t>(buffer);
   size_t total_bytes = 0;
   size_t remaining = size;
   while (remaining > 0) {
-    FileOperationResult bytes_read =
-        Read(reinterpret_cast<char*>(buffer_int), remaining, can_log);
+    const FileOperationResult bytes_read =
+        read_function(reinterpret_cast<char*>(buffer_int), remaining);
     if (bytes_read < 0) {
-      return false;
+      return bytes_read;
     }
 
     DCHECK_LE(static_cast<size_t>(bytes_read), remaining);
@@ -89,10 +81,27 @@ bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) {
     remaining -= bytes_read;
     total_bytes += bytes_read;
   }
+  return total_bytes;
+}
 
-  if (total_bytes != size) {
+}  // namespace
+
+namespace internal {
+
+bool ReadExactly(
+    std::function<FileOperationResult(bool, void*, size_t)> read_function,
+    bool can_log,
+    void* buffer,
+    size_t size) {
+  const FileOperationResult result =
+      ReadUntil(std::bind_front(read_function, can_log), buffer, size);
+  if (result < 0) {
+    return false;
+  }
+
+  if (static_cast<size_t>(result) != size) {
     LOG_IF(ERROR, can_log) << "ReadExactly: expected " << size << ", observed "
-                           << total_bytes;
+                           << result;
     return false;
   }
 
@@ -121,13 +130,23 @@ bool WriteAllInternal::WriteAll(const void* buffer, size_t size) {
 }  // namespace internal
 
 bool ReadFileExactly(FileHandle file, void* buffer, size_t size) {
-  FileIOReadExactly read_exactly(file);
-  return read_exactly.ReadExactly(buffer, size, false);
+  return internal::ReadExactly(
+      std::bind_front(&FileIORead, file), false, buffer, size);
+}
+
+FileOperationResult ReadFileUntil(FileHandle file, void* buffer, size_t size) {
+  return ReadUntil(std::bind_front(&FileIORead, file, false), buffer, size);
 }
 
 bool LoggingReadFileExactly(FileHandle file, void* buffer, size_t size) {
-  FileIOReadExactly read_exactly(file);
-  return read_exactly.ReadExactly(buffer, size, true);
+  return internal::ReadExactly(
+      std::bind_front(&FileIORead, file), true, buffer, size);
+}
+
+FileOperationResult LoggingReadFileUntil(FileHandle file,
+                                         void* buffer,
+                                         size_t size) {
+  return ReadUntil(std::bind_front(&FileIORead, file, true), buffer, size);
 }
 
 bool WriteFile(FileHandle file, const void* buffer, size_t size) {

util/file/file_io.h

@@ -17,6 +17,7 @@
 
 #include <sys/types.h>
 
+#include <functional>
 #include <string>
 
 #include "build/build_config.h"
@@ -162,30 +163,11 @@ constexpr char kNativeWriteFunctionName[] = "WriteFile";
 //! intended to be used more generally. Use ReadFileExactly(),
 //! LoggingReadFileExactly(), CheckedReadFileExactly(), or
 //! FileReaderInterface::ReadExactly() instead.
-class ReadExactlyInternal {
- public:
-  ReadExactlyInternal(const ReadExactlyInternal&) = delete;
-  ReadExactlyInternal& operator=(const ReadExactlyInternal&) = delete;
-
-  //! \brief Calls Read(), retrying following a short read, ensuring that
-  //!     exactly \a size bytes are read.
-  //!
-  //! \return `true` on success. `false` if the underlying Read() fails or if
-  //!     fewer than \a size bytes were read. When returning `false`, if \a
-  //!     can_log is `true`, logs a message.
-  bool ReadExactly(void* buffer, size_t size, bool can_log);
-
- protected:
-  ReadExactlyInternal() {}
-  ~ReadExactlyInternal() {}
-
- private:
-  //! \brief Wraps a read operation, such as ReadFile().
-  //!
-  //! \return The number of bytes read and placed into \a buffer, or `-1` on
-  //!     error. When returning `-1`, if \a can_log is `true`, logs a message.
-  virtual FileOperationResult Read(void* buffer, size_t size, bool can_log) = 0;
-};
+bool ReadExactly(
+    std::function<FileOperationResult(bool, void*, size_t)> read_function,
+    bool can_log,
+    void* buffer,
+    size_t size);
 
 //! \brief The internal implementation of WriteFile() and its wrappers.
 //!
@@ -252,7 +234,9 @@ FileOperationResult NativeWriteFile(FileHandle file,
 //!
 //! \sa WriteFile
 //! \sa ReadFileExactly
+//! \sa ReadFileUntil
 //! \sa LoggingReadFileExactly
+//! \sa LoggingReadFileUntil
 //! \sa CheckedReadFileExactly
 //! \sa CheckedReadFileAtEOF
 FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size);
@@ -273,33 +257,62 @@ FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size);
 bool WriteFile(FileHandle file, const void* buffer, size_t size);
 
 //! \brief Wraps ReadFile(), retrying following a short read, ensuring that
-//!     exactly \a size bytes are read.
+//!     exactly \a size bytes are read. Does not log on failure.
 //!
-//! \return `true` on success. If the underlying ReadFile() fails, or if fewer
-//!     than \a size bytes were read, this function logs a message and
-//!     returns `false`.
+//! \return `true` on success. Returns `false` if the underlying ReadFile()
+//!     fails or if fewer than \a size bytes were read.
 //!
 //! \sa LoggingWriteFile
 //! \sa ReadFile
+//! \sa ReadFileUntil
 //! \sa LoggingReadFileExactly
+//! \sa LoggingReadFileUntil
 //! \sa CheckedReadFileExactly
 //! \sa CheckedReadFileAtEOF
 bool ReadFileExactly(FileHandle file, void* buffer, size_t size);
 
-//! \brief Wraps ReadFile(), retrying following a short read, ensuring that
-//!     exactly \a size bytes are read.
+//! \brief Wraps ReadFile(), retrying following a short read. Does not log on
+//!     failure.
 //!
-//! \return `true` on success. If the underlying ReadFile() fails, or if fewer
-//!     than \a size bytes were read, this function logs a message and
-//!     returns `false`.
+//! \returns The number of bytes read or `-1` if the underlying ReadFile()
+//!     fails.
+//!
+//! \sa ReadFile
+//! \sa ReadFileExactly
+//! \sa LoggingReadFileExactly
+//! \sa LoggingReadFileUntil
+FileOperationResult ReadFileUntil(FileHandle file, void* buffer, size_t size);
+
+//! \brief Wraps ReadFile(), retrying following a short read, ensuring that
+//!     exactly \a size bytes are read. Logs an error on failure.
+//!
+//! \return `true` on success. Returns `false` if the underlying ReadFile()
+//!     fails or if fewer than \a size bytes were read.
 //!
 //! \sa LoggingWriteFile
 //! \sa ReadFile
 //! \sa ReadFileExactly
+//! \sa ReadFileUntil
+//! \sa LoggingReadFileUntil
 //! \sa CheckedReadFileExactly
 //! \sa CheckedReadFileAtEOF
 bool LoggingReadFileExactly(FileHandle file, void* buffer, size_t size);
 
+//! \brief Wraps ReadFile(), retrying following a short read. Logs an error on
+//!     failure.
+//!
+//! \returns The number of bytes read or `-1` if the underlying ReadFile()
+//!     fails.
+//!
+//! \sa ReadFileExactly
+//! \sa ReadFileUntil
+//! \sa LoggingReadFileExactly
+//! \sa CheckedReadFileExactly
+//! \sa CheckedReadFileAtEOF
+FileOperationResult LoggingReadFileUntil(FileHandle file,
+                                         void* buffer,
+                                         size_t size);
+
 //! \brief Wraps WriteFile(), ensuring that exactly \a size bytes are written.
 //!
 //! \return `true` on success. If the underlying WriteFile() fails, or if fewer

util/file/file_io_test.cc

@@ -16,6 +16,7 @@
 
 #include <stdio.h>
 
+#include <functional>
 #include <iterator>
 #include <limits>
 #include <type_traits>
@@ -39,202 +40,204 @@ using testing::_;
 using testing::InSequence;
 using testing::Return;
 
-class MockReadExactly : public internal::ReadExactlyInternal {
+class MockReadExactly {
  public:
-  MockReadExactly() : ReadExactlyInternal() {}
+  MockReadExactly() = default;
 
   MockReadExactly(const MockReadExactly&) = delete;
   MockReadExactly& operator=(const MockReadExactly&) = delete;
 
-  ~MockReadExactly() {}
+  ~MockReadExactly() = default;
 
   // Since it’s more convenient for the test to use uintptr_t than void*,
   // ReadExactlyInt() and ReadInt() adapt the types.
 
-  bool ReadExactlyInt(uintptr_t data, size_t size, bool can_log) {
-    return ReadExactly(reinterpret_cast<void*>(data), size, can_log);
+  bool ReadExactlyInt(bool can_log, uintptr_t data, size_t size) {
+    return internal::ReadExactly(std::bind_front(&MockReadExactly::Read, this),
+                                 can_log,
+                                 reinterpret_cast<void*>(data),
+                                 size);
   }
 
-  MOCK_METHOD(FileOperationResult, ReadInt, (uintptr_t, size_t, bool));
+  MOCK_METHOD(FileOperationResult, ReadInt, (bool, uintptr_t, size_t));
 
-  // ReadExactlyInternal:
-  FileOperationResult Read(void* data, size_t size, bool can_log) {
-    return ReadInt(reinterpret_cast<uintptr_t>(data), size, can_log);
+  FileOperationResult Read(bool can_log, void* data, size_t size) {
+    return ReadInt(can_log, reinterpret_cast<uintptr_t>(data), size);
   }
 };
 
 TEST(FileIO, ReadExactly_Zero) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(_, _, false)).Times(0);
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(100, 0, false));
+  EXPECT_CALL(read_exactly, ReadInt(false, _, _)).Times(0);
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 100, 0));
 }
 
 TEST(FileIO, ReadExactly_SingleSmallSuccess) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(1000, 1, false)).WillOnce(Return(1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(1000, 1, false));
+  EXPECT_CALL(read_exactly, ReadInt(false, 1000, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 1000, 1));
 }
 
 TEST(FileIO, ReadExactly_SingleSmallSuccessCanLog) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(1000, 1, true)).WillOnce(Return(1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(1000, 1, true));
+  EXPECT_CALL(read_exactly, ReadInt(true, 1000, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(true, 1000, 1));
 }
 
 TEST(FileIO, ReadExactly_SingleSmallFailure) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(1000, 1, false)).WillOnce(Return(-1));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(1000, 1, false));
+  EXPECT_CALL(read_exactly, ReadInt(false, 1000, 1)).WillOnce(Return(-1));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(false, 1000, 1));
 }
 
 TEST(FileIO, ReadExactly_SingleSmallFailureCanLog) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(1000, 1, true)).WillOnce(Return(-1));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(1000, 1, true));
+  EXPECT_CALL(read_exactly, ReadInt(true, 1000, 1)).WillOnce(Return(-1));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(true, 1000, 1));
 }
 
 TEST(FileIO, ReadExactly_DoubleSmallSuccess) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0x1000, 2, false)).WillOnce(Return(1));
-  EXPECT_CALL(read_exactly, ReadInt(0x1001, 1, false)).WillOnce(Return(1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(0x1000, 2, false));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x1000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x1001, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 0x1000, 2));
 }
 
 TEST(FileIO, ReadExactly_DoubleSmallShort) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0x20000, 2, false)).WillOnce(Return(1));
-  EXPECT_CALL(read_exactly, ReadInt(0x20001, 1, false)).WillOnce(Return(0));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(0x20000, 2, false));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x20000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x20001, 1)).WillOnce(Return(0));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(false, 0x20000, 2));
 }
 
 TEST(FileIO, ReadExactly_DoubleSmallShortCanLog) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0x20000, 2, true)).WillOnce(Return(1));
-  EXPECT_CALL(read_exactly, ReadInt(0x20001, 1, true)).WillOnce(Return(0));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(0x20000, 2, true));
+  EXPECT_CALL(read_exactly, ReadInt(true, 0x20000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(true, 0x20001, 1)).WillOnce(Return(0));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(true, 0x20000, 2));
 }
 
 TEST(FileIO, ReadExactly_Medium) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0x80000000, 0x20000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x80000000, 0x20000000))
       .WillOnce(Return(0x10000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x90000000, 0x10000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x90000000, 0x10000000))
       .WillOnce(Return(0x8000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x98000000, 0x8000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x98000000, 0x8000000))
       .WillOnce(Return(0x4000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9c000000, 0x4000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9c000000, 0x4000000))
       .WillOnce(Return(0x2000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9e000000, 0x2000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9e000000, 0x2000000))
       .WillOnce(Return(0x1000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9f000000, 0x1000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9f000000, 0x1000000))
       .WillOnce(Return(0x800000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9f800000, 0x800000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9f800000, 0x800000))
       .WillOnce(Return(0x400000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fc00000, 0x400000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fc00000, 0x400000))
       .WillOnce(Return(0x200000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fe00000, 0x200000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fe00000, 0x200000))
       .WillOnce(Return(0x100000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ff00000, 0x100000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ff00000, 0x100000))
       .WillOnce(Return(0x80000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ff80000, 0x80000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ff80000, 0x80000))
       .WillOnce(Return(0x40000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffc0000, 0x40000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffc0000, 0x40000))
       .WillOnce(Return(0x20000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffe0000, 0x20000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffe0000, 0x20000))
       .WillOnce(Return(0x10000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fff0000, 0x10000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fff0000, 0x10000))
       .WillOnce(Return(0x8000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fff8000, 0x8000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fff8000, 0x8000))
       .WillOnce(Return(0x4000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffc000, 0x4000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffc000, 0x4000))
       .WillOnce(Return(0x2000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffe000, 0x2000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffe000, 0x2000))
       .WillOnce(Return(0x1000));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffff000, 0x1000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffff000, 0x1000))
       .WillOnce(Return(0x800));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffff800, 0x800, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffff800, 0x800))
       .WillOnce(Return(0x400));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffc00, 0x400, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffc00, 0x400))
       .WillOnce(Return(0x200));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffe00, 0x200, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffe00, 0x200))
       .WillOnce(Return(0x100));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffff00, 0x100, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffff00, 0x100))
       .WillOnce(Return(0x80));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffff80, 0x80, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffff80, 0x80))
       .WillOnce(Return(0x40));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffffc0, 0x40, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffffc0, 0x40))
       .WillOnce(Return(0x20));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffffe0, 0x20, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffffe0, 0x20))
       .WillOnce(Return(0x10));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffff0, 0x10, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffff0, 0x10))
       .WillOnce(Return(0x8));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffff8, 0x8, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffff8, 0x8))
       .WillOnce(Return(0x4));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffffc, 0x4, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffffc, 0x4))
       .WillOnce(Return(0x2));
-  EXPECT_CALL(read_exactly, ReadInt(0x9ffffffe, 0x2, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9ffffffe, 0x2))
       .WillOnce(Return(0x1));
-  EXPECT_CALL(read_exactly, ReadInt(0x9fffffff, 0x1, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x9fffffff, 0x1))
       .WillOnce(Return(0x1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(0x80000000, 0x20000000, false));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 0x80000000, 0x20000000));
 }
 
 TEST(FileIO, ReadExactly_LargeSuccess) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  constexpr size_t max = std::numeric_limits<uint32_t>::max();
-  constexpr size_t increment = std::numeric_limits<int32_t>::max();
-  EXPECT_CALL(read_exactly, ReadInt(0, max, false)).WillOnce(Return(increment));
-  EXPECT_CALL(read_exactly, ReadInt(increment, max - increment, false))
+  constexpr size_t max = std::numeric_limits<int32_t>::max();
+  constexpr size_t increment = max / 2;
+  EXPECT_CALL(read_exactly, ReadInt(false, 0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(false, increment, max - increment))
       .WillOnce(Return(increment));
-  EXPECT_CALL(read_exactly, ReadInt(2 * increment, 1, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 2 * increment, 1))
       .WillOnce(Return(1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(0, max, false));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 0, max));
 }
 
 TEST(FileIO, ReadExactly_LargeShort) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0, 0xffffffff, false))
-      .WillOnce(Return(0x7fffffff));
-  EXPECT_CALL(read_exactly, ReadInt(0x7fffffff, 0x80000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0, 0x7fffffff))
+      .WillOnce(Return(0x3fffffff));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x3fffffff, 0x40000000))
       .WillOnce(Return(0x10000000));
-  EXPECT_CALL(read_exactly, ReadInt(0x8fffffff, 0x70000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x4fffffff, 0x30000000))
       .WillOnce(Return(0));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(0, 0xffffffff, false));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(false, 0, 0x7fffffff));
 }
 
 TEST(FileIO, ReadExactly_LargeFailure) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  EXPECT_CALL(read_exactly, ReadInt(0, 0xffffffff, false))
-      .WillOnce(Return(0x7fffffff));
-  EXPECT_CALL(read_exactly, ReadInt(0x7fffffff, 0x80000000, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 0, 0x7fffffff))
+      .WillOnce(Return(0x3fffffff));
+  EXPECT_CALL(read_exactly, ReadInt(false, 0x3fffffff, 0x40000000))
       .WillOnce(Return(-1));
-  EXPECT_FALSE(read_exactly.ReadExactlyInt(0, 0xffffffff, false));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(false, 0, 0x7fffffff));
 }
 
 TEST(FileIO, ReadExactly_TripleMax) {
   MockReadExactly read_exactly;
   InSequence in_sequence;
-  constexpr size_t max = std::numeric_limits<size_t>::max();
-  constexpr size_t increment =
-      std::numeric_limits<std::make_signed<size_t>::type>::max();
-  EXPECT_CALL(read_exactly, ReadInt(0, max, false)).WillOnce(Return(increment));
-  EXPECT_CALL(read_exactly, ReadInt(increment, max - increment, false))
+  // ReadExactly supports at most int32_t bytes.
+  constexpr size_t max = std::numeric_limits<int32_t>::max();
+  constexpr size_t increment = max / 2;
+  EXPECT_CALL(read_exactly, ReadInt(false, 0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(false, increment, max - increment))
       .WillOnce(Return(increment));
-  EXPECT_CALL(read_exactly, ReadInt(2 * increment, 1, false))
+  EXPECT_CALL(read_exactly, ReadInt(false, 2 * increment, 1))
       .WillOnce(Return(1));
-  EXPECT_TRUE(read_exactly.ReadExactlyInt(0, max, false));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(false, 0, max));
 }
 
 class MockWriteAll : public internal::WriteAllInternal {

util/file/file_reader.cc

@@ -21,33 +21,15 @@
 
 namespace crashpad {
 
-namespace {
-
-class FileReaderReadExactly final : public internal::ReadExactlyInternal {
- public:
-  explicit FileReaderReadExactly(FileReaderInterface* file_reader)
-      : ReadExactlyInternal(), file_reader_(file_reader) {}
-
-  FileReaderReadExactly(const FileReaderReadExactly&) = delete;
-  FileReaderReadExactly& operator=(const FileReaderReadExactly&) = delete;
-
-  ~FileReaderReadExactly() {}
-
- private:
-  // ReadExactlyInternal:
-  FileOperationResult Read(void* buffer, size_t size, bool can_log) override {
-    DCHECK(can_log);
-    return file_reader_->Read(buffer, size);
-  }
-
-  FileReaderInterface* file_reader_;  // weak
-};
-
-}  // namespace
-
 bool FileReaderInterface::ReadExactly(void* data, size_t size) {
-  FileReaderReadExactly read_exactly(this);
-  return read_exactly.ReadExactly(data, size, true);
+  return internal::ReadExactly(
+      [this](bool can_log, void* buffer, size_t size) {
+        DCHECK(can_log);
+        return Read(buffer, size);
+      },
+      true,
+      data,
+      size);
 }
 
 WeakFileHandleFileReader::WeakFileHandleFileReader(FileHandle file_handle)

util/file/file_reader_test.cc

@@ -156,8 +156,8 @@ TEST(FileReader, ReadExactly_Medium) {
 TEST(FileReader, ReadExactly_LargeSuccess) {
   MockFileReader file_reader;
   InSequence in_sequence;
-  constexpr size_t max = std::numeric_limits<uint32_t>::max();
-  constexpr size_t increment = std::numeric_limits<int32_t>::max();
+  constexpr size_t max = std::numeric_limits<int32_t>::max();
+  constexpr size_t increment = max / 2;
   EXPECT_CALL(file_reader, ReadInt(0, max)).WillOnce(Return(increment));
   EXPECT_CALL(file_reader, ReadInt(increment, max - increment))
       .WillOnce(Return(increment));
@@ -169,30 +169,30 @@ TEST(FileReader, ReadExactly_LargeSuccess) {
 TEST(FileReader, ReadExactly_LargeShort) {
   MockFileReader file_reader;
   InSequence in_sequence;
-  EXPECT_CALL(file_reader, ReadInt(0, 0xffffffff)).WillOnce(Return(0x7fffffff));
-  EXPECT_CALL(file_reader, ReadInt(0x7fffffff, 0x80000000))
+  EXPECT_CALL(file_reader, ReadInt(0, 0x7fffffff)).WillOnce(Return(0x3fffffff));
+  EXPECT_CALL(file_reader, ReadInt(0x3fffffff, 0x40000000))
       .WillOnce(Return(0x10000000));
-  EXPECT_CALL(file_reader, ReadInt(0x8fffffff, 0x70000000)).WillOnce(Return(0));
+  EXPECT_CALL(file_reader, ReadInt(0x4fffffff, 0x30000000)).WillOnce(Return(0));
   EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
-  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0xffffffff));
+  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0x7fffffff));
 }
 
 TEST(FileReader, ReadExactly_LargeFailure) {
   MockFileReader file_reader;
   InSequence in_sequence;
-  EXPECT_CALL(file_reader, ReadInt(0, 0xffffffff)).WillOnce(Return(0x7fffffff));
-  EXPECT_CALL(file_reader, ReadInt(0x7fffffff, 0x80000000))
+  EXPECT_CALL(file_reader, ReadInt(0, 0x7fffffff)).WillOnce(Return(0x3fffffff));
+  EXPECT_CALL(file_reader, ReadInt(0x3fffffff, 0x40000000))
       .WillOnce(Return(-1));
   EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
-  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0xffffffff));
+  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0x7fffffff));
 }
 
 TEST(FileReader, ReadExactly_TripleMax) {
   MockFileReader file_reader;
   InSequence in_sequence;
-  constexpr size_t max = std::numeric_limits<size_t>::max();
-  constexpr size_t increment =
-      std::numeric_limits<std::make_signed<size_t>::type>::max();
+  // ReadExactly supports at most int32_t bytes.
+  constexpr size_t max = std::numeric_limits<int32_t>::max();
+  constexpr size_t increment = max / 2;
   EXPECT_CALL(file_reader, ReadInt(0, max)).WillOnce(Return(increment));
   EXPECT_CALL(file_reader, ReadInt(increment, max - increment))
       .WillOnce(Return(increment));