Refactor extent writer to take filedescriptor in constructor am: 4d22ca2ab6 am: ac10c6c57a
Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/1581806
Change-Id: Ibd6bd0d9e702f8b73fbac3ae8b3d97f8f1e55e08
diff --git a/payload_consumer/bzip_extent_writer.cc b/payload_consumer/bzip_extent_writer.cc
index 0c25c71..26fdc5f 100644
--- a/payload_consumer/bzip_extent_writer.cc
+++ b/payload_consumer/bzip_extent_writer.cc
@@ -29,8 +29,7 @@
TEST_AND_RETURN(input_buffer_.empty());
}
-bool BzipExtentWriter::Init(FileDescriptorPtr fd,
- const RepeatedPtrField<Extent>& extents,
+bool BzipExtentWriter::Init(const RepeatedPtrField<Extent>& extents,
uint32_t block_size) {
// Init bzip2 stream
int rc = BZ2_bzDecompressInit(&stream_,
@@ -39,7 +38,7 @@
TEST_AND_RETURN_FALSE(rc == BZ_OK);
- return next_->Init(fd, extents, block_size);
+ return next_->Init(extents, block_size);
}
bool BzipExtentWriter::Write(const void* bytes, size_t count) {
diff --git a/payload_consumer/bzip_extent_writer.h b/payload_consumer/bzip_extent_writer.h
index ec181a7..38c041a 100644
--- a/payload_consumer/bzip_extent_writer.h
+++ b/payload_consumer/bzip_extent_writer.h
@@ -40,8 +40,7 @@
}
~BzipExtentWriter() override;
- bool Init(FileDescriptorPtr fd,
- const google::protobuf::RepeatedPtrField<Extent>& extents,
+ bool Init(const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override;
bool Write(const void* bytes, size_t count) override;
diff --git a/payload_consumer/bzip_extent_writer_unittest.cc b/payload_consumer/bzip_extent_writer_unittest.cc
index b587040..c93545a 100644
--- a/payload_consumer/bzip_extent_writer_unittest.cc
+++ b/payload_consumer/bzip_extent_writer_unittest.cc
@@ -29,7 +29,6 @@
#include "update_engine/common/utils.h"
#include "update_engine/payload_generator/extent_ranges.h"
-using google::protobuf::RepeatedPtrField;
using std::min;
using std::string;
using std::vector;
@@ -64,9 +63,8 @@
0x22, 0x9c, 0x28, 0x48, 0x66, 0x61, 0xb8, 0xea, 0x00,
};
- BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>());
- EXPECT_TRUE(
- bzip_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>(fd_));
+ EXPECT_TRUE(bzip_writer.Init({extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(bzip_writer.Write(test, sizeof(test)));
brillo::Blob buf;
@@ -97,9 +95,8 @@
vector<Extent> extents = {ExtentForBytes(kBlockSize, 0, kDecompressedLength)};
- BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>());
- EXPECT_TRUE(
- bzip_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>(fd_));
+ EXPECT_TRUE(bzip_writer.Init({extents.begin(), extents.end()}, kBlockSize));
brillo::Blob original_compressed_data = compressed_data;
for (brillo::Blob::size_type i = 0; i < compressed_data.size();
diff --git a/payload_consumer/extent_writer.h b/payload_consumer/extent_writer.h
index 9e53561..8b1b532 100644
--- a/payload_consumer/extent_writer.h
+++ b/payload_consumer/extent_writer.h
@@ -38,8 +38,7 @@
virtual ~ExtentWriter() = default;
// Returns true on success.
- virtual bool Init(FileDescriptorPtr fd,
- const google::protobuf::RepeatedPtrField<Extent>& extents,
+ virtual bool Init(const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) = 0;
// Returns true on success.
@@ -51,13 +50,11 @@
class DirectExtentWriter : public ExtentWriter {
public:
- DirectExtentWriter() = default;
+ explicit DirectExtentWriter(FileDescriptorPtr fd) : fd_(fd) {}
~DirectExtentWriter() override = default;
- bool Init(FileDescriptorPtr fd,
- const google::protobuf::RepeatedPtrField<Extent>& extents,
+ bool Init(const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override {
- fd_ = fd;
block_size_ = block_size;
extents_ = extents;
cur_extent_ = extents_.begin();
diff --git a/payload_consumer/extent_writer_unittest.cc b/payload_consumer/extent_writer_unittest.cc
index afebb1a..5c67d3e 100644
--- a/payload_consumer/extent_writer_unittest.cc
+++ b/payload_consumer/extent_writer_unittest.cc
@@ -65,9 +65,8 @@
TEST_F(ExtentWriterTest, SimpleTest) {
vector<Extent> extents = {ExtentForRange(1, 1)};
const string bytes = "1234";
- DirectExtentWriter direct_writer;
- EXPECT_TRUE(
- direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ DirectExtentWriter direct_writer{fd_};
+ EXPECT_TRUE(direct_writer.Init({extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(direct_writer.Write(bytes.data(), bytes.size()));
EXPECT_EQ(static_cast<off_t>(kBlockSize + bytes.size()),
@@ -84,9 +83,8 @@
TEST_F(ExtentWriterTest, ZeroLengthTest) {
vector<Extent> extents = {ExtentForRange(1, 1)};
- DirectExtentWriter direct_writer;
- EXPECT_TRUE(
- direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ DirectExtentWriter direct_writer{fd_};
+ EXPECT_TRUE(direct_writer.Init({extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(direct_writer.Write(nullptr, 0));
}
@@ -109,9 +107,8 @@
brillo::Blob data(kBlockSize * 3);
test_utils::FillWithData(&data);
- DirectExtentWriter direct_writer;
- EXPECT_TRUE(
- direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ DirectExtentWriter direct_writer{fd_};
+ EXPECT_TRUE(direct_writer.Init({extents.begin(), extents.end()}, kBlockSize));
size_t bytes_written = 0;
while (bytes_written < data.size()) {
@@ -150,9 +147,8 @@
brillo::Blob data(17);
test_utils::FillWithData(&data);
- DirectExtentWriter direct_writer;
- EXPECT_TRUE(
- direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ DirectExtentWriter direct_writer{fd_};
+ EXPECT_TRUE(direct_writer.Init({extents.begin(), extents.end()}, kBlockSize));
size_t bytes_written = 0;
while (bytes_written < (block_count * kBlockSize)) {
diff --git a/payload_consumer/fake_extent_writer.h b/payload_consumer/fake_extent_writer.h
index 7b2b7ac..680b1b3 100644
--- a/payload_consumer/fake_extent_writer.h
+++ b/payload_consumer/fake_extent_writer.h
@@ -33,8 +33,7 @@
~FakeExtentWriter() override = default;
// ExtentWriter overrides.
- bool Init(FileDescriptorPtr /* fd */,
- const google::protobuf::RepeatedPtrField<Extent>& /* extents */,
+ bool Init(const google::protobuf::RepeatedPtrField<Extent>& /* extents */,
uint32_t /* block_size */) override {
init_called_ = true;
return true;
diff --git a/payload_consumer/file_descriptor_utils.cc b/payload_consumer/file_descriptor_utils.cc
index 846cbd7..9a6a601 100644
--- a/payload_consumer/file_descriptor_utils.cc
+++ b/payload_consumer/file_descriptor_utils.cc
@@ -82,8 +82,8 @@
const RepeatedPtrField<Extent>& tgt_extents,
uint64_t block_size,
brillo::Blob* hash_out) {
- DirectExtentWriter writer;
- TEST_AND_RETURN_FALSE(writer.Init(target, tgt_extents, block_size));
+ DirectExtentWriter writer{target};
+ TEST_AND_RETURN_FALSE(writer.Init(tgt_extents, block_size));
TEST_AND_RETURN_FALSE(utils::BlocksInExtents(src_extents) ==
utils::BlocksInExtents(tgt_extents));
TEST_AND_RETURN_FALSE(
diff --git a/payload_consumer/partition_writer.cc b/payload_consumer/partition_writer.cc
index 6f06dd2..6f98ba3 100644
--- a/payload_consumer/partition_writer.cc
+++ b/payload_consumer/partition_writer.cc
@@ -326,8 +326,7 @@
writer.reset(new XzExtentWriter(std::move(writer)));
}
- TEST_AND_RETURN_FALSE(
- writer->Init(target_fd_, operation.dst_extents(), block_size_));
+ TEST_AND_RETURN_FALSE(writer->Init(operation.dst_extents(), block_size_));
TEST_AND_RETURN_FALSE(writer->Write(data, operation.data_length()));
return true;
@@ -377,7 +376,7 @@
const auto& partition_control = dynamic_control_;
InstallOperation buf;
- bool should_optimize = partition_control->OptimizeOperation(
+ const bool should_optimize = partition_control->OptimizeOperation(
partition.partition_name(), operation, &buf);
const InstallOperation& optimized = should_optimize ? buf : operation;
@@ -493,8 +492,7 @@
utils::BlocksInExtents(operation.src_extents()) * block_size_);
auto writer = CreateBaseExtentWriter();
- TEST_AND_RETURN_FALSE(
- writer->Init(target_fd_, operation.dst_extents(), block_size_));
+ TEST_AND_RETURN_FALSE(writer->Init(operation.dst_extents(), block_size_));
auto dst_file = std::make_unique<BsdiffExtentFile>(
std::move(writer),
utils::BlocksInExtents(operation.dst_extents()) * block_size_);
@@ -522,8 +520,7 @@
utils::BlocksInExtents(operation.src_extents()) * block_size_));
auto writer = CreateBaseExtentWriter();
- TEST_AND_RETURN_FALSE(
- writer->Init(target_fd_, operation.dst_extents(), block_size_));
+ TEST_AND_RETURN_FALSE(writer->Init(operation.dst_extents(), block_size_));
puffin::UniqueStreamPtr dst_stream(new PuffinExtentStream(
std::move(writer),
utils::BlocksInExtents(operation.dst_extents()) * block_size_));
@@ -658,7 +655,7 @@
}
std::unique_ptr<ExtentWriter> PartitionWriter::CreateBaseExtentWriter() {
- return std::make_unique<DirectExtentWriter>();
+ return std::make_unique<DirectExtentWriter>(target_fd_);
}
} // namespace chromeos_update_engine
diff --git a/payload_consumer/partition_writer_unittest.cc b/payload_consumer/partition_writer_unittest.cc
index 91e5e26..263f338 100644
--- a/payload_consumer/partition_writer_unittest.cc
+++ b/payload_consumer/partition_writer_unittest.cc
@@ -82,10 +82,10 @@
brillo::Blob PerformSourceCopyOp(const InstallOperation& op,
const brillo::Blob blob_data) {
ScopedTempFile source_partition("Blob-XXXXXX");
- DirectExtentWriter extent_writer;
FileDescriptorPtr fd(new EintrSafeFileDescriptor());
+ DirectExtentWriter extent_writer{fd};
EXPECT_TRUE(fd->Open(source_partition.path().c_str(), O_RDWR));
- EXPECT_TRUE(extent_writer.Init(fd, op.src_extents(), kBlockSize));
+ EXPECT_TRUE(extent_writer.Init(op.src_extents(), kBlockSize));
EXPECT_TRUE(extent_writer.Write(blob_data.data(), blob_data.size()));
ScopedTempFile target_partition("Blob-XXXXXX");
diff --git a/payload_consumer/snapshot_extent_writer.cc b/payload_consumer/snapshot_extent_writer.cc
index c9e6f31..242e726 100644
--- a/payload_consumer/snapshot_extent_writer.cc
+++ b/payload_consumer/snapshot_extent_writer.cc
@@ -36,7 +36,6 @@
}
bool SnapshotExtentWriter::Init(
- FileDescriptorPtr /*fd*/,
const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) {
extents_ = extents;
diff --git a/payload_consumer/snapshot_extent_writer.h b/payload_consumer/snapshot_extent_writer.h
index 6d9fe7d..c3c64cd 100644
--- a/payload_consumer/snapshot_extent_writer.h
+++ b/payload_consumer/snapshot_extent_writer.h
@@ -29,8 +29,7 @@
explicit SnapshotExtentWriter(android::snapshot::ICowWriter* cow_writer);
~SnapshotExtentWriter();
// Returns true on success.
- bool Init(FileDescriptorPtr fd,
- const google::protobuf::RepeatedPtrField<Extent>& extents,
+ bool Init(const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override;
// Returns true on success.
// This will construct a COW_REPLACE operation and forward it to CowWriter. It
diff --git a/payload_consumer/snapshot_extent_writer_unittest.cc b/payload_consumer/snapshot_extent_writer_unittest.cc
index 0e22482..2201043 100644
--- a/payload_consumer/snapshot_extent_writer_unittest.cc
+++ b/payload_consumer/snapshot_extent_writer_unittest.cc
@@ -107,7 +107,7 @@
TEST_F(SnapshotExtentWriterTest, BufferWrites) {
google::protobuf::RepeatedPtrField<Extent> extents;
AddExtent(&extents, 123, 1);
- writer_.Init(nullptr, extents, kBlockSize);
+ writer_.Init(extents, kBlockSize);
std::vector<uint8_t> buf(kBlockSize, 0);
buf[123] = 231;
@@ -130,7 +130,7 @@
google::protobuf::RepeatedPtrField<Extent> extents;
AddExtent(&extents, 123, 1);
AddExtent(&extents, 125, 1);
- writer_.Init(nullptr, extents, kBlockSize);
+ writer_.Init(extents, kBlockSize);
std::vector<uint8_t> buf(kBlockSize * 2, 0);
buf[123] = 231;
@@ -153,7 +153,7 @@
google::protobuf::RepeatedPtrField<Extent> extents;
AddExtent(&extents, 123, 1);
AddExtent(&extents, 125, 2);
- writer_.Init(nullptr, extents, kBlockSize);
+ writer_.Init(extents, kBlockSize);
std::vector<uint8_t> buf(kBlockSize * 3);
std::memset(buf.data(), 0, buf.size());
diff --git a/payload_consumer/xz_extent_writer.cc b/payload_consumer/xz_extent_writer.cc
index a5b939d..a648351 100644
--- a/payload_consumer/xz_extent_writer.cc
+++ b/payload_consumer/xz_extent_writer.cc
@@ -57,12 +57,11 @@
TEST_AND_RETURN(input_buffer_.empty());
}
-bool XzExtentWriter::Init(FileDescriptorPtr fd,
- const RepeatedPtrField<Extent>& extents,
+bool XzExtentWriter::Init(const RepeatedPtrField<Extent>& extents,
uint32_t block_size) {
stream_ = xz_dec_init(XZ_DYNALLOC, kXzMaxDictSize);
TEST_AND_RETURN_FALSE(stream_ != nullptr);
- return underlying_writer_->Init(fd, extents, block_size);
+ return underlying_writer_->Init(extents, block_size);
}
bool XzExtentWriter::Write(const void* bytes, size_t count) {
diff --git a/payload_consumer/xz_extent_writer.h b/payload_consumer/xz_extent_writer.h
index e022274..70338f2 100644
--- a/payload_consumer/xz_extent_writer.h
+++ b/payload_consumer/xz_extent_writer.h
@@ -39,8 +39,7 @@
: underlying_writer_(std::move(underlying_writer)) {}
~XzExtentWriter() override;
- bool Init(FileDescriptorPtr fd,
- const google::protobuf::RepeatedPtrField<Extent>& extents,
+ bool Init(const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override;
bool Write(const void* bytes, size_t count) override;
diff --git a/payload_consumer/xz_extent_writer_unittest.cc b/payload_consumer/xz_extent_writer_unittest.cc
index 34980a9..5269dbc 100644
--- a/payload_consumer/xz_extent_writer_unittest.cc
+++ b/payload_consumer/xz_extent_writer_unittest.cc
@@ -87,7 +87,7 @@
}
void WriteAll(const brillo::Blob& compressed) {
- EXPECT_TRUE(xz_writer_->Init(fd_, {}, 1024));
+ EXPECT_TRUE(xz_writer_->Init({}, 1024));
EXPECT_TRUE(xz_writer_->Write(compressed.data(), compressed.size()));
EXPECT_TRUE(fake_extent_writer_->InitCalled());
@@ -130,7 +130,7 @@
}
TEST_F(XzExtentWriterTest, GarbageDataRejected) {
- EXPECT_TRUE(xz_writer_->Init(fd_, {}, 1024));
+ EXPECT_TRUE(xz_writer_->Init({}, 1024));
// The sample_data_ is an uncompressed string.
EXPECT_FALSE(xz_writer_->Write(sample_data_.data(), sample_data_.size()));
}
@@ -138,7 +138,7 @@
TEST_F(XzExtentWriterTest, PartialDataIsKept) {
brillo::Blob compressed(std::begin(kCompressed30KiBofA),
std::end(kCompressed30KiBofA));
- EXPECT_TRUE(xz_writer_->Init(fd_, {}, 1024));
+ EXPECT_TRUE(xz_writer_->Init({}, 1024));
for (uint8_t byte : compressed) {
EXPECT_TRUE(xz_writer_->Write(&byte, 1));
}
diff --git a/payload_generator/zip_unittest.cc b/payload_generator/zip_unittest.cc
index e357b15..10e899b 100644
--- a/payload_generator/zip_unittest.cc
+++ b/payload_generator/zip_unittest.cc
@@ -33,7 +33,6 @@
using chromeos_update_engine::test_utils::kRandomString;
using google::protobuf::RepeatedPtrField;
using std::string;
-using std::vector;
namespace chromeos_update_engine {
@@ -50,8 +49,7 @@
}
~MemoryExtentWriter() override = default;
- bool Init(FileDescriptorPtr fd,
- const RepeatedPtrField<Extent>& extents,
+ bool Init(const RepeatedPtrField<Extent>& extents,
uint32_t block_size) override {
return true;
}
@@ -72,7 +70,7 @@
std::unique_ptr<ExtentWriter> writer(
new W(std::make_unique<MemoryExtentWriter>(out)));
// Init() parameters are ignored by the testing MemoryExtentWriter.
- bool ok = writer->Init(nullptr, {}, 1);
+ bool ok = writer->Init({}, 1);
ok = writer->Write(in.data(), in.size()) && ok;
return ok;
}