update_engine: Remove the duplicate BlocksInExtents

This patch removes the duplicate BlocksInExtents from extent_utils.h and fixes
the remainder of the code to reflect this change.

BUG=none
TEST=unittests pass;

Change-Id: I76f5106f75072b20cd8f41f081b2f2b07aeac9a8
Reviewed-on: https://chromium-review.googlesource.com/812009
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Alex Deymo <deymo@google.com>
Reviewed-by: Sen Jiang <senj@chromium.org>

13 files changed, 65 insertions, 84 deletions

diff --git a/payload_generator/ab_generator.cc b/payload_generator/ab_generator.cc
index f727c710..089dfd9c 100644
--- a/payload_generator/ab_generator.cc
+++ b/payload_generator/ab_generator.cc
@@ -272,7 +272,7 @@ bool ABGenerator::AddDataAndSetType(AnnotatedOperation* aop,
 
   vector<Extent> dst_extents;
   ExtentsToVector(aop->op.dst_extents(), &dst_extents);
-  brillo::Blob data(BlocksInExtents(dst_extents) * kBlockSize);
+  brillo::Blob data(utils::BlocksInExtents(dst_extents) * kBlockSize);
   TEST_AND_RETURN_FALSE(utils::ReadExtents(target_part_path,
                                            dst_extents,
                                            &data,
@@ -306,7 +306,7 @@ bool ABGenerator::AddSourceHash(vector<AnnotatedOperation>* aops,
     uint64_t src_length =
         aop.op.has_src_length()
             ? aop.op.src_length()
-            : BlocksInExtents(aop.op.src_extents()) * kBlockSize;
+            : utils::BlocksInExtents(aop.op.src_extents()) * kBlockSize;
     TEST_AND_RETURN_FALSE(utils::ReadExtents(
         source_part_path, src_extents, &src_data, src_length, kBlockSize));
     TEST_AND_RETURN_FALSE(HashCalculator::RawHashOfData(src_data, &src_hash));
diff --git a/payload_generator/deflate_utils.cc b/payload_generator/deflate_utils.cc
index 7a5d9a42..88e42e0d 100644
--- a/payload_generator/deflate_utils.cc
+++ b/payload_generator/deflate_utils.cc
@@ -49,7 +49,7 @@ bool CopyExtentsToFile(const string& in_path,
                        const vector<Extent> extents,
                        const string& out_path,
                        size_t block_size) {
-  brillo::Blob data(BlocksInExtents(extents) * block_size);
+  brillo::Blob data(utils::BlocksInExtents(extents) * block_size);
   TEST_AND_RETURN_FALSE(
       utils::ReadExtents(in_path, extents, &data, data.size(), block_size));
   TEST_AND_RETURN_FALSE(
@@ -61,7 +61,8 @@ bool IsSquashfsImage(const string& part_path,
                      const FilesystemInterface::File& file) {
   // Only check for files with img postfix.
   if (base::EndsWith(file.name, ".img", base::CompareCase::SENSITIVE) &&
-      BlocksInExtents(file.extents) >= kMinimumSquashfsImageSize / kBlockSize) {
+      utils::BlocksInExtents(file.extents) >=
+          kMinimumSquashfsImageSize / kBlockSize) {
     brillo::Blob super_block;
     TEST_AND_RETURN_FALSE(
         utils::ReadFileChunk(part_path,
@@ -87,11 +88,11 @@ bool RealignSplittedFiles(const FilesystemInterface::File& file,
         ShiftBitExtentsOverExtents(file.extents, &in_file.deflates));
 
     in_file.name = file.name + "/" + in_file.name;
-    num_blocks += BlocksInExtents(in_file.extents);
+    num_blocks += utils::BlocksInExtents(in_file.extents);
   }
 
   // Check that all files in |in_files| cover the entire image.
-  TEST_AND_RETURN_FALSE(BlocksInExtents(file.extents) == num_blocks);
+  TEST_AND_RETURN_FALSE(utils::BlocksInExtents(file.extents) == num_blocks);
   return true;
 }
 
@@ -111,7 +112,8 @@ ByteExtent ExpandToByteExtent(const BitExtent& extent) {
 
 bool ShiftExtentsOverExtents(const vector<Extent>& base_extents,
                              vector<Extent>* over_extents) {
-  if (BlocksInExtents(base_extents) < BlocksInExtents(*over_extents)) {
+  if (utils::BlocksInExtents(base_extents) <
+      utils::BlocksInExtents(*over_extents)) {
     LOG(ERROR) << "over_extents have more blocks than base_extents! Invalid!";
     return false;
   }
@@ -160,7 +162,7 @@ bool ShiftBitExtentsOverExtents(const vector<Extent>& base_extents,
   // does not exceed |base_extents|.
   auto last_extent = ExpandToByteExtent(over_extents->back());
   TEST_AND_RETURN_FALSE(last_extent.offset + last_extent.length <=
-                        BlocksInExtents(base_extents) * kBlockSize);
+                        utils::BlocksInExtents(base_extents) * kBlockSize);
 
   for (auto o_ext = over_extents->begin(); o_ext != over_extents->end();) {
     size_t gap_blocks = base_extents[0].start_block();
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 13027ab2..fbb6066d 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -234,7 +234,7 @@ void FileDeltaProcessor::Run() {
   TEST_AND_RETURN(blob_file_ != nullptr);
 
   LOG(INFO) << "Encoding file " << name_ << " ("
-            << BlocksInExtents(new_extents_) << " blocks)";
+            << utils::BlocksInExtents(new_extents_) << " blocks)";
 
   if (!DeltaReadFile(&file_aops_,
                      old_part_,
@@ -248,7 +248,7 @@ void FileDeltaProcessor::Run() {
                      version_,
                      blob_file_)) {
     LOG(ERROR) << "Failed to generate delta for " << name_ << " ("
-               << BlocksInExtents(new_extents_) << " blocks)";
+               << utils::BlocksInExtents(new_extents_) << " blocks)";
   }
 }
 
@@ -367,9 +367,9 @@ bool DeltaReadPartition(vector<AnnotatedOperation>* aops,
     old_unvisited = FilterExtentRanges(old_unvisited, old_visited_blocks);
   }
 
-  LOG(INFO) << "Scanning " << BlocksInExtents(new_unvisited)
-            << " unwritten blocks using chunk size of "
-            << soft_chunk_blocks << " blocks.";
+  LOG(INFO) << "Scanning " << utils::BlocksInExtents(new_unvisited)
+            << " unwritten blocks using chunk size of " << soft_chunk_blocks
+            << " blocks.";
   // We use the soft_chunk_blocks limit for the <non-file-data> as we don't
   // really know the structure of this data and we should not expect it to have
   // redundancy between partitions.
@@ -492,7 +492,7 @@ bool DeltaMovedAndZeroBlocks(vector<AnnotatedOperation>* aops,
                                         blob_file));
   }
   LOG(INFO) << "Produced " << (aops->size() - num_ops) << " operations for "
-            << BlocksInExtents(new_zeros) << " zeroed blocks";
+            << utils::BlocksInExtents(new_zeros) << " zeroed blocks";
 
   // Produce MOVE/SOURCE_COPY operations for the moved blocks.
   num_ops = aops->size();
@@ -555,7 +555,7 @@ bool DeltaReadFile(vector<AnnotatedOperation>* aops,
   brillo::Blob data;
   InstallOperation operation;
 
-  uint64_t total_blocks = BlocksInExtents(new_extents);
+  uint64_t total_blocks = utils::BlocksInExtents(new_extents);
   if (chunk_blocks == -1)
     chunk_blocks = total_blocks;
 
@@ -676,8 +676,8 @@ bool ReadExtentsToDiff(const string& old_part,
   InstallOperation operation;
 
   // We read blocks from old_extents and write blocks to new_extents.
-  uint64_t blocks_to_read = BlocksInExtents(old_extents);
-  uint64_t blocks_to_write = BlocksInExtents(new_extents);
+  uint64_t blocks_to_read = utils::BlocksInExtents(old_extents);
+  uint64_t blocks_to_write = utils::BlocksInExtents(new_extents);
 
   // Disable bsdiff, and puffdiff when the data is too big.
   bool bsdiff_allowed =
diff --git a/payload_generator/delta_diff_utils_unittest.cc b/payload_generator/delta_diff_utils_unittest.cc
index 46e68c52..a83cea29 100644
--- a/payload_generator/delta_diff_utils_unittest.cc
+++ b/payload_generator/delta_diff_utils_unittest.cc
@@ -194,9 +194,9 @@ TEST_F(DeltaDiffUtilsTest, MoveSmallTest) {
   EXPECT_EQ(kBlockSize, op.src_length());
   EXPECT_EQ(1, op.dst_extents_size());
   EXPECT_EQ(kBlockSize, op.dst_length());
-  EXPECT_EQ(BlocksInExtents(op.src_extents()),
-            BlocksInExtents(op.dst_extents()));
-  EXPECT_EQ(1U, BlocksInExtents(op.dst_extents()));
+  EXPECT_EQ(utils::BlocksInExtents(op.src_extents()),
+            utils::BlocksInExtents(op.dst_extents()));
+  EXPECT_EQ(1U, utils::BlocksInExtents(op.dst_extents()));
 }
 
 TEST_F(DeltaDiffUtilsTest, MoveWithSameBlock) {
@@ -220,8 +220,8 @@ TEST_F(DeltaDiffUtilsTest, MoveWithSameBlock) {
       ExtentForRange(24, 3),
       ExtentForRange(29, 1) };
 
-  uint64_t num_blocks = BlocksInExtents(old_extents);
-  EXPECT_EQ(num_blocks, BlocksInExtents(new_extents));
+  uint64_t num_blocks = utils::BlocksInExtents(old_extents);
+  EXPECT_EQ(num_blocks, utils::BlocksInExtents(new_extents));
 
   // The size of the data should match the total number of blocks. Each block
   // has a different content.
@@ -262,7 +262,7 @@ TEST_F(DeltaDiffUtilsTest, MoveWithSameBlock) {
       ExtentForRange(18, 1),
       ExtentForRange(20, 1),
       ExtentForRange(26, 1) };
-  num_blocks = BlocksInExtents(old_extents);
+  num_blocks = utils::BlocksInExtents(old_extents);
 
   EXPECT_EQ(num_blocks * kBlockSize, op.src_length());
   EXPECT_EQ(num_blocks * kBlockSize, op.dst_length());
@@ -321,9 +321,9 @@ TEST_F(DeltaDiffUtilsTest, BsdiffSmallTest) {
   EXPECT_EQ(kBlockSize, op.src_length());
   EXPECT_EQ(1, op.dst_extents_size());
   EXPECT_EQ(kBlockSize, op.dst_length());
-  EXPECT_EQ(BlocksInExtents(op.src_extents()),
-            BlocksInExtents(op.dst_extents()));
-  EXPECT_EQ(1U, BlocksInExtents(op.dst_extents()));
+  EXPECT_EQ(utils::BlocksInExtents(op.src_extents()),
+            utils::BlocksInExtents(op.dst_extents()));
+  EXPECT_EQ(1U, utils::BlocksInExtents(op.dst_extents()));
 }
 
 TEST_F(DeltaDiffUtilsTest, ReplaceSmallTest) {
@@ -373,7 +373,7 @@ TEST_F(DeltaDiffUtilsTest, ReplaceSmallTest) {
     EXPECT_FALSE(op.has_src_length());
     EXPECT_EQ(1, op.dst_extents_size());
     EXPECT_FALSE(op.has_dst_length());
-    EXPECT_EQ(1U, BlocksInExtents(op.dst_extents()));
+    EXPECT_EQ(1U, utils::BlocksInExtents(op.dst_extents()));
   }
 }
 
@@ -634,7 +634,8 @@ TEST_F(DeltaDiffUtilsTest, ZeroBlocksUseReplaceBz) {
       // The last range is split since the old image has zeros in part of it.
       ExtentForRange(30, 20),
   };
-  brillo::Blob zeros_data(BlocksInExtents(new_zeros) * block_size_, '\0');
+  brillo::Blob zeros_data(utils::BlocksInExtents(new_zeros) * block_size_,
+                          '\0');
   EXPECT_TRUE(WriteExtents(new_part_.path, new_zeros, block_size_, zeros_data));
 
   vector<Extent> old_zeros = vector<Extent>{ExtentForRange(43, 7)};
diff --git a/payload_generator/ext2_filesystem.cc b/payload_generator/ext2_filesystem.cc
index b8840219..07ec3710 100644
--- a/payload_generator/ext2_filesystem.cc
+++ b/payload_generator/ext2_filesystem.cc
@@ -18,7 +18,7 @@
 
 #include <et/com_err.h>
 #if defined(__clang__)
-// TODO: Remove these pragmas when b/35721782 is fixed.
+// TODO(*): Remove these pragmas when b/35721782 is fixed.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wmacro-redefined"
 #endif
@@ -348,7 +348,7 @@ bool Ext2Filesystem::LoadSettings(brillo::KeyValueStore* store) const {
     return false;
 
   brillo::Blob blob;
-  uint64_t physical_size = BlocksInExtents(extents) * filsys_->blocksize;
+  uint64_t physical_size = utils::BlocksInExtents(extents) * filsys_->blocksize;
   // Sparse holes in the settings file are not supported.
   if (EXT2_I_SIZE(&ino_data) > physical_size)
     return false;
diff --git a/payload_generator/ext2_filesystem_unittest.cc b/payload_generator/ext2_filesystem_unittest.cc
index a3c7731e..5360e6cd 100644
--- a/payload_generator/ext2_filesystem_unittest.cc
+++ b/payload_generator/ext2_filesystem_unittest.cc
@@ -158,7 +158,8 @@ TEST_F(Ext2FilesystemTest, ParseGeneratedImages) {
 
     // Small symlinks don't actually have data blocks.
     EXPECT_TRUE(map_files["/link-short_symlink"].extents.empty());
-    EXPECT_EQ(1U, BlocksInExtents(map_files["/link-long_symlink"].extents));
+    EXPECT_EQ(1U,
+              utils::BlocksInExtents(map_files["/link-long_symlink"].extents));
 
     // Hard-links report the same list of blocks.
     EXPECT_EQ(map_files["/link-hard-regular-16k"].extents,
@@ -168,14 +169,19 @@ TEST_F(Ext2FilesystemTest, ParseGeneratedImages) {
     // The number of blocks in these files doesn't depend on the
     // block size.
     EXPECT_TRUE(map_files["/empty-file"].extents.empty());
-    EXPECT_EQ(1U, BlocksInExtents(map_files["/regular-small"].extents));
-    EXPECT_EQ(1U, BlocksInExtents(map_files["/regular-with_net_cap"].extents));
+    EXPECT_EQ(1U, utils::BlocksInExtents(map_files["/regular-small"].extents));
+    EXPECT_EQ(
+        1U, utils::BlocksInExtents(map_files["/regular-with_net_cap"].extents));
     EXPECT_TRUE(map_files["/sparse_empty-10k"].extents.empty());
     EXPECT_TRUE(map_files["/sparse_empty-2blocks"].extents.empty());
-    EXPECT_EQ(1U, BlocksInExtents(map_files["/sparse-16k-last_block"].extents));
-    EXPECT_EQ(1U,
-              BlocksInExtents(map_files["/sparse-16k-first_block"].extents));
-    EXPECT_EQ(2U, BlocksInExtents(map_files["/sparse-16k-holes"].extents));
+    EXPECT_EQ(
+        1U,
+        utils::BlocksInExtents(map_files["/sparse-16k-last_block"].extents));
+    EXPECT_EQ(
+        1U,
+        utils::BlocksInExtents(map_files["/sparse-16k-first_block"].extents));
+    EXPECT_EQ(2U,
+              utils::BlocksInExtents(map_files["/sparse-16k-holes"].extents));
   }
 }
 
diff --git a/payload_generator/extent_ranges.cc b/payload_generator/extent_ranges.cc
index 0e0cdf7a..c1d3d638 100644
--- a/payload_generator/extent_ranges.cc
+++ b/payload_generator/extent_ranges.cc
@@ -23,6 +23,7 @@
 
 #include <base/logging.h>
 
+#include "update_engine/common/utils.h"
 #include "update_engine/payload_consumer/payload_constants.h"
 #include "update_engine/payload_generator/extent_utils.h"
 
@@ -250,7 +251,7 @@ vector<Extent> ExtentRanges::GetExtentsForBlockCount(
     out.back().set_num_blocks(blocks_needed);
     break;
   }
-  CHECK(out_blocks == BlocksInExtents(out));
+  CHECK(out_blocks == utils::BlocksInExtents(out));
   return out;
 }
 
diff --git a/payload_generator/extent_utils.cc b/payload_generator/extent_utils.cc
index 89ccca27..47073f9a 100644
--- a/payload_generator/extent_utils.cc
+++ b/payload_generator/extent_utils.cc
@@ -53,16 +53,6 @@ void AppendBlockToExtents(vector<Extent>* extents, uint64_t block) {
   extents->push_back(new_extent);
 }
 
-Extent GetElement(const vector<Extent>& collection, size_t index) {
-  return collection[index];
-}
-
-Extent GetElement(
-    const google::protobuf::RepeatedPtrField<Extent>& collection,
-    size_t index) {
-  return collection.Get(index);
-}
-
 void ExtendExtents(
     google::protobuf::RepeatedPtrField<Extent>* extents,
     const google::protobuf::RepeatedPtrField<Extent>& extents_to_add) {
diff --git a/payload_generator/extent_utils.h b/payload_generator/extent_utils.h
index 3e45264b..f5fbb0e0 100644
--- a/payload_generator/extent_utils.h
+++ b/payload_generator/extent_utils.h
@@ -32,31 +32,12 @@ namespace chromeos_update_engine {
 // into an arbitrary place in the extents.
 void AppendBlockToExtents(std::vector<Extent>* extents, uint64_t block);
 
-// Get/SetElement are intentionally overloaded so that templated functions
-// can accept either type of collection of Extents.
-Extent GetElement(const std::vector<Extent>& collection, size_t index);
-Extent GetElement(
-    const google::protobuf::RepeatedPtrField<Extent>& collection,
-    size_t index);
-
-// Return the total number of blocks in a collection (vector or
-// RepeatedPtrField) of Extents.
-template<typename T>
-uint64_t BlocksInExtents(const T& collection) {
-  uint64_t ret = 0;
-  for (size_t i = 0; i < static_cast<size_t>(collection.size()); ++i) {
-    ret += GetElement(collection, i).num_blocks();
-  }
-  return ret;
-}
-
 // Takes a collection (vector or RepeatedPtrField) of Extent and
 // returns a vector of the blocks referenced, in order.
 template<typename T>
 std::vector<uint64_t> ExpandExtents(const T& extents) {
   std::vector<uint64_t> ret;
-  for (size_t i = 0, e = static_cast<size_t>(extents.size()); i != e; ++i) {
-    const Extent extent = GetElement(extents, i);
+  for (const auto& extent : extents) {
     if (extent.start_block() == kSparseHole) {
       ret.resize(ret.size() + extent.num_blocks(), kSparseHole);
     } else {
diff --git a/payload_generator/extent_utils_unittest.cc b/payload_generator/extent_utils_unittest.cc
index d470e7ba..eef4385e 100644
--- a/payload_generator/extent_utils_unittest.cc
+++ b/payload_generator/extent_utils_unittest.cc
@@ -54,23 +54,23 @@ TEST(ExtentUtilsTest, AppendSparseToExtentsTest) {
 TEST(ExtentUtilsTest, BlocksInExtentsTest) {
   {
     vector<Extent> extents;
-    EXPECT_EQ(0U, BlocksInExtents(extents));
+    EXPECT_EQ(0U, utils::BlocksInExtents(extents));
     extents.push_back(ExtentForRange(0, 1));
-    EXPECT_EQ(1U, BlocksInExtents(extents));
+    EXPECT_EQ(1U, utils::BlocksInExtents(extents));
     extents.push_back(ExtentForRange(23, 55));
-    EXPECT_EQ(56U, BlocksInExtents(extents));
+    EXPECT_EQ(56U, utils::BlocksInExtents(extents));
     extents.push_back(ExtentForRange(1, 2));
-    EXPECT_EQ(58U, BlocksInExtents(extents));
+    EXPECT_EQ(58U, utils::BlocksInExtents(extents));
   }
   {
     google::protobuf::RepeatedPtrField<Extent> extents;
-    EXPECT_EQ(0U, BlocksInExtents(extents));
+    EXPECT_EQ(0U, utils::BlocksInExtents(extents));
     *extents.Add() = ExtentForRange(0, 1);
-    EXPECT_EQ(1U, BlocksInExtents(extents));
+    EXPECT_EQ(1U, utils::BlocksInExtents(extents));
     *extents.Add() = ExtentForRange(23, 55);
-    EXPECT_EQ(56U, BlocksInExtents(extents));
+    EXPECT_EQ(56U, utils::BlocksInExtents(extents));
     *extents.Add() = ExtentForRange(1, 2);
-    EXPECT_EQ(58U, BlocksInExtents(extents));
+    EXPECT_EQ(58U, utils::BlocksInExtents(extents));
   }
 }
 
diff --git a/payload_generator/full_update_generator_unittest.cc b/payload_generator/full_update_generator_unittest.cc
index 9e62de28..6da4d109 100644
--- a/payload_generator/full_update_generator_unittest.cc
+++ b/payload_generator/full_update_generator_unittest.cc
@@ -16,6 +16,7 @@
 
 #include "update_engine/payload_generator/full_update_generator.h"
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -116,9 +117,9 @@ TEST_F(FullUpdateGeneratorTest, ChunkSizeTooBig) {
   // new_part has one chunk and a half.
   EXPECT_EQ(2U, aops.size());
   EXPECT_EQ(config_.hard_chunk_size / config_.block_size,
-            BlocksInExtents(aops[0].op.dst_extents()));
+            utils::BlocksInExtents(aops[0].op.dst_extents()));
   EXPECT_EQ((new_part.size() - config_.hard_chunk_size) / config_.block_size,
-            BlocksInExtents(aops[1].op.dst_extents()));
+            utils::BlocksInExtents(aops[1].op.dst_extents()));
 }
 
 // Test that if the image size is much smaller than the chunk size, it handles
@@ -138,7 +139,7 @@ TEST_F(FullUpdateGeneratorTest, ImageSizeTooSmall) {
   // new_part has less than one chunk.
   EXPECT_EQ(1U, aops.size());
   EXPECT_EQ(new_part.size() / config_.block_size,
-            BlocksInExtents(aops[0].op.dst_extents()));
+            utils::BlocksInExtents(aops[0].op.dst_extents()));
 }
 
 }  // namespace chromeos_update_engine
diff --git a/payload_generator/graph_utils.cc b/payload_generator/graph_utils.cc
index 2d5fb631..4829b21e 100644
--- a/payload_generator/graph_utils.cc
+++ b/payload_generator/graph_utils.cc
@@ -104,9 +104,9 @@ namespace {
 template<typename T>
 void DumpExtents(const T& field, int prepend_space_count) {
   string header(prepend_space_count, ' ');
-  for (int i = 0, e = field.size(); i != e; ++i) {
-    LOG(INFO) << header << "(" << GetElement(field, i).start_block() << ", "
-              << GetElement(field, i).num_blocks() << ")";
+  for (const auto& extent : field) {
+    LOG(INFO) << header << "(" << extent.start_block() << ", "
+              << extent.num_blocks() << ")";
   }
 }
 
diff --git a/payload_generator/inplace_generator.cc b/payload_generator/inplace_generator.cc
index cf02c913..b858c2be 100644
--- a/payload_generator/inplace_generator.cc
+++ b/payload_generator/inplace_generator.cc
@@ -299,8 +299,7 @@ namespace {
 
 template<typename T>
 bool TempBlocksExistInExtents(const T& extents) {
-  for (int i = 0, e = extents.size(); i < e; ++i) {
-    Extent extent = GetElement(extents, i);
+  for (const auto& extent : extents) {
     uint64_t start = extent.start_block();
     uint64_t num = extent.num_blocks();
     if (start >= kTempBlockStart || (start + num) >= kTempBlockStart) {