Fix clang-tidy performance warnings in update_engine.
* Use const reference type for parameters, local variables,
and for-loop index variables to avoid unnecessary copy.
* Convert some for-loops to for-rang loops.
Bug: 30407689
Bug: 30413223
Bug: 30413862
Change-Id: I78996b3f799639fc57ced45e110807625be7dcce
Test: build with WITH_TIDY=1
diff --git a/common/fake_boot_control.h b/common/fake_boot_control.h
index 5c6c160..3eccc80 100644
--- a/common/fake_boot_control.h
+++ b/common/fake_boot_control.h
@@ -85,9 +85,9 @@
current_slot_ = slot;
}
- void SetPartitionDevice(const std::string partition_name,
+ void SetPartitionDevice(const std::string& partition_name,
BootControlInterface::Slot slot,
- const std::string device) {
+ const std::string& device) {
DCHECK(slot < num_slots_);
devices_[slot][partition_name] = device;
}
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index f7c0286..5d0fca3 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -109,15 +109,15 @@
is_oobe_complete_ = false;
}
- void SetHardwareClass(std::string hardware_class) {
+ void SetHardwareClass(const std::string& hardware_class) {
hardware_class_ = hardware_class;
}
- void SetFirmwareVersion(std::string firmware_version) {
+ void SetFirmwareVersion(const std::string& firmware_version) {
firmware_version_ = firmware_version;
}
- void SetECVersion(std::string ec_version) {
+ void SetECVersion(const std::string& ec_version) {
ec_version_ = ec_version;
}
diff --git a/common/subprocess_unittest.cc b/common/subprocess_unittest.cc
index 58d4191..7dbdf98 100644
--- a/common/subprocess_unittest.cc
+++ b/common/subprocess_unittest.cc
@@ -88,7 +88,7 @@
void ExpectedEnvVars(int return_code, const string& output) {
EXPECT_EQ(0, return_code);
const std::set<string> allowed_envs = {"LD_LIBRARY_PATH", "PATH"};
- for (string key_value : brillo::string_utils::Split(output, "\n")) {
+ for (const string& key_value : brillo::string_utils::Split(output, "\n")) {
auto key_value_pair = brillo::string_utils::SplitAtFirst(
key_value, "=", true);
EXPECT_NE(allowed_envs.end(), allowed_envs.find(key_value_pair.first));
diff --git a/common/utils.cc b/common/utils.cc
index a352961..101e0b8 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -246,7 +246,7 @@
return true;
}
-bool WriteAll(FileDescriptorPtr fd, const void* buf, size_t count) {
+bool WriteAll(const FileDescriptorPtr& fd, const void* buf, size_t count) {
const char* c_buf = static_cast<const char*>(buf);
ssize_t bytes_written = 0;
while (bytes_written < static_cast<ssize_t>(count)) {
@@ -257,7 +257,7 @@
return true;
}
-bool PWriteAll(FileDescriptorPtr fd,
+bool PWriteAll(const FileDescriptorPtr& fd,
const void* buf,
size_t count,
off_t offset) {
@@ -283,7 +283,7 @@
return true;
}
-bool PReadAll(FileDescriptorPtr fd, void* buf, size_t count, off_t offset,
+bool PReadAll(const FileDescriptorPtr& fd, void* buf, size_t count, off_t offset,
ssize_t* out_bytes_read) {
TEST_AND_RETURN_FALSE_ERRNO(fd->Seek(offset, SEEK_SET) !=
static_cast<off_t>(-1));
diff --git a/common/utils.h b/common/utils.h
index 3987484..6bc030b 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -69,8 +69,8 @@
bool WriteAll(int fd, const void* buf, size_t count);
bool PWriteAll(int fd, const void* buf, size_t count, off_t offset);
-bool WriteAll(FileDescriptorPtr fd, const void* buf, size_t count);
-bool PWriteAll(FileDescriptorPtr fd,
+bool WriteAll(const FileDescriptorPtr& fd, const void* buf, size_t count);
+bool PWriteAll(const FileDescriptorPtr& fd,
const void* buf,
size_t count,
off_t offset);
@@ -88,7 +88,7 @@
bool PReadAll(int fd, void* buf, size_t count, off_t offset,
ssize_t* out_bytes_read);
-bool PReadAll(FileDescriptorPtr fd, void* buf, size_t count, off_t offset,
+bool PReadAll(const FileDescriptorPtr& fd, void* buf, size_t count, off_t offset,
ssize_t* out_bytes_read);
// Opens |path| for reading and appends its entire content to the container
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 072cd57..a1cf2ab 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -135,7 +135,7 @@
// Discard the tail of the block device referenced by |fd|, from the offset
// |data_size| until the end of the block device. Returns whether the data was
// discarded.
-bool DiscardPartitionTail(FileDescriptorPtr fd, uint64_t data_size) {
+bool DiscardPartitionTail(const FileDescriptorPtr& fd, uint64_t data_size) {
uint64_t part_size = fd->BlockDevSize();
if (!part_size || part_size <= data_size)
return false;
@@ -951,8 +951,7 @@
#endif // !defined(BLKZEROOUT)
brillo::Blob zeros;
- for (int i = 0; i < operation.dst_extents_size(); i++) {
- Extent extent = operation.dst_extents(i);
+ for (const Extent& extent : operation.dst_extents()) {
const uint64_t start = extent.start_block() * block_size_;
const uint64_t length = extent.num_blocks() * block_size_;
if (attempt_ioctl) {
@@ -1030,7 +1029,7 @@
// each block in |extents|. For example, [(3, 2), (8, 1)] would give [3, 4, 8].
void ExtentsToBlocks(const RepeatedPtrField<Extent>& extents,
vector<uint64_t>* blocks) {
- for (Extent ext : extents) {
+ for (const Extent& ext : extents) {
for (uint64_t j = 0; j < ext.num_blocks(); j++)
blocks->push_back(ext.start_block() + j);
}
@@ -1039,7 +1038,7 @@
// Takes |extents| and returns the number of blocks in those extents.
uint64_t GetBlockCount(const RepeatedPtrField<Extent>& extents) {
uint64_t sum = 0;
- for (Extent ext : extents) {
+ for (const Extent& ext : extents) {
sum += ext.num_blocks();
}
return sum;
@@ -1151,8 +1150,7 @@
string* positions_string) {
string ret;
uint64_t length = 0;
- for (int i = 0; i < extents.size(); i++) {
- Extent extent = extents.Get(i);
+ for (const Extent& extent : extents) {
int64_t start = extent.start_block() * block_size;
uint64_t this_length =
min(full_length - length,
@@ -1709,7 +1707,7 @@
}
bool DeltaPerformer::CanResumeUpdate(PrefsInterface* prefs,
- string update_check_response_hash) {
+ const string& update_check_response_hash) {
int64_t next_operation = kUpdateStateOperationInvalid;
if (!(prefs->GetInt64(kPrefsUpdateStateNextOperation, &next_operation) &&
next_operation != kUpdateStateOperationInvalid &&
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 7af59e6..74143e0 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -140,7 +140,7 @@
// Returns true if a previous update attempt can be continued based on the
// persistent preferences and the new update check response hash.
static bool CanResumeUpdate(PrefsInterface* prefs,
- std::string update_check_response_hash);
+ const std::string& update_check_response_hash);
// Resets the persistent update progress state to indicate that an update
// can't be resumed. Performs a quick update-in-progress reset if |quick| is
diff --git a/payload_generator/ab_generator.cc b/payload_generator/ab_generator.cc
index 8c736a5..efb8ccf 100644
--- a/payload_generator/ab_generator.cc
+++ b/payload_generator/ab_generator.cc
@@ -110,7 +110,7 @@
int curr_src_ext_index = 0;
Extent curr_src_ext = original_op.src_extents(curr_src_ext_index);
for (int i = 0; i < original_op.dst_extents_size(); i++) {
- Extent dst_ext = original_op.dst_extents(i);
+ const Extent& dst_ext = original_op.dst_extents(i);
// The new operation which will have only one dst extent.
InstallOperation new_op;
uint64_t blocks_left = dst_ext.num_blocks();
@@ -165,7 +165,7 @@
uint32_t data_offset = original_op.data_offset();
for (int i = 0; i < original_op.dst_extents_size(); i++) {
- Extent dst_ext = original_op.dst_extents(i);
+ const Extent& dst_ext = original_op.dst_extents(i);
// Make a new operation with only one dst extent.
InstallOperation new_op;
*(new_op.add_dst_extents()) = dst_ext;
diff --git a/payload_generator/ab_generator_unittest.cc b/payload_generator/ab_generator_unittest.cc
index 224880d..3fd2323 100644
--- a/payload_generator/ab_generator_unittest.cc
+++ b/payload_generator/ab_generator_unittest.cc
@@ -42,7 +42,7 @@
namespace {
-bool ExtentEquals(Extent ext, uint64_t start_block, uint64_t num_blocks) {
+bool ExtentEquals(const Extent& ext, uint64_t start_block, uint64_t num_blocks) {
return ext.start_block() == start_block && ext.num_blocks() == num_blocks;
}
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 41bf0b3..50fbdf2 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -370,7 +370,7 @@
// TODO(deymo): Produce ZERO operations instead of calling DeltaReadFile().
size_t num_ops = aops->size();
new_visited_blocks->AddExtents(new_zeros);
- for (Extent extent : new_zeros) {
+ for (const Extent& extent : new_zeros) {
TEST_AND_RETURN_FALSE(DeltaReadFile(aops,
"",
new_part,
@@ -391,7 +391,7 @@
uint64_t used_blocks = 0;
old_visited_blocks->AddExtents(old_identical_blocks);
new_visited_blocks->AddExtents(new_identical_blocks);
- for (Extent extent : new_identical_blocks) {
+ for (const Extent& extent : new_identical_blocks) {
// We split the operation at the extent boundary or when bigger than
// chunk_blocks.
for (uint64_t op_block_offset = 0; op_block_offset < extent.num_blocks();
diff --git a/payload_generator/fake_filesystem.cc b/payload_generator/fake_filesystem.cc
index c765286..234e2f6 100644
--- a/payload_generator/fake_filesystem.cc
+++ b/payload_generator/fake_filesystem.cc
@@ -39,7 +39,7 @@
}
void FakeFilesystem::AddFile(const std::string& filename,
- const std::vector<Extent> extents) {
+ const std::vector<Extent>& extents) {
File file;
file.name = filename;
file.extents = extents;
diff --git a/payload_generator/fake_filesystem.h b/payload_generator/fake_filesystem.h
index a14b8d3..1b13920 100644
--- a/payload_generator/fake_filesystem.h
+++ b/payload_generator/fake_filesystem.h
@@ -43,7 +43,7 @@
// Fake methods.
// Add a file to the list of fake files.
- void AddFile(const std::string& filename, const std::vector<Extent> extents);
+ void AddFile(const std::string& filename, const std::vector<Extent>& extents);
// Sets the PAYLOAD_MINOR_VERSION key stored by LoadSettings(). Use a negative
// value to produce an error in LoadSettings().
diff --git a/payload_generator/inplace_generator.cc b/payload_generator/inplace_generator.cc
index be8b487..bc140e8 100644
--- a/payload_generator/inplace_generator.cc
+++ b/payload_generator/inplace_generator.cc
@@ -704,7 +704,7 @@
bool InplaceGenerator::AddInstallOpToGraph(Graph* graph,
Vertex::Index existing_vertex,
vector<Block>* blocks,
- const InstallOperation operation,
+ const InstallOperation& operation,
const string& op_name) {
Vertex::Index vertex = existing_vertex;
if (vertex == Vertex::kInvalidIndex) {
diff --git a/payload_generator/inplace_generator.h b/payload_generator/inplace_generator.h
index 48a1fac..f108639 100644
--- a/payload_generator/inplace_generator.h
+++ b/payload_generator/inplace_generator.h
@@ -193,7 +193,7 @@
static bool AddInstallOpToGraph(Graph* graph,
Vertex::Index existing_vertex,
std::vector<Block>* blocks,
- const InstallOperation operation,
+ const InstallOperation& operation,
const std::string& op_name);
// Apply the transformation stored in |the_map| to the |collection| vector
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index e159f08..62b6e7a 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -174,7 +174,7 @@
EXPECT_EQ(1, signatures.signatures_size());
const Signatures_Signature& signature = signatures.signatures(0);
EXPECT_EQ(1U, signature.version());
- const string sig_data = signature.data();
+ const string& sig_data = signature.data();
ASSERT_EQ(arraysize(kDataSignature), sig_data.size());
for (size_t i = 0; i < arraysize(kDataSignature); i++) {
EXPECT_EQ(kDataSignature[i], static_cast<uint8_t>(sig_data[i]));