Refactor extent writer to take filedescriptor in constructor
Functions which receive an instance of extent writer need to manually
pass fd to ExtentWriter via Init() call, which breaks separation of
concerns. It makes it hard for us to decouple InstallOp execution from
writing of data, as the execution unit must be aware of which fd to pass
to extent writer. In addition, many extents writer, such as snapshot
extent writer, simply ignores the fd parameter, which is a indication of
poor code structure.
To address the above issue, we pass FileDescriptorPtr via constructor if
needed. This way, whoever is "executing" InstallOps don't need to care
about where the output data is going, and whoever's writing the data
would be responsible for initializing an ExtentWriter.
Test: th
Change-Id: I6d1eabde085eefd55da9ecc0352d4a16ae458698
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