Use raw pointer instead of shared_ptr am: 1a0ed71c16
Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/1962420
Change-Id: I69c1333e0d8e29b557833c674a3d0686dbc3a148
diff --git a/aosp/dynamic_partition_control_android.cc b/aosp/dynamic_partition_control_android.cc
index 825e0d6..6d33a09 100644
--- a/aosp/dynamic_partition_control_android.cc
+++ b/aosp/dynamic_partition_control_android.cc
@@ -1420,7 +1420,7 @@
return snapshot_->OpenSnapshotWriter(params, std::move(source_path));
} // namespace chromeos_update_engine
-FileDescriptorPtr DynamicPartitionControlAndroid::OpenCowFd(
+std::unique_ptr<FileDescriptor> DynamicPartitionControlAndroid::OpenCowFd(
const std::string& unsuffixed_partition_name,
const std::optional<std::string>& source_path,
bool is_append) {
@@ -1438,7 +1438,7 @@
LOG(ERROR) << "ICowWriter::OpenReader() failed.";
return nullptr;
}
- return std::make_shared<CowWriterFileDescriptor>(std::move(cow_writer),
+ return std::make_unique<CowWriterFileDescriptor>(std::move(cow_writer),
std::move(reader));
}
diff --git a/aosp/dynamic_partition_control_android.h b/aosp/dynamic_partition_control_android.h
index d0842a9..cebca07 100644
--- a/aosp/dynamic_partition_control_android.h
+++ b/aosp/dynamic_partition_control_android.h
@@ -104,9 +104,10 @@
const std::string& unsuffixed_partition_name,
const std::optional<std::string>& source_path,
bool is_append) override;
- FileDescriptorPtr OpenCowFd(const std::string& unsuffixed_partition_name,
- const std::optional<std::string>&,
- bool is_append = false) override;
+ std::unique_ptr<FileDescriptor> OpenCowFd(
+ const std::string& unsuffixed_partition_name,
+ const std::optional<std::string>&,
+ bool is_append = false) override;
bool MapAllPartitions() override;
bool UnmapAllPartitions() override;
diff --git a/aosp/mock_dynamic_partition_control_android.h b/aosp/mock_dynamic_partition_control_android.h
index 428b6c7..33ef39c 100644
--- a/aosp/mock_dynamic_partition_control_android.h
+++ b/aosp/mock_dynamic_partition_control_android.h
@@ -94,7 +94,7 @@
const std::optional<std::string>& source_path,
bool is_append),
(override));
- MOCK_METHOD(FileDescriptorPtr,
+ MOCK_METHOD(std::unique_ptr<FileDescriptor>,
OpenCowFd,
(const std::string& unsuffixed_partition_name,
const std::optional<std::string>& source_path,
diff --git a/common/dynamic_partition_control_interface.h b/common/dynamic_partition_control_interface.h
index a5be6e1..e6ebe6a 100644
--- a/common/dynamic_partition_control_interface.h
+++ b/common/dynamic_partition_control_interface.h
@@ -167,7 +167,7 @@
bool is_append = false) = 0;
// Open a general purpose FD capable to reading and writing to COW. Note that
// writes must be block aligned.
- virtual FileDescriptorPtr OpenCowFd(
+ virtual std::unique_ptr<FileDescriptor> OpenCowFd(
const std::string& unsuffixed_partition_name,
const std::optional<std::string>&,
bool is_append = false) = 0;
diff --git a/common/dynamic_partition_control_stub.h b/common/dynamic_partition_control_stub.h
index 515ec7c..5aa4336 100644
--- a/common/dynamic_partition_control_stub.h
+++ b/common/dynamic_partition_control_stub.h
@@ -65,9 +65,10 @@
const std::optional<std::string>&,
bool is_append) override;
- FileDescriptorPtr OpenCowFd(const std::string& unsuffixed_partition_name,
- const std::optional<std::string>&,
- bool is_append = false) override {
+ std::unique_ptr<FileDescriptor> OpenCowFd(
+ const std::string& unsuffixed_partition_name,
+ const std::optional<std::string>&,
+ bool is_append = false) override {
return nullptr;
}
diff --git a/common/mock_dynamic_partition_control.h b/common/mock_dynamic_partition_control.h
index c6b0b2d..f3a446a 100644
--- a/common/mock_dynamic_partition_control.h
+++ b/common/mock_dynamic_partition_control.h
@@ -36,7 +36,7 @@
MOCK_METHOD(FeatureFlag, GetVirtualAbCompressionFeatureFlag, (), (override));
MOCK_METHOD(FeatureFlag, GetVirtualAbFeatureFlag, (), (override));
MOCK_METHOD(bool, FinishUpdate, (bool), (override));
- MOCK_METHOD(FileDescriptorPtr,
+ MOCK_METHOD(std::unique_ptr<FileDescriptor>,
OpenCowFd,
(const std::string& unsuffixed_partition_name,
const std::optional<std::string>& source_path,
diff --git a/common/utils.cc b/common/utils.cc
index aa6c6b3..794b832 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -181,7 +181,7 @@
return true;
}
-bool WriteAll(const FileDescriptorPtr& fd, const void* buf, size_t count) {
+bool WriteAll(FileDescriptor* 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)) {
@@ -218,7 +218,7 @@
return true;
}
-bool ReadAll(const FileDescriptorPtr& fd,
+bool ReadAll(FileDescriptor* fd,
void* buf,
size_t count,
off_t offset,
@@ -239,7 +239,7 @@
return true;
}
-bool PReadAll(const FileDescriptorPtr& fd,
+bool PReadAll(FileDescriptor* fd,
void* buf,
size_t count,
off_t offset,
diff --git a/common/utils.h b/common/utils.h
index a33efb2..0f8da22 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -68,12 +68,15 @@
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(const FileDescriptorPtr& fd, const void* buf, size_t count);
+bool WriteAll(FileDescriptor* fd, const void* buf, size_t count);
+
+constexpr bool WriteAll(const FileDescriptorPtr& fd,
+ const void* buf,
+ size_t count) {
+ return WriteAll(fd.get(), buf, count);
+}
// WriteAll writes data at specified offset, but it modifies file position.
-bool WriteAll(const FileDescriptorPtr& fd,
- const void* buf,
- size_t count,
- off_t off);
+bool WriteAll(FileDescriptorPtr* fd, const void* buf, size_t count, off_t off);
// https://man7.org/linux/man-pages/man2/pread.2.html
// PWriteAll writes data at specified offset, but it DOES NOT modify file
@@ -97,21 +100,38 @@
int fd, void* buf, size_t count, off_t offset, ssize_t* out_bytes_read);
// Reads data at specified offset, this function does change file position.
-bool ReadAll(const FileDescriptorPtr& fd,
+
+bool ReadAll(FileDescriptor* fd,
void* buf,
size_t count,
off_t offset,
ssize_t* out_bytes_read);
+constexpr bool ReadAll(const FileDescriptorPtr& fd,
+ void* buf,
+ size_t count,
+ off_t offset,
+ ssize_t* out_bytes_read) {
+ return ReadAll(fd.get(), buf, count, offset, out_bytes_read);
+}
+
// https://man7.org/linux/man-pages/man2/pread.2.html
// Reads data at specified offset, this function DOES NOT change file position.
// Behavior is similar to linux's pread syscall.
-bool PReadAll(const FileDescriptorPtr& fd,
+bool PReadAll(FileDescriptor* fd,
void* buf,
size_t count,
off_t offset,
ssize_t* out_bytes_read);
+constexpr bool PReadAll(const FileDescriptorPtr& fd,
+ void* buf,
+ size_t count,
+ off_t offset,
+ ssize_t* out_bytes_read) {
+ return PReadAll(fd.get(), buf, count, offset, out_bytes_read);
+}
+
// Opens |path| for reading and appends its entire content to the container
// pointed to by |out_p|. Returns true upon successfully reading all of the
// file's content, false otherwise, in which case the state of the output
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index a8b9269..2770aff 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -182,7 +182,7 @@
}
bool FilesystemVerifierAction::InitializeFd(const std::string& part_path) {
- partition_fd_ = FileDescriptorPtr(new EintrSafeFileDescriptor());
+ partition_fd_ = std::make_unique<EintrSafeFileDescriptor>();
const bool write_verity = ShouldWriteVerity();
int flags = write_verity ? O_RDWR : O_RDONLY;
if (!utils::SetBlockDeviceReadOnly(part_path, !write_verity)) {
@@ -197,11 +197,12 @@
}
void FilesystemVerifierAction::WriteVerityAndHashPartition(
- FileDescriptorPtr fd,
const off64_t start_offset,
const off64_t end_offset,
void* buffer,
const size_t buffer_size) {
+ auto fd = partition_fd_.get();
+ TEST_AND_RETURN(fd != nullptr);
if (start_offset >= end_offset) {
LOG_IF(WARNING, start_offset > end_offset)
<< "start_offset is greater than end_offset : " << start_offset << " > "
@@ -219,7 +220,7 @@
return;
}
}
- HashPartition(partition_fd_, 0, partition_size_, buffer, buffer_size);
+ HashPartition(0, partition_size_, buffer, buffer_size);
return;
}
const auto cur_offset = fd->Seek(start_offset, SEEK_SET);
@@ -249,18 +250,18 @@
FROM_HERE,
base::BindOnce(&FilesystemVerifierAction::WriteVerityAndHashPartition,
base::Unretained(this),
- fd,
start_offset + bytes_read,
end_offset,
buffer,
buffer_size)));
}
-void FilesystemVerifierAction::HashPartition(FileDescriptorPtr fd,
- const off64_t start_offset,
+void FilesystemVerifierAction::HashPartition(const off64_t start_offset,
const off64_t end_offset,
void* buffer,
const size_t buffer_size) {
+ auto fd = partition_fd_.get();
+ TEST_AND_RETURN(fd != nullptr);
if (start_offset >= end_offset) {
LOG_IF(WARNING, start_offset > end_offset)
<< "start_offset is greater than end_offset : " << start_offset << " > "
@@ -295,7 +296,6 @@
FROM_HERE,
base::BindOnce(&FilesystemVerifierAction::HashPartition,
base::Unretained(this),
- fd,
start_offset + bytes_read,
end_offset,
buffer,
@@ -361,6 +361,7 @@
CHECK_LE(partition.hash_tree_offset, partition.fec_offset)
<< " Hash tree is expected to come before FEC data";
}
+ CHECK_NE(partition_fd_, nullptr);
if (partition.hash_tree_offset != 0) {
filesystem_data_end_ = partition.hash_tree_offset;
} else if (partition.fec_offset != 0) {
@@ -374,11 +375,10 @@
return;
}
WriteVerityAndHashPartition(
- partition_fd_, 0, filesystem_data_end_, buffer_.data(), buffer_.size());
+ 0, filesystem_data_end_, buffer_.data(), buffer_.size());
} else {
LOG(INFO) << "Verity writes disabled on partition " << partition.name;
- HashPartition(
- partition_fd_, 0, partition_size_, buffer_.data(), buffer_.size());
+ HashPartition(0, partition_size_, buffer_.data(), buffer_.size());
}
}
@@ -430,7 +430,7 @@
Cleanup(ErrorCode::kError);
return;
}
- InstallPlan::Partition& partition =
+ const InstallPlan::Partition& partition =
install_plan_.partitions[partition_index_];
LOG(INFO) << "Hash of " << partition.name << ": "
<< HexEncode(hasher_->raw_hash());
@@ -492,7 +492,6 @@
return;
}
// Start hashing the next partition, if any.
- hasher_.reset();
buffer_.clear();
if (partition_fd_) {
partition_fd_->Close();
diff --git a/payload_consumer/filesystem_verifier_action.h b/payload_consumer/filesystem_verifier_action.h
index 850abda..edc8e53 100644
--- a/payload_consumer/filesystem_verifier_action.h
+++ b/payload_consumer/filesystem_verifier_action.h
@@ -86,13 +86,11 @@
private:
friend class FilesystemVerifierActionTestDelegate;
- void WriteVerityAndHashPartition(FileDescriptorPtr fd,
- const off64_t start_offset,
+ void WriteVerityAndHashPartition(const off64_t start_offset,
const off64_t end_offset,
void* buffer,
const size_t buffer_size);
- void HashPartition(FileDescriptorPtr fd,
- const off64_t start_offset,
+ void HashPartition(const off64_t start_offset,
const off64_t end_offset,
void* buffer,
const size_t buffer_size);
@@ -138,7 +136,7 @@
// If not null, the FileDescriptor used to read from the device.
// verity writer might attempt to write to this fd, if verity is enabled.
- FileDescriptorPtr partition_fd_;
+ std::unique_ptr<FileDescriptor> partition_fd_;
// Buffer for storing data we read.
brillo::Blob buffer_;
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index f2f2954..533292a 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -170,7 +170,7 @@
bytes_to_read -= bytes_read;
offset += bytes_read;
}
- ASSERT_TRUE(verity_writer.Finalize(fd, fd));
+ ASSERT_TRUE(verity_writer.Finalize(fd.get(), fd.get()));
ASSERT_TRUE(fd->IsOpen());
ASSERT_TRUE(HashCalculator::RawHashOfFile(target_part_.path(),
&partition->target_hash));
@@ -565,7 +565,7 @@
EnableVABC(&dynamic_control, part.name);
auto open_cow = [part]() {
- auto cow_fd = std::make_shared<EintrSafeFileDescriptor>();
+ auto cow_fd = std::make_unique<EintrSafeFileDescriptor>();
EXPECT_TRUE(cow_fd->Open(part.readonly_target_path.c_str(), O_RDWR))
<< "Failed to open part " << part.readonly_target_path
<< strerror(errno);
@@ -618,14 +618,14 @@
if (enable_verity) {
std::vector<unsigned char> actual_fec(fec_size);
ssize_t bytes_read = 0;
- ASSERT_TRUE(utils::PReadAll(cow_fd,
+ ASSERT_TRUE(utils::PReadAll(cow_fd.get(),
actual_fec.data(),
actual_fec.size(),
fec_start_offset,
&bytes_read));
ASSERT_EQ(actual_fec, fec_data_);
std::vector<unsigned char> actual_hash_tree(hash_tree_size);
- ASSERT_TRUE(utils::PReadAll(cow_fd,
+ ASSERT_TRUE(utils::PReadAll(cow_fd.get(),
actual_hash_tree.data(),
actual_hash_tree.size(),
HASH_TREE_START_OFFSET,
diff --git a/payload_consumer/verity_writer_android.cc b/payload_consumer/verity_writer_android.cc
index e2fab7d..b233b58 100644
--- a/payload_consumer/verity_writer_android.cc
+++ b/payload_consumer/verity_writer_android.cc
@@ -104,8 +104,8 @@
return true;
}
-bool VerityWriterAndroid::Finalize(FileDescriptorPtr read_fd,
- FileDescriptorPtr write_fd) {
+bool VerityWriterAndroid::Finalize(FileDescriptor* read_fd,
+ FileDescriptor* write_fd) {
const auto hash_tree_data_end =
partition_->hash_tree_data_offset + partition_->hash_tree_data_size;
if (total_offset_ < hash_tree_data_end) {
@@ -142,8 +142,8 @@
return true;
}
-bool VerityWriterAndroid::EncodeFEC(FileDescriptorPtr read_fd,
- FileDescriptorPtr write_fd,
+bool VerityWriterAndroid::EncodeFEC(FileDescriptor* read_fd,
+ FileDescriptor* write_fd,
uint64_t data_offset,
uint64_t data_size,
uint64_t fec_offset,
@@ -162,10 +162,6 @@
init_rs_char(FEC_PARAMS(fec_roots)), &free_rs_char);
TEST_AND_RETURN_FALSE(rs_char != nullptr);
- // Cache at most 1MB of fec data, in VABC, we need to re-open fd if we
- // perform a read() operation after write(). So reduce the number of writes
- // can save unnecessary re-opens.
- write_fd = std::make_shared<CachedFileDescriptor>(write_fd, 1 * (1 << 20));
for (size_t i = 0; i < rounds; i++) {
// Encodes |block_size| number of rs blocks each round so that we can read
// one block each time instead of 1 byte to increase random read
@@ -229,11 +225,10 @@
uint32_t fec_roots,
uint32_t block_size,
bool verify_mode) {
- FileDescriptorPtr fd(new EintrSafeFileDescriptor());
- TEST_AND_RETURN_FALSE(
- fd->Open(path.c_str(), verify_mode ? O_RDONLY : O_RDWR));
- return EncodeFEC(fd,
- fd,
+ EintrSafeFileDescriptor fd;
+ TEST_AND_RETURN_FALSE(fd.Open(path.c_str(), verify_mode ? O_RDONLY : O_RDWR));
+ return EncodeFEC(&fd,
+ &fd,
data_offset,
data_size,
fec_offset,
diff --git a/payload_consumer/verity_writer_android.h b/payload_consumer/verity_writer_android.h
index 8339528..a6a4920 100644
--- a/payload_consumer/verity_writer_android.h
+++ b/payload_consumer/verity_writer_android.h
@@ -34,7 +34,7 @@
bool Init(const InstallPlan::Partition& partition);
bool Update(uint64_t offset, const uint8_t* buffer, size_t size) override;
- bool Finalize(FileDescriptorPtr read_fd, FileDescriptorPtr write_fd) override;
+ bool Finalize(FileDescriptor* read_fd, FileDescriptor* write_fd) override;
// Read [data_offset : data_offset + data_size) from |path| and encode FEC
// data, if |verify_mode|, then compare the encoded FEC with the one in
@@ -42,8 +42,8 @@
// in each Update() like hash tree, because for every rs block, its data are
// spreaded across entire |data_size|, unless we can cache all data in
// memory, we have to re-read them from disk.
- static bool EncodeFEC(FileDescriptorPtr read_fd,
- FileDescriptorPtr write_fd,
+ static bool EncodeFEC(FileDescriptor* read_fd,
+ FileDescriptor* write_fd,
uint64_t data_offset,
uint64_t data_size,
uint64_t fec_offset,
diff --git a/payload_consumer/verity_writer_android_unittest.cc b/payload_consumer/verity_writer_android_unittest.cc
index 75da0ae..693bcda 100644
--- a/payload_consumer/verity_writer_android_unittest.cc
+++ b/payload_consumer/verity_writer_android_unittest.cc
@@ -54,7 +54,8 @@
ASSERT_TRUE(verity_writer_.Init(partition_));
ASSERT_TRUE(verity_writer_.Update(0, part_data.data(), 4096));
ASSERT_TRUE(verity_writer_.Update(4096, part_data.data() + 4096, 4096));
- ASSERT_TRUE(verity_writer_.Finalize(partition_fd_, partition_fd_));
+ ASSERT_TRUE(
+ verity_writer_.Finalize(partition_fd_.get(), partition_fd_.get()));
brillo::Blob actual_part;
utils::ReadFile(partition_.target_path, &actual_part);
// dd if=/dev/zero bs=4096 count=1 2>/dev/null | sha1sum | xxd -r -p |
@@ -102,7 +103,8 @@
ASSERT_TRUE(verity_writer_.Init(partition_));
ASSERT_TRUE(verity_writer_.Update(0, part_data.data(), 4096));
ASSERT_TRUE(verity_writer_.Update(4096, part_data.data() + 4096, 4096));
- ASSERT_TRUE(verity_writer_.Finalize(partition_fd_, partition_fd_));
+ ASSERT_TRUE(
+ verity_writer_.Finalize(partition_fd_.get(), partition_fd_.get()));
brillo::Blob actual_part;
utils::ReadFile(partition_.target_path, &actual_part);
// dd if=/dev/zero bs=4096 count=1 2>/dev/null | sha256sum | xxd -r -p |
@@ -127,7 +129,8 @@
ASSERT_TRUE(verity_writer_.Update(4096, part_data.data() + 4096, 4096));
ASSERT_TRUE(verity_writer_.Update(
8192, part_data.data() + 8192, partition_.hash_tree_data_offset));
- ASSERT_TRUE(verity_writer_.Finalize(partition_fd_, partition_fd_));
+ ASSERT_TRUE(
+ verity_writer_.Finalize(partition_fd_.get(), partition_fd_.get()));
brillo::Blob actual_part;
utils::ReadFile(partition_.target_path, &actual_part);
// dd if=/dev/zero bs=4096 count=1 2>/dev/null | sha256sum | xxd -r -p |
@@ -150,7 +153,8 @@
test_utils::WriteFileVector(partition_.target_path, part_data);
ASSERT_TRUE(verity_writer_.Init(partition_));
ASSERT_TRUE(verity_writer_.Update(0, part_data.data(), part_data.size()));
- ASSERT_TRUE(verity_writer_.Finalize(partition_fd_, partition_fd_));
+ ASSERT_TRUE(
+ verity_writer_.Finalize(partition_fd_.get(), partition_fd_.get()));
brillo::Blob actual_part;
utils::ReadFile(partition_.target_path, &actual_part);
// Write FEC data.
diff --git a/payload_consumer/verity_writer_interface.h b/payload_consumer/verity_writer_interface.h
index 37ed605..432ede7 100644
--- a/payload_consumer/verity_writer_interface.h
+++ b/payload_consumer/verity_writer_interface.h
@@ -39,8 +39,7 @@
virtual bool Update(uint64_t offset, const uint8_t* buffer, size_t size) = 0;
// Write hash tree && FEC data to underlying fd, if they are present
- virtual bool Finalize(FileDescriptorPtr read_fd,
- FileDescriptorPtr write_fd) = 0;
+ virtual bool Finalize(FileDescriptor* read_fd, FileDescriptor* write_fd) = 0;
protected:
VerityWriterInterface() = default;