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>
diff --git a/payload_generator/ab_generator.cc b/payload_generator/ab_generator.cc
index f727c71..089dfd9 100644
--- a/payload_generator/ab_generator.cc
+++ b/payload_generator/ab_generator.cc
@@ -272,7 +272,7 @@
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 @@
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 7a5d9a4..88e42e0 100644
--- a/payload_generator/deflate_utils.cc
+++ b/payload_generator/deflate_utils.cc
@@ -49,7 +49,7 @@
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 @@
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 @@
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 @@
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 @@
// 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 13027ab..fbb6066 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -234,7 +234,7 @@
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 @@
version_,
blob_file_)) {
LOG(ERROR) << "Failed to generate delta for " << name_ << " ("
- << BlocksInExtents(new_extents_) << " blocks)";
+ << utils::BlocksInExtents(new_extents_) << " blocks)";
}
}
@@ -367,9 +367,9 @@
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 @@
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 @@
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 @@
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 46e68c5..a83cea2 100644
--- a/payload_generator/delta_diff_utils_unittest.cc
+++ b/payload_generator/delta_diff_utils_unittest.cc
@@ -194,9 +194,9 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 b884021..07ec371 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 @@
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 a3c7731..5360e6c 100644
--- a/payload_generator/ext2_filesystem_unittest.cc
+++ b/payload_generator/ext2_filesystem_unittest.cc
@@ -158,7 +158,8 @@
// 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 @@
// 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 0e0cdf7..c1d3d63 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 @@
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 89ccca2..47073f9 100644
--- a/payload_generator/extent_utils.cc
+++ b/payload_generator/extent_utils.cc
@@ -53,16 +53,6 @@
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 3e45264..f5fbb0e 100644
--- a/payload_generator/extent_utils.h
+++ b/payload_generator/extent_utils.h
@@ -32,31 +32,12 @@
// 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 d470e7b..eef4385 100644
--- a/payload_generator/extent_utils_unittest.cc
+++ b/payload_generator/extent_utils_unittest.cc
@@ -54,23 +54,23 @@
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 9e62de2..6da4d10 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 @@
// 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 @@
// 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 2d5fb63..4829b21 100644
--- a/payload_generator/graph_utils.cc
+++ b/payload_generator/graph_utils.cc
@@ -104,9 +104,9 @@
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 cf02c91..b858c2b 100644
--- a/payload_generator/inplace_generator.cc
+++ b/payload_generator/inplace_generator.cc
@@ -299,8 +299,7 @@
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) {
