Merge changes Idfaedbd4,I17aac1d3,I1f005f10,I01eded0b,I26846be9

* changes:
  Pass the size of the new file to the PatchWriterInterface::Init()
  Cleanup bsdiff public API.
  Expose the PatchWriter generator in the public API.
  Correct SplitPatchWriter() comment.
  Add/remove includes to match used types.

Android.bp

@@ -62,6 +62,7 @@ cc_library_static {
         "bz2_compressor.cc",
         "diff_encoder.cc",
         "patch_writer.cc",
+        "patch_writer_factory.cc",
         "split_patch_writer.cc",
         "suffix_array_index.cc",
     ],

Makefile

@@ -37,6 +37,7 @@ bsdiff_src_files := \
     bz2_compressor.cc \
     diff_encoder.cc \
     patch_writer.cc \
+    patch_writer_factory.cc \
     split_patch_writer.cc \
     suffix_array_index.cc
 

bsdiff.cc

@@ -33,11 +33,7 @@ __FBSDID("$FreeBSD: src/usr.bin/bsdiff/bsdiff/bsdiff.c,v 1.1 2005/08/06 01:59:05
 #include <sys/types.h>
 
 #include <err.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
 
 #include <algorithm>
 
@@ -47,38 +43,6 @@ __FBSDID("$FreeBSD: src/usr.bin/bsdiff/bsdiff/bsdiff.c,v 1.1 2005/08/06 01:59:05
 
 namespace bsdiff {
 
-int bsdiff(const char* old_filename, const char* new_filename,
-           const char* patch_filename) {
-	int fd;
-	uint8_t *old_buf,*new_buf;
-	ssize_t oldsize,newsize;
-
-	/* Allocate oldsize+1 bytes instead of oldsize bytes to ensure
-		that we never try to malloc(0) and get a NULL pointer */
-	if(((fd=open(old_filename,O_RDONLY,0))<0) ||
-		((oldsize=lseek(fd,0,SEEK_END))==-1) ||
-		((old_buf=static_cast<uint8_t*>(malloc(oldsize+1)))==NULL) ||
-		(lseek(fd,0,SEEK_SET)!=0) ||
-		(read(fd,old_buf,oldsize)!=oldsize) ||
-		(close(fd)==-1)) err(1,"%s",old_filename);
-
-	/* Allocate newsize+1 bytes instead of newsize bytes to ensure
-		that we never try to malloc(0) and get a NULL pointer */
-	if(((fd=open(new_filename,O_RDONLY,0))<0) ||
-		((newsize=lseek(fd,0,SEEK_END))==-1) ||
-		((new_buf = static_cast<uint8_t*>(malloc(newsize+1)))==NULL) ||
-		(lseek(fd,0,SEEK_SET)!=0) ||
-		(read(fd,new_buf,newsize)!=newsize) ||
-		(close(fd)==-1)) err(1,"%s",new_filename);
-
-	int ret = bsdiff(old_buf, oldsize, new_buf, newsize, patch_filename, nullptr);
-
-	free(old_buf);
-	free(new_buf);
-
-	return ret;
-}
-
 // TODO(deymo): Deprecate this version of the interface and move all callers
 // to the underlying version using PatchWriterInterface instead. This allows
 // more flexible options including different encodings.
@@ -118,9 +82,9 @@ int bsdiff(const uint8_t* old_buf, size_t oldsize, const uint8_t* new_buf,
 	}
 
 	/* Initialize the patch file encoder */
-	if (!patch->Init())
-		return 1;
 	DiffEncoder diff_encoder(patch, old_buf, oldsize, new_buf, newsize);
+	if (!diff_encoder.Init())
+		return 1;
 
 	/* Compute the differences, writing ctrl as we go */
 	scan=0;len=0;

bsdiff.gyp

@@ -59,6 +59,7 @@
         'bz2_compressor.cc',
         'diff_encoder.cc',
         'patch_writer.cc',
+        'patch_writer_factory.cc',
         'split_patch_writer.cc',
         'suffix_array_index.cc',
       ],

bsdiff_main.cc

@@ -3,12 +3,85 @@
 // found in the LICENSE file.
 
 #include <err.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <limits>
 
 #include "bsdiff/bsdiff.h"
+#include "bsdiff/patch_writer_factory.h"
+
+namespace {
+
+// mmap() the passed |filename| to read-only memory and store in |filesize| the
+// size of the file. To release the memory, call munmap with the returned
+// pointer and filesize. In case of error returns nullptr.
+void* MapFile(const char* filename, size_t* filesize) {
+  int fd = open(filename, O_RDONLY);
+  if (fd < 0) {
+    perror("open()");
+    return nullptr;
+  }
+
+  struct stat st;
+  fstat(fd, &st);
+  if (static_cast<uint64_t>(st.st_size) > std::numeric_limits<size_t>::max()) {
+    fprintf(stderr, "File too big\n");
+    close(fd);
+    return nullptr;
+  }
+  *filesize = st.st_size;
+
+  void* ret = mmap(nullptr, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+  if (ret == MAP_FAILED) {
+    perror("mmap()");
+    close(fd);
+    return nullptr;
+  }
+  close(fd);
+  return ret;
+}
+
+// Generate bsdiff patch from the |old_filename| file to the |new_filename|
+// file storing the resulting patch in a new |patch_filename| file.
+int GenerateBsdiffFromFiles(const char* old_filename,
+                            const char* new_filename,
+                            const char* patch_filename) {
+  size_t oldsize, newsize;
+  int ret = 0;
+
+  uint8_t* old_buf = static_cast<uint8_t*>(MapFile(old_filename, &oldsize));
+  uint8_t* new_buf = static_cast<uint8_t*>(MapFile(new_filename, &newsize));
+
+  if (old_buf && new_buf) {
+    auto patch_writer = bsdiff::CreateBsdiffPatchWriter(patch_filename);
+
+    ret = bsdiff::bsdiff(old_buf, oldsize, new_buf, newsize, patch_writer.get(),
+                         nullptr);
+  } else {
+    ret = 1;
+  }
+
+  if (old_buf)
+    munmap(old_buf, oldsize);
+  if (new_buf)
+    munmap(new_buf, newsize);
+
+  return ret;
+}
+
+}  // namespace
 
 int main(int argc, char* argv[]) {
   if (argc != 4)
     errx(1, "usage: %s oldfile newfile patchfile\n", argv[0]);
 
-  return bsdiff::bsdiff(argv[1], argv[2], argv[3]);
+  return GenerateBsdiffFromFiles(argv[1], argv[2], argv[3]);
 }

bspatch.cc

@@ -33,6 +33,7 @@ __FBSDID("$FreeBSD: src/usr.bin/bsdiff/bspatch/bspatch.c,v 1.1 2005/08/06 01:59:
 #include <errno.h>
 #include <fcntl.h>
 #include <inttypes.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>

diff_encoder.cc

@@ -21,6 +21,10 @@ const uint64_t kMaxEncodedUint64Value = (1ULL << 63) - 1;
 
 namespace bsdiff {
 
+bool DiffEncoder::Init() {
+  return patch_->Init(new_size_);
+}
+
 bool DiffEncoder::AddControlEntry(const ControlEntry& entry) {
   if (entry.diff_size > kMaxEncodedUint64Value) {
     LOG(ERROR) << "Encoding value out of range " << entry.diff_size << endl;

diff_encoder.h

@@ -20,7 +20,7 @@ namespace bsdiff {
 class DiffEncoder {
  public:
   // Initialize the DiffEncoder with the old and new file buffers, as well as
-  // the path writer used. The |patch| must be already initialized.
+  // the path writer used. The |patch| will be initialized when calling Init().
   DiffEncoder(PatchWriterInterface* patch,
               const uint8_t* old_buf,
               uint64_t old_size,
@@ -32,6 +32,9 @@ class DiffEncoder {
         new_buf_(new_buf),
         new_size_(new_size) {}
 
+  // Initialize the diff encoder and the underlying patch.
+  bool Init();
+
   // Add a new control triplet entry to the patch. The |entry.diff_size| bytes
   // for the diff stream and the |entry.extra_size| bytes for the extra stream
   // will be computed and added to the corresponding streams in the patch.

diff_encoder_unittest.cc

@@ -27,7 +27,6 @@ namespace bsdiff {
 class DiffEncoderTest : public testing::Test {
  protected:
   void SetUp() {
-    EXPECT_TRUE(fake_patch_.Init());
     // By default, set the encoder to kHelloWorld to kHelloWorld.
     diff_encoder_.reset(new DiffEncoder(&fake_patch_, kHelloWorld,
                                         sizeof(kHelloWorld), kHelloWorld,
@@ -40,6 +39,7 @@ class DiffEncoderTest : public testing::Test {
 
 TEST_F(DiffEncoderTest, CreateEmptyPatchTest) {
   diff_encoder_.reset(new DiffEncoder(&fake_patch_, nullptr, 0, nullptr, 0));
+  EXPECT_TRUE(diff_encoder_->Init());
   EXPECT_TRUE(diff_encoder_->Close());
 
   // Both diff and extra stream must be empty stream, and not control entries.
@@ -51,6 +51,7 @@ TEST_F(DiffEncoderTest, CreateEmptyPatchTest) {
 TEST_F(DiffEncoderTest, AllInExtraStreamTest) {
   diff_encoder_.reset(new DiffEncoder(&fake_patch_, nullptr, 0, kHelloWorld,
                                       sizeof(kHelloWorld)));
+  EXPECT_TRUE(diff_encoder_->Init());
 
   // Write to the extra stream in two parts: first 5 bytes, then the rest.
   EXPECT_TRUE(diff_encoder_->AddControlEntry(ControlEntry(0, 5, 0)));
@@ -66,6 +67,7 @@ TEST_F(DiffEncoderTest, AllInExtraStreamTest) {
 }
 
 TEST_F(DiffEncoderTest, AllInDiffStreamTest) {
+  EXPECT_TRUE(diff_encoder_->Init());
   EXPECT_TRUE(
       diff_encoder_->AddControlEntry(ControlEntry(sizeof(kHelloWorld), 0, 0)));
   EXPECT_TRUE(diff_encoder_->Close());
@@ -76,6 +78,7 @@ TEST_F(DiffEncoderTest, AllInDiffStreamTest) {
 }
 
 TEST_F(DiffEncoderTest, OldPosNegativeErrorTest) {
+  EXPECT_TRUE(diff_encoder_->Init());
   // Referencing negative values in oldpos is fine, until you use them.
   EXPECT_TRUE(diff_encoder_->AddControlEntry(ControlEntry(0, 0, -5)));
   EXPECT_TRUE(diff_encoder_->AddControlEntry(ControlEntry(0, 0, 2)));
@@ -84,6 +87,7 @@ TEST_F(DiffEncoderTest, OldPosNegativeErrorTest) {
 
 // Test that using an oldpos past the end of the file fails.
 TEST_F(DiffEncoderTest, OldPosTooBigErrorTest) {
+  EXPECT_TRUE(diff_encoder_->Init());
   EXPECT_TRUE(
       diff_encoder_->AddControlEntry(ControlEntry(0, 0, sizeof(kHelloWorld))));
   EXPECT_FALSE(diff_encoder_->AddControlEntry(ControlEntry(1, 0, 0)));
@@ -92,6 +96,7 @@ TEST_F(DiffEncoderTest, OldPosTooBigErrorTest) {
 // Test that diffing against a section of the old file past the end of the file
 // fails.
 TEST_F(DiffEncoderTest, OldPosPlusSizeTooBigErrorTest) {
+  EXPECT_TRUE(diff_encoder_->Init());
   // The oldpos is set to a range inside the word, the we try to copy past the
   // end of it.
   EXPECT_TRUE(diff_encoder_->AddControlEntry(
@@ -101,6 +106,7 @@ TEST_F(DiffEncoderTest, OldPosPlusSizeTooBigErrorTest) {
 }
 
 TEST_F(DiffEncoderTest, ExtraStreamTooBigErrorTest) {
+  EXPECT_TRUE(diff_encoder_->Init());
   EXPECT_TRUE(diff_encoder_->AddControlEntry(ControlEntry(3, 0, 0)));
   // This writes too many bytes in the stream because we already have 3 bytes.
   EXPECT_FALSE(

fake_patch_writer.h

@@ -20,9 +20,10 @@ class FakePatchWriter : public PatchWriterInterface {
   ~FakePatchWriter() override = default;
 
   // PatchWriterInterface overrides.
-  bool Init() override {
+  bool Init(size_t new_size) override {
     EXPECT_FALSE(initialized_);
     initialized_ = true;
+    new_size_ = new_size;
     return true;
   }
 
@@ -53,6 +54,7 @@ class FakePatchWriter : public PatchWriterInterface {
   const std::vector<ControlEntry>& entries() const { return entries_; }
   const std::vector<uint8_t>& diff_stream() const { return diff_stream_; }
   const std::vector<uint8_t>& extra_stream() const { return extra_stream_; }
+  size_t new_size() const { return new_size_; }
 
  private:
   // The list of ControlEntry passed to this class.
@@ -60,6 +62,9 @@ class FakePatchWriter : public PatchWriterInterface {
   std::vector<uint8_t> diff_stream_;
   std::vector<uint8_t> extra_stream_;
 
+  // The size of the new file for the patch we are writing.
+  size_t new_size_{0};
+
   // Whether this class was initialized.
   bool initialized_{false};
 

include/bsdiff/bsdiff.h

@@ -5,6 +5,7 @@
 #ifndef _BSDIFF_BSDIFF_H_
 #define _BSDIFF_BSDIFF_H_
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include "bsdiff/common.h"
@@ -13,17 +14,10 @@
 
 namespace bsdiff {
 
-// Generate bsdiff patch from the |old_filename| file to the |new_filename|
-// file storing the resulting patch in a new |patch_filename| file.
-BSDIFF_EXPORT
-int bsdiff(const char* old_filename,
-           const char* new_filename,
-           const char* patch_filename);
-
 // Generate bsdiff patch from |old_buf| to |new_buf|, save the patch file to
 // |patch_filename|. Returns 0 on success.
-// |I_cache| can be used to cache the suffix array if the same |old_buf| is used
-// repeatedly, pass nullptr if not needed.
+// |sai_cache| can be used to cache the suffix array if the same |old_buf| is
+//  used repeatedly, pass nullptr if not needed.
 BSDIFF_EXPORT
 int bsdiff(const uint8_t* old_buf,
            size_t oldsize,

include/bsdiff/bspatch.h

@@ -5,6 +5,9 @@
 #ifndef _BSDIFF_BSPATCH_H_
 #define _BSDIFF_BSPATCH_H_
 
+#include <stddef.h>
+#include <stdint.h>
+
 #include <functional>
 #include <memory>
 #include <vector>

include/bsdiff/extents_file.h

@@ -5,7 +5,8 @@
 #ifndef _BSDIFF_EXTENTS_FILE_H_
 #define _BSDIFF_EXTENTS_FILE_H_
 
-#include <stdio.h>
+#include <stddef.h>
+#include <stdint.h>
 
 #include <memory>
 #include <vector>

include/bsdiff/file_interface.h

@@ -5,9 +5,11 @@
 #ifndef _BSDIFF_FILE_INTERFACE_H_
 #define _BSDIFF_FILE_INTERFACE_H_
 
-#include <stdint.h>
 #include <sys/types.h>
 
+#include <stddef.h>
+#include <stdint.h>
+
 #include "bsdiff/common.h"
 
 namespace bsdiff {

include/bsdiff/patch_writer_factory.h

@@ -0,0 +1,22 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _BSDIFF_PATCH_WRITER_FACTORY_H_
+#define _BSDIFF_PATCH_WRITER_FACTORY_H_
+
+#include <memory>
+#include <string>
+
+#include "bsdiff/patch_writer_interface.h"
+
+namespace bsdiff {
+
+// Create a patch writer compatible with upstream's "BSDIFF40" patch format
+// using bz2 as a compressor.
+std::unique_ptr<PatchWriterInterface> CreateBsdiffPatchWriter(
+    const std::string& patch_filename);
+
+}  // namespace bsdiff
+
+#endif  // _BSDIFF_PATCH_WRITER_FACTORY_H_

include/bsdiff/patch_writer_interface.h

@@ -5,10 +5,9 @@
 #ifndef _BSDIFF_PATCH_WRITER_INTERFACE_H_
 #define _BSDIFF_PATCH_WRITER_INTERFACE_H_
 
+#include <stddef.h>
 #include <stdint.h>
 
-#include <cstddef>
-
 #include "bsdiff/control_entry.h"
 
 namespace bsdiff {
@@ -17,8 +16,9 @@ class PatchWriterInterface {
  public:
   virtual ~PatchWriterInterface() = default;
 
-  // Initialize the patch writer.
-  virtual bool Init() = 0;
+  // Initialize the patch writer for a patch where the new file will have
+  // |new_size| bytes.
+  virtual bool Init(size_t new_size) = 0;
 
   // Write the passed |data| buffer of length |size| to the Diff or Extra
   // streams respectively. Each method can be called independently from each

include/bsdiff/suffix_array_index_interface.h

@@ -10,6 +10,7 @@
 // this index can vary on technical details, such as the size of the internal
 // suffix array elements, which are not visible in this interface.
 
+#include <stddef.h>
 #include <stdint.h>
 
 namespace bsdiff {

patch_writer.cc

@@ -27,7 +27,7 @@ void EncodeInt64(int64_t x, uint8_t* buf) {
 
 namespace bsdiff {
 
-bool BsdiffPatchWriter::Init() {
+bool BsdiffPatchWriter::Init(size_t /* new_size */) {
   fp_ = fopen(patch_filename_.c_str(), "w");
   if (!fp_) {
     LOG(ERROR) << "Opening " << patch_filename_ << endl;

patch_writer.h

@@ -23,7 +23,7 @@ class BsdiffPatchWriter : public PatchWriterInterface {
       : patch_filename_(patch_filename) {}
 
   // PatchWriterInterface overrides.
-  bool Init() override;
+  bool Init(size_t new_size) override;
   bool WriteDiffStream(const uint8_t* data, size_t size) override;
   bool WriteExtraStream(const uint8_t* data, size_t size) override;
   bool AddControlEntry(const ControlEntry& entry) override;

patch_writer_factory.cc

@@ -0,0 +1,17 @@
+// Copyright 2017 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "bsdiff/patch_writer_factory.h"
+
+#include "bsdiff/patch_writer.h"
+
+namespace bsdiff {
+
+std::unique_ptr<PatchWriterInterface> CreateBsdiffPatchWriter(
+    const std::string& patch_filename) {
+  return std::unique_ptr<PatchWriterInterface>(
+      new BsdiffPatchWriter(patch_filename));
+}
+
+}  // namespace bsdiff

patch_writer_unittest.cc

@@ -46,7 +46,11 @@ namespace bsdiff {
 
 class BsdiffPatchWriterTest : public testing::Test {
  protected:
-  void SetUp() override { EXPECT_TRUE(patch_writer_.Init()); }
+  void SetUp() override {
+    // This patch writer doesn't use the |new_size| value passed on init, so we
+    // don't pass any meaningful value.
+    EXPECT_TRUE(patch_writer_.Init(0));
+  }
 
   test_utils::ScopedTempFile patch_file_{"bsdiff_newfile.XXXXXX"};
   BsdiffPatchWriter patch_writer_{patch_file_.filename()};

split_patch_writer.cc

@@ -12,12 +12,24 @@ using std::endl;
 
 namespace bsdiff {
 
-bool SplitPatchWriter::Init() {
+bool SplitPatchWriter::Init(size_t new_size) {
+  new_size_ = new_size;
   // Fail gracefully if re-initialized.
   if (current_patch_ || patches_.empty())
     return false;
 
-  return patches_[0]->Init();
+  size_t expected_patches = (new_size_ + new_chunk_size_ - 1) / new_chunk_size_;
+  if (expected_patches == 0)
+    expected_patches = 1;
+  if (expected_patches != patches_.size()) {
+    LOG(ERROR) << "Expected " << expected_patches << " for a new file of size "
+               << new_size_ << " split in chunks of " << new_chunk_size_
+               << " but got " << patches_.size() << " instead." << endl;
+    return false;
+  }
+
+  return patches_[0]->Init(
+      std::min(static_cast<uint64_t>(new_size_), new_chunk_size_));
 }
 
 bool SplitPatchWriter::WriteDiffStream(const uint8_t* data, size_t size) {
@@ -61,7 +73,8 @@ bool SplitPatchWriter::AddControlEntry(const ControlEntry& entry) {
         LOG(ERROR) << "Writing past the last patch" << endl;
         return false;
       }
-      if (!patches_[current_patch_]->Init()) {
+      if (!patches_[current_patch_]->Init(std::min(
+              new_size_ - current_patch_ * new_chunk_size_, new_chunk_size_))) {
         LOG(ERROR) << "Failed to initialize patch " << current_patch_ << endl;
         return false;
       }

split_patch_writer.h

@@ -18,9 +18,9 @@ namespace bsdiff {
 // the contents of the new file data, and won't necessarily be uniform.
 class SplitPatchWriter : public PatchWriterInterface {
  public:
-  // Create a PatchWriter using that will split in several patches where each
-  // one will write |new_chunk_size| bytes of new file data. Each patch will
-  // use the old file as a whole input file.
+  // Create a PatchWriter that will split the patch in several patches where
+  // each one will write |new_chunk_size| bytes of new file data. Each patch
+  // will use the old file as a whole input file.
   SplitPatchWriter(uint64_t new_chunk_size,
                    const std::vector<PatchWriterInterface*>& patches)
       : new_chunk_size_(new_chunk_size), patches_(patches) {
@@ -33,7 +33,7 @@ class SplitPatchWriter : public PatchWriterInterface {
   // corresponding AddControlEntry() is not supported and will fail. The reason
   // for this is because which underlying patch takes the bytes depends on the
   // control entries.
-  bool Init() override;
+  bool Init(size_t new_size) override;
   bool WriteDiffStream(const uint8_t* data, size_t size) override;
   bool WriteExtraStream(const uint8_t* data, size_t size) override;
   bool AddControlEntry(const ControlEntry& entry) override;
@@ -53,6 +53,9 @@ class SplitPatchWriter : public PatchWriterInterface {
                      const uint8_t* data,
                      size_t size);
 
+  // The size of the new file for the patch we are writing.
+  size_t new_size_{0};
+
   // The size of each chunk of the new file written to.
   uint64_t new_chunk_size_;
   std::vector<PatchWriterInterface*> patches_;

split_patch_writer_unittest.cc

@@ -16,23 +16,33 @@ namespace bsdiff {
 
 class SplitPatchWriterTest : public testing::Test {
  protected:
-  void SetUpForSize(size_t num_chunks, uint64_t new_chunk_size) {
+  void SetUpForSize(size_t num_chunks,
+                    uint64_t new_chunk_size,
+                    size_t new_size) {
     fake_patches_.resize(num_chunks);
     std::vector<PatchWriterInterface*> patches;
     for (auto& fake_patch : fake_patches_)
       patches.push_back(&fake_patch);
 
     patch_writer_.reset(new SplitPatchWriter(new_chunk_size, patches));
-    EXPECT_TRUE(patch_writer_->Init());
+    EXPECT_TRUE(patch_writer_->Init(new_size));
   }
 
   std::vector<FakePatchWriter> fake_patches_;
   std::unique_ptr<SplitPatchWriter> patch_writer_;
 };
 
+TEST_F(SplitPatchWriterTest, InvalidNumberOfPatchesForSizeTest) {
+  FakePatchWriter p;
+  std::vector<PatchWriterInterface*> patches = {&p};
+  patch_writer_.reset(new SplitPatchWriter(10, patches));
+  // We should have pass two patches.
+  EXPECT_FALSE(patch_writer_->Init(15));
+}
+
 // A single empty patch is allowed.
 TEST_F(SplitPatchWriterTest, NonSplitEmptyPatchTest) {
-  SetUpForSize(1, 100);
+  SetUpForSize(1, 100, 0);
   EXPECT_TRUE(patch_writer_->Close());
 
   EXPECT_TRUE(fake_patches_[0].entries().empty());
@@ -40,13 +50,13 @@ TEST_F(SplitPatchWriterTest, NonSplitEmptyPatchTest) {
 
 // Leaving patches at the end that are empty is considered an error.
 TEST_F(SplitPatchWriterTest, NotAllPatchesWrittenErrorTest) {
-  SetUpForSize(2, 100);
+  SetUpForSize(2, 100, 200);
   // We write less than the amount needed for two patches, which should fail.
   EXPECT_FALSE(patch_writer_->Close());
 }
 
 TEST_F(SplitPatchWriterTest, MissingDiffBytesErrorTest) {
-  SetUpForSize(2, 10);
+  SetUpForSize(2, 10, 20);
 
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(15, 5, 0)));
   std::vector<uint8_t> zeros(20, 0);
@@ -58,7 +68,7 @@ TEST_F(SplitPatchWriterTest, MissingDiffBytesErrorTest) {
 }
 
 TEST_F(SplitPatchWriterTest, MissingExtraBytesErrorTest) {
-  SetUpForSize(2, 10);
+  SetUpForSize(2, 10, 20);
 
   std::vector<uint8_t> zeros(20, 0);
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(5, 15, -5)));
@@ -72,7 +82,7 @@ TEST_F(SplitPatchWriterTest, MissingExtraBytesErrorTest) {
 // Test all sort of corner cases when splitting the ControlEntry across multiple
 // patches
 TEST_F(SplitPatchWriterTest, SplitControlAcrossSeveralPatchesTest) {
-  SetUpForSize(4, 10);
+  SetUpForSize(4, 10, 40);
   // The middle control entry would be split in tree different patches.
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(5, 1, -5)));
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(4, 0, -4)));
@@ -116,6 +126,10 @@ TEST_F(SplitPatchWriterTest, SplitControlAcrossSeveralPatchesTest) {
                 ControlEntry(4, 0, 0),  // (4, 0, -5) ignoring the offset.
             }),
             fake_patches_[3].entries());
+
+  for (size_t i = 0; i < fake_patches_.size(); ++i) {
+    EXPECT_EQ(10U, fake_patches_[i].new_size()) << "where i = " << i;
+  }
 }
 
 TEST_F(SplitPatchWriterTest, WriteStreamsAfterControlAcrossPatchesTest) {
@@ -123,7 +137,7 @@ TEST_F(SplitPatchWriterTest, WriteStreamsAfterControlAcrossPatchesTest) {
   for (size_t i = 0; i < numbers.size(); ++i)
     numbers[i] = 'A' + i;
 
-  SetUpForSize(4, 10);
+  SetUpForSize(4, 10, 40);
   // The sequence is 15 diff, 10 extra, 15 diff.
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(15, 10, 0)));
   EXPECT_TRUE(patch_writer_->AddControlEntry(ControlEntry(15, 0, 0)));

suffix_array_index_unittest.cc

@@ -11,7 +11,6 @@
 #include <utility>
 #include <vector>
 
-#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 namespace bsdiff {