update_engine: Fix leaking unit tests
Some of the unit tests have been leaking temp files because they don't
properly unlink them. In this CL, we did some rearrangement of the
ScopedTempFile class and moved it into the utils.h (instead of testing
only location) so it can be used everywhere and more efficiently. Also
added functionality to open an file descriptor too so users don't have
to keep a different object for the file descriptor.
BUG=b:162766400
TEST=cros_workon_make --board reef --test; Then looked at the
/build/reef/tmp directory and no files were leaked.
Change-Id: Id64a2923d30f27628120497fdefe16bf65fa3fb0
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2500772
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/common/hash_calculator_unittest.cc b/common/hash_calculator_unittest.cc
index e8f73d5..fe7d543 100644
--- a/common/hash_calculator_unittest.cc
+++ b/common/hash_calculator_unittest.cc
@@ -104,7 +104,7 @@
}
TEST_F(HashCalculatorTest, UpdateFileSimpleTest) {
- test_utils::ScopedTempFile data_file("data.XXXXXX");
+ ScopedTempFile data_file("data.XXXXXX");
ASSERT_TRUE(test_utils::WriteFileString(data_file.path(), "hi"));
for (const int length : {-1, 2, 10}) {
@@ -126,7 +126,7 @@
}
TEST_F(HashCalculatorTest, RawHashOfFileSimpleTest) {
- test_utils::ScopedTempFile data_file("data.XXXXXX");
+ ScopedTempFile data_file("data.XXXXXX");
ASSERT_TRUE(test_utils::WriteFileString(data_file.path(), "hi"));
for (const int length : {-1, 2, 10}) {
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index 1ead681..99ea99b 100644
--- a/common/http_fetcher_unittest.cc
+++ b/common/http_fetcher_unittest.cc
@@ -369,7 +369,7 @@
HttpServer* CreateServer() override { return new NullHttpServer; }
private:
- test_utils::ScopedTempFile temp_file_{"ue_file_fetcher.XXXXXX"};
+ ScopedTempFile temp_file_{"ue_file_fetcher.XXXXXX"};
};
class MultiRangeHttpFetcherOverFileFetcherTest : public FileFetcherTest {
diff --git a/common/test_utils.h b/common/test_utils.h
index 63ea749..bb5a678 100644
--- a/common/test_utils.h
+++ b/common/test_utils.h
@@ -138,22 +138,6 @@
DISALLOW_COPY_AND_ASSIGN(ScopedLoopbackDeviceBinder);
};
-class ScopedTempFile {
- public:
- ScopedTempFile() : ScopedTempFile("update_engine_test_temp_file.XXXXXX") {}
-
- explicit ScopedTempFile(const std::string& pattern) {
- EXPECT_TRUE(utils::MakeTempFile(pattern, &path_, nullptr));
- unlinker_.reset(new ScopedPathUnlinker(path_));
- }
-
- const std::string& path() const { return path_; }
-
- private:
- std::string path_;
- std::unique_ptr<ScopedPathUnlinker> unlinker_;
-};
-
class ScopedLoopMounter {
public:
explicit ScopedLoopMounter(const std::string& file_path,
diff --git a/common/utils.h b/common/utils.h
index f364bfd..05a92be 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -370,6 +370,42 @@
DISALLOW_COPY_AND_ASSIGN(ScopedPathUnlinker);
};
+class ScopedTempFile {
+ public:
+ ScopedTempFile() : ScopedTempFile("update_engine_temp.XXXXXX") {}
+
+ // If |open_fd| is true, a writable file descriptor will be opened for this
+ // file.
+ explicit ScopedTempFile(const std::string& pattern, bool open_fd = false) {
+ CHECK(utils::MakeTempFile(pattern, &path_, open_fd ? &fd_ : nullptr));
+ unlinker_.reset(new ScopedPathUnlinker(path_));
+ if (open_fd) {
+ CHECK_GE(fd_, 0);
+ fd_closer_.reset(new ScopedFdCloser(&fd_));
+ }
+ }
+ virtual ~ScopedTempFile() = default;
+
+ const std::string& path() const { return path_; }
+ int fd() const {
+ CHECK(fd_closer_);
+ return fd_;
+ }
+ void CloseFd() {
+ CHECK(fd_closer_);
+ fd_closer_.reset();
+ }
+
+ private:
+ std::string path_;
+ std::unique_ptr<ScopedPathUnlinker> unlinker_;
+
+ int fd_{-1};
+ std::unique_ptr<ScopedFdCloser> fd_closer_;
+
+ DISALLOW_COPY_AND_ASSIGN(ScopedTempFile);
+};
+
// A little object to call ActionComplete on the ActionProcessor when
// it's destructed.
class ScopedActionCompleter {
diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index bc5c3b0..20c6b84 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -46,7 +46,7 @@
}
TEST(UtilsTest, WriteFileReadFile) {
- test_utils::ScopedTempFile file;
+ ScopedTempFile file;
EXPECT_TRUE(utils::WriteFile(file.path().c_str(), "hello", 5));
brillo::Blob readback;
@@ -60,7 +60,7 @@
}
TEST(UtilsTest, ReadFileChunk) {
- test_utils::ScopedTempFile file;
+ ScopedTempFile file;
brillo::Blob data;
const size_t kSize = 1024 * 1024;
for (size_t i = 0; i < kSize; i++) {
@@ -149,7 +149,7 @@
namespace {
void GetFileFormatTester(const string& expected,
const vector<uint8_t>& contents) {
- test_utils::ScopedTempFile file;
+ ScopedTempFile file;
ASSERT_TRUE(utils::WriteFile(file.path().c_str(),
reinterpret_cast<const char*>(contents.data()),
contents.size()));
@@ -378,7 +378,7 @@
}
TEST(UtilsTest, RunAsRootUnmountFilesystemBusyFailureTest) {
- test_utils::ScopedTempFile tmp_image("img.XXXXXX");
+ ScopedTempFile tmp_image("img.XXXXXX");
EXPECT_TRUE(base::CopyFile(
test_utils::GetBuildArtifactsPath().Append("gen/disk_ext2_4k.img"),
@@ -418,7 +418,7 @@
EXPECT_TRUE(mnt_dir.CreateUniqueTempDir());
EXPECT_FALSE(utils::IsMountpoint(mnt_dir.GetPath().value()));
- test_utils::ScopedTempFile file;
+ ScopedTempFile file;
EXPECT_FALSE(utils::IsMountpoint(file.path()));
}
@@ -460,7 +460,7 @@
}
TEST(UtilsTest, GetFilePathTest) {
- test_utils::ScopedTempFile file;
+ ScopedTempFile file;
int fd = HANDLE_EINTR(open(file.path().c_str(), O_RDONLY));
EXPECT_GE(fd, 0);
EXPECT_EQ(file.path(), utils::GetFilePath(fd));
diff --git a/dynamic_partition_control_android_unittest.cc b/dynamic_partition_control_android_unittest.cc
index 223e177..c1e0daf 100644
--- a/dynamic_partition_control_android_unittest.cc
+++ b/dynamic_partition_control_android_unittest.cc
@@ -34,7 +34,6 @@
using android::dm::DmDeviceState;
using android::snapshot::MockSnapshotManager;
using chromeos_update_engine::test_utils::ScopedLoopbackDeviceBinder;
-using chromeos_update_engine::test_utils::ScopedTempFile;
using std::string;
using testing::_;
using testing::AnyNumber;
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 4e421b0..9613e8d 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -176,7 +176,7 @@
}
TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
- test_utils::ScopedTempFile test_deadline_file(
+ ScopedTempFile test_deadline_file(
"omaha_response_handler_action_unittest-XXXXXX");
{
OmahaResponse in;
diff --git a/payload_consumer/bzip_extent_writer_unittest.cc b/payload_consumer/bzip_extent_writer_unittest.cc
index 125e1e5..b587040 100644
--- a/payload_consumer/bzip_extent_writer_unittest.cc
+++ b/payload_consumer/bzip_extent_writer_unittest.cc
@@ -49,7 +49,7 @@
void TearDown() override { fd_->Close(); }
FileDescriptorPtr fd_;
- test_utils::ScopedTempFile temp_file_{"BzipExtentWriterTest-file.XXXXXX"};
+ ScopedTempFile temp_file_{"BzipExtentWriterTest-file.XXXXXX"};
};
TEST_F(BzipExtentWriterTest, SimpleTest) {
diff --git a/payload_consumer/cached_file_descriptor_unittest.cc b/payload_consumer/cached_file_descriptor_unittest.cc
index d2965fc..b64420a 100644
--- a/payload_consumer/cached_file_descriptor_unittest.cc
+++ b/payload_consumer/cached_file_descriptor_unittest.cc
@@ -73,7 +73,7 @@
protected:
FileDescriptorPtr fd_{new EintrSafeFileDescriptor};
- test_utils::ScopedTempFile temp_file_{"CachedFileDescriptor-file.XXXXXX"};
+ ScopedTempFile temp_file_{"CachedFileDescriptor-file.XXXXXX"};
int value_{1};
FileDescriptorPtr cfd_;
};
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 6857018..74ddd27 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -52,7 +52,9 @@
namespace chromeos_update_engine {
+using std::list;
using std::string;
+using std::unique_ptr;
using std::vector;
using test_utils::GetBuildArtifactsPath;
using test_utils::kRandomString;
@@ -76,22 +78,24 @@
namespace {
struct DeltaState {
- string a_img;
- string b_img;
- string result_img;
+ unique_ptr<ScopedTempFile> a_img;
+ unique_ptr<ScopedTempFile> b_img;
+ unique_ptr<ScopedTempFile> result_img;
size_t image_size;
- string delta_path;
+ unique_ptr<ScopedTempFile> delta_file;
+ // The in-memory copy of delta file.
+ brillo::Blob delta;
uint64_t metadata_size;
uint32_t metadata_signature_size;
- string old_kernel;
+ unique_ptr<ScopedTempFile> old_kernel;
brillo::Blob old_kernel_data;
- string new_kernel;
+ unique_ptr<ScopedTempFile> new_kernel;
brillo::Blob new_kernel_data;
- string result_kernel;
+ unique_ptr<ScopedTempFile> result_kernel;
brillo::Blob result_kernel_data;
size_t kernel_size;
@@ -99,9 +103,6 @@
// the DeltaPerformer.
InstallPlan install_plan;
- // The in-memory copy of delta file.
- brillo::Blob delta;
-
// Mock and fake instances used by the delta performer.
FakeBootControl fake_boot_control_;
FakeHardware fake_hardware_;
@@ -155,7 +156,7 @@
EXPECT_EQ(expected, performer.ValidateManifest());
}
void AddPartition(DeltaArchiveManifest* manifest,
- std::string name,
+ string name,
int timestamp) {
auto& partition = *manifest->add_partitions();
partition.set_version(std::to_string(timestamp));
@@ -259,8 +260,7 @@
}
string signature_size_string = base::JoinString(signature_size_strings, ":");
- test_utils::ScopedTempFile hash_file("hash.XXXXXX"),
- metadata_hash_file("hash.XXXXXX");
+ ScopedTempFile hash_file("hash.XXXXXX"), metadata_hash_file("hash.XXXXXX");
string delta_generator_path = GetBuildArtifactsPath("delta_generator");
ASSERT_EQ(0,
System(base::StringPrintf(
@@ -273,29 +273,27 @@
metadata_hash_file.path().c_str())));
// Sign the hash with all private keys.
- vector<test_utils::ScopedTempFile> sig_files, metadata_sig_files;
+ list<ScopedTempFile> sig_files, metadata_sig_files;
vector<string> sig_file_paths, metadata_sig_file_paths;
for (const auto& key_path : private_key_paths) {
brillo::Blob hash, signature;
ASSERT_TRUE(utils::ReadFile(hash_file.path(), &hash));
ASSERT_TRUE(PayloadSigner::SignHash(hash, key_path, &signature));
- test_utils::ScopedTempFile sig_file("signature.XXXXXX");
- ASSERT_TRUE(test_utils::WriteFileVector(sig_file.path(), signature));
- sig_file_paths.push_back(sig_file.path());
- sig_files.push_back(std::move(sig_file));
+ sig_files.emplace_back("signature.XXXXXX");
+ ASSERT_TRUE(
+ test_utils::WriteFileVector(sig_files.back().path(), signature));
+ sig_file_paths.push_back(sig_files.back().path());
brillo::Blob metadata_hash, metadata_signature;
ASSERT_TRUE(utils::ReadFile(metadata_hash_file.path(), &metadata_hash));
ASSERT_TRUE(
PayloadSigner::SignHash(metadata_hash, key_path, &metadata_signature));
- test_utils::ScopedTempFile metadata_sig_file("signature.XXXXXX");
- ASSERT_TRUE(test_utils::WriteFileVector(metadata_sig_file.path(),
+ metadata_sig_files.emplace_back("metadata_signature.XXXXXX");
+ ASSERT_TRUE(test_utils::WriteFileVector(metadata_sig_files.back().path(),
metadata_signature));
-
- metadata_sig_file_paths.push_back(metadata_sig_file.path());
- metadata_sig_files.push_back(std::move(metadata_sig_file));
+ metadata_sig_file_paths.push_back(metadata_sig_files.back().path());
}
string sig_files_string = base::JoinString(sig_file_paths, ":");
string metadata_sig_files_string =
@@ -377,7 +375,7 @@
GetBuildArtifactsPath(kUnittestPrivateKey2Path));
}
- std::string public_key;
+ string public_key;
if (signature_test == kSignatureGeneratedShellRotateCl2) {
public_key = GetBuildArtifactsPath(kUnittestPublicKey2Path);
} else if (signature_test == kSignatureGeneratedShellECKey) {
@@ -397,24 +395,23 @@
SignatureTest signature_test,
DeltaState* state,
uint32_t minor_version) {
- EXPECT_TRUE(utils::MakeTempFile("a_img.XXXXXX", &state->a_img, nullptr));
- EXPECT_TRUE(utils::MakeTempFile("b_img.XXXXXX", &state->b_img, nullptr));
+ state->a_img.reset(new ScopedTempFile("a_img.XXXXXX"));
+ state->b_img.reset(new ScopedTempFile("b_img.XXXXXX"));
// result_img is used in minor version 2. Instead of applying the update
// in-place on A, we apply it to a new image, result_img.
- EXPECT_TRUE(
- utils::MakeTempFile("result_img.XXXXXX", &state->result_img, nullptr));
+ state->result_img.reset(new ScopedTempFile("result_img.XXXXXX"));
EXPECT_TRUE(
base::CopyFile(GetBuildArtifactsPath().Append("gen/disk_ext2_4k.img"),
- base::FilePath(state->a_img)));
+ base::FilePath(state->a_img->path())));
- state->image_size = utils::FileSize(state->a_img);
+ state->image_size = utils::FileSize(state->a_img->path());
// Make some changes to the A image.
{
string a_mnt;
- ScopedLoopMounter b_mounter(state->a_img, &a_mnt, 0);
+ ScopedLoopMounter b_mounter(state->a_img->path(), &a_mnt, 0);
brillo::Blob hardtocompress;
while (hardtocompress.size() < 3 * kBlockSize) {
@@ -451,17 +448,18 @@
// Create a result image with image_size bytes of garbage.
brillo::Blob ones(state->image_size, 0xff);
- EXPECT_TRUE(
- utils::WriteFile(state->result_img.c_str(), ones.data(), ones.size()));
- EXPECT_EQ(utils::FileSize(state->a_img), utils::FileSize(state->result_img));
+ EXPECT_TRUE(utils::WriteFile(
+ state->result_img->path().c_str(), ones.data(), ones.size()));
+ EXPECT_EQ(utils::FileSize(state->a_img->path()),
+ utils::FileSize(state->result_img->path()));
EXPECT_TRUE(
base::CopyFile(GetBuildArtifactsPath().Append("gen/disk_ext2_4k.img"),
- base::FilePath(state->b_img)));
+ base::FilePath(state->b_img->path())));
{
// Make some changes to the B image.
string b_mnt;
- ScopedLoopMounter b_mounter(state->b_img, &b_mnt, 0);
+ ScopedLoopMounter b_mounter(state->b_img->path(), &b_mnt, 0);
base::FilePath mnt_path(b_mnt);
EXPECT_TRUE(base::CopyFile(mnt_path.Append("regular-small"),
@@ -509,18 +507,9 @@
hardtocompress.size()));
}
- string old_kernel;
- EXPECT_TRUE(
- utils::MakeTempFile("old_kernel.XXXXXX", &state->old_kernel, nullptr));
-
- string new_kernel;
- EXPECT_TRUE(
- utils::MakeTempFile("new_kernel.XXXXXX", &state->new_kernel, nullptr));
-
- string result_kernel;
- EXPECT_TRUE(utils::MakeTempFile(
- "result_kernel.XXXXXX", &state->result_kernel, nullptr));
-
+ state->old_kernel.reset(new ScopedTempFile("old_kernel.XXXXXX"));
+ state->new_kernel.reset(new ScopedTempFile("new_kernel.XXXXXX"));
+ state->result_kernel.reset(new ScopedTempFile("result_kernel.XXXXXX"));
state->kernel_size = kDefaultKernelSize;
state->old_kernel_data.resize(kDefaultKernelSize);
state->new_kernel_data.resize(state->old_kernel_data.size());
@@ -534,18 +523,17 @@
std::begin(kNewData), std::end(kNewData), state->new_kernel_data.begin());
// Write kernels to disk
- EXPECT_TRUE(utils::WriteFile(state->old_kernel.c_str(),
+ EXPECT_TRUE(utils::WriteFile(state->old_kernel->path().c_str(),
state->old_kernel_data.data(),
state->old_kernel_data.size()));
- EXPECT_TRUE(utils::WriteFile(state->new_kernel.c_str(),
+ EXPECT_TRUE(utils::WriteFile(state->new_kernel->path().c_str(),
state->new_kernel_data.data(),
state->new_kernel_data.size()));
- EXPECT_TRUE(utils::WriteFile(state->result_kernel.c_str(),
+ EXPECT_TRUE(utils::WriteFile(state->result_kernel->path().c_str(),
state->result_kernel_data.data(),
state->result_kernel_data.size()));
- EXPECT_TRUE(utils::MakeTempFile("delta.XXXXXX", &state->delta_path, nullptr));
- LOG(INFO) << "delta path: " << state->delta_path;
+ state->delta_file.reset(new ScopedTempFile("delta.XXXXXX"));
{
const string private_key =
signature_test == kSignatureGenerator
@@ -561,9 +549,10 @@
if (!full_rootfs) {
payload_config.source.partitions.emplace_back(kPartitionNameRoot);
payload_config.source.partitions.emplace_back(kPartitionNameKernel);
- payload_config.source.partitions.front().path = state->a_img;
+ payload_config.source.partitions.front().path = state->a_img->path();
if (!full_kernel)
- payload_config.source.partitions.back().path = state->old_kernel;
+ payload_config.source.partitions.back().path =
+ state->old_kernel->path();
EXPECT_TRUE(payload_config.source.LoadImageSize());
for (PartitionConfig& part : payload_config.source.partitions)
EXPECT_TRUE(part.OpenFilesystem());
@@ -573,28 +562,30 @@
payload_config.hard_chunk_size = 1024 * 1024;
}
payload_config.target.partitions.emplace_back(kPartitionNameRoot);
- payload_config.target.partitions.back().path = state->b_img;
+ payload_config.target.partitions.back().path = state->b_img->path();
payload_config.target.partitions.emplace_back(kPartitionNameKernel);
- payload_config.target.partitions.back().path = state->new_kernel;
+ payload_config.target.partitions.back().path = state->new_kernel->path();
EXPECT_TRUE(payload_config.target.LoadImageSize());
for (PartitionConfig& part : payload_config.target.partitions)
EXPECT_TRUE(part.OpenFilesystem());
EXPECT_TRUE(payload_config.Validate());
- EXPECT_TRUE(GenerateUpdatePayloadFile(
- payload_config, state->delta_path, private_key, &state->metadata_size));
+ EXPECT_TRUE(GenerateUpdatePayloadFile(payload_config,
+ state->delta_file->path(),
+ private_key,
+ &state->metadata_size));
}
// Extend the "partitions" holding the file system a bit.
EXPECT_EQ(0,
- HANDLE_EINTR(truncate(state->a_img.c_str(),
+ HANDLE_EINTR(truncate(state->a_img->path().c_str(),
state->image_size + 1024 * 1024)));
EXPECT_EQ(static_cast<off_t>(state->image_size + 1024 * 1024),
- utils::FileSize(state->a_img));
+ utils::FileSize(state->a_img->path()));
EXPECT_EQ(0,
- HANDLE_EINTR(truncate(state->b_img.c_str(),
+ HANDLE_EINTR(truncate(state->b_img->path().c_str(),
state->image_size + 1024 * 1024)));
EXPECT_EQ(static_cast<off_t>(state->image_size + 1024 * 1024),
- utils::FileSize(state->b_img));
+ utils::FileSize(state->b_img->path()));
if (signature_test == kSignatureGeneratedPlaceholder ||
signature_test == kSignatureGeneratedPlaceholderMismatch) {
@@ -603,13 +594,13 @@
GetBuildArtifactsPath(kUnittestPrivateKeyPath), &signature_size));
LOG(INFO) << "Inserting placeholder signature.";
ASSERT_TRUE(InsertSignaturePlaceholder(
- signature_size, state->delta_path, &state->metadata_size));
+ signature_size, state->delta_file->path(), &state->metadata_size));
if (signature_test == kSignatureGeneratedPlaceholderMismatch) {
signature_size -= 1;
LOG(INFO) << "Inserting mismatched placeholder signature.";
ASSERT_FALSE(InsertSignaturePlaceholder(
- signature_size, state->delta_path, &state->metadata_size));
+ signature_size, state->delta_file->path(), &state->metadata_size));
return;
}
}
@@ -621,13 +612,13 @@
// reflect the new size after adding the signature operation to the
// manifest.
LOG(INFO) << "Signing payload.";
- SignGeneratedPayload(state->delta_path, &state->metadata_size);
+ SignGeneratedPayload(state->delta_file->path(), &state->metadata_size);
} else if (signature_test == kSignatureGeneratedShell ||
signature_test == kSignatureGeneratedShellECKey ||
signature_test == kSignatureGeneratedShellBadKey ||
signature_test == kSignatureGeneratedShellRotateCl1 ||
signature_test == kSignatureGeneratedShellRotateCl2) {
- SignGeneratedShellPayload(signature_test, state->delta_path);
+ SignGeneratedShellPayload(signature_test, state->delta_file->path());
}
}
@@ -641,7 +632,7 @@
uint32_t minor_version) {
// Check the metadata.
{
- EXPECT_TRUE(utils::ReadFile(state->delta_path, &state->delta));
+ EXPECT_TRUE(utils::ReadFile(state->delta_file->path(), &state->delta));
PayloadMetadata payload_metadata;
EXPECT_TRUE(payload_metadata.ParsePayloadHeader(state->delta));
state->metadata_size = payload_metadata.GetMetadataSize();
@@ -794,9 +785,10 @@
(*performer)->set_public_key_path(public_key_path);
(*performer)->set_update_certificates_path("");
- EXPECT_EQ(static_cast<off_t>(state->image_size),
- HashCalculator::RawHashOfFile(
- state->a_img, state->image_size, &root_part.source_hash));
+ EXPECT_EQ(
+ static_cast<off_t>(state->image_size),
+ HashCalculator::RawHashOfFile(
+ state->a_img->path(), state->image_size, &root_part.source_hash));
EXPECT_TRUE(HashCalculator::RawHashOfData(state->old_kernel_data,
&kernel_part.source_hash));
@@ -804,13 +796,15 @@
install_plan->partitions.clear();
state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameRoot, install_plan->source_slot, state->a_img);
+ kPartitionNameRoot, install_plan->source_slot, state->a_img->path());
+ state->fake_boot_control_.SetPartitionDevice(kPartitionNameKernel,
+ install_plan->source_slot,
+ state->old_kernel->path());
state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameKernel, install_plan->source_slot, state->old_kernel);
- state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameRoot, install_plan->target_slot, state->result_img);
- state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameKernel, install_plan->target_slot, state->result_kernel);
+ kPartitionNameRoot, install_plan->target_slot, state->result_img->path());
+ state->fake_boot_control_.SetPartitionDevice(kPartitionNameKernel,
+ install_plan->target_slot,
+ state->result_kernel->path());
ErrorCode expected_error, actual_error;
bool continue_writing;
@@ -889,12 +883,15 @@
return;
}
+ CompareFilesByBlock(state->result_kernel->path(),
+ state->new_kernel->path(),
+ state->kernel_size);
CompareFilesByBlock(
- state->result_kernel, state->new_kernel, state->kernel_size);
- CompareFilesByBlock(state->result_img, state->b_img, state->image_size);
+ state->result_img->path(), state->b_img->path(), state->image_size);
brillo::Blob updated_kernel_partition;
- EXPECT_TRUE(utils::ReadFile(state->result_kernel, &updated_kernel_partition));
+ EXPECT_TRUE(
+ utils::ReadFile(state->result_kernel->path(), &updated_kernel_partition));
ASSERT_GE(updated_kernel_partition.size(), base::size(kNewData));
EXPECT_TRUE(std::equal(std::begin(kNewData),
std::end(kNewData),
@@ -913,9 +910,10 @@
EXPECT_EQ(state->image_size, partitions[0].target_size);
brillo::Blob expected_new_rootfs_hash;
- EXPECT_EQ(static_cast<off_t>(state->image_size),
- HashCalculator::RawHashOfFile(
- state->b_img, state->image_size, &expected_new_rootfs_hash));
+ EXPECT_EQ(
+ static_cast<off_t>(state->image_size),
+ HashCalculator::RawHashOfFile(
+ state->b_img->path(), state->image_size, &expected_new_rootfs_hash));
EXPECT_EQ(expected_new_rootfs_hash, partitions[0].target_hash);
}
@@ -953,13 +951,6 @@
&state,
minor_version);
- ScopedPathUnlinker a_img_unlinker(state.a_img);
- ScopedPathUnlinker b_img_unlinker(state.b_img);
- ScopedPathUnlinker new_img_unlinker(state.result_img);
- ScopedPathUnlinker delta_unlinker(state.delta_path);
- ScopedPathUnlinker old_kernel_unlinker(state.old_kernel);
- ScopedPathUnlinker new_kernel_unlinker(state.new_kernel);
- ScopedPathUnlinker result_kernel_unlinker(state.result_kernel);
ApplyDeltaFile(full_kernel,
full_rootfs,
signature_test,
@@ -977,11 +968,6 @@
DeltaState state;
uint64_t minor_version = kFullPayloadMinorVersion;
GenerateDeltaFile(true, true, -1, kSignatureGenerated, &state, minor_version);
- ScopedPathUnlinker a_img_unlinker(state.a_img);
- ScopedPathUnlinker b_img_unlinker(state.b_img);
- ScopedPathUnlinker delta_unlinker(state.delta_path);
- ScopedPathUnlinker old_kernel_unlinker(state.old_kernel);
- ScopedPathUnlinker new_kernel_unlinker(state.new_kernel);
DeltaPerformer* performer = nullptr;
ApplyDeltaFile(true,
true,
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 449201c..65b9dac 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -202,7 +202,7 @@
uint64_t major_version,
uint32_t minor_version,
PartitionConfig* old_part = nullptr) {
- test_utils::ScopedTempFile blob_file("Blob-XXXXXX");
+ ScopedTempFile blob_file("Blob-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(blob_file.path(), blob_data));
PayloadGenerationConfig config;
@@ -236,7 +236,7 @@
new_part.size = 0;
payload.AddPartition(*old_part, new_part, {}, {});
- test_utils::ScopedTempFile payload_file("Payload-XXXXXX");
+ ScopedTempFile payload_file("Payload-XXXXXX");
string private_key =
sign_payload ? GetBuildArtifactsPath(kUnittestPrivateKeyPath) : "";
EXPECT_TRUE(payload.WritePayload(payload_file.path(),
@@ -287,7 +287,7 @@
const string& source_path,
const brillo::Blob& target_data,
bool expect_success) {
- test_utils::ScopedTempFile new_part("Partition-XXXXXX");
+ ScopedTempFile new_part("Partition-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(new_part.path(), target_data));
payload_.size = payload_data.size();
@@ -591,7 +591,7 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(expected_data, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), expected_data));
PartitionConfig old_part(kPartitionNameRoot);
@@ -619,7 +619,7 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(src, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), src));
PartitionConfig old_part(kPartitionNameRoot);
@@ -647,7 +647,7 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(expected_data, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), actual_data));
PartitionConfig old_part(kPartitionNameRoot);
@@ -664,7 +664,7 @@
// since the source partition doesn't match the operation hash.
TEST_F(DeltaPerformerTest, ErrorCorrectionSourceCopyFallbackTest) {
constexpr size_t kCopyOperationSize = 4 * 4096;
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
// Write invalid data to the source image, which doesn't match the expected
// hash.
brillo::Blob invalid_data(kCopyOperationSize, 0x55);
@@ -692,7 +692,7 @@
// file descriptor when the size of the error corrected one is too small.
TEST_F(DeltaPerformerTest, ErrorCorrectionSourceCopyWhenNoHashFallbackTest) {
constexpr size_t kCopyOperationSize = 4 * 4096;
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
// Setup the source path with the right expected data.
brillo::Blob expected_data = FakeFileDescriptorData(kCopyOperationSize);
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), expected_data));
@@ -720,7 +720,7 @@
TEST_F(DeltaPerformerTest, ChooseSourceFDTest) {
constexpr size_t kSourceSize = 4 * 4096;
- test_utils::ScopedTempFile source("Source-XXXXXX");
+ ScopedTempFile source("Source-XXXXXX");
// Write invalid data to the source image, which doesn't match the expected
// hash.
brillo::Blob invalid_data(kSourceSize, 0x55);
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index e6ca219..9daa791 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -51,7 +51,6 @@
using base::WriteFile;
using std::string;
using std::unique_ptr;
-using test_utils::ScopedTempFile;
using testing::_;
using testing::AtLeast;
using testing::InSequence;
@@ -133,7 +132,6 @@
loop.SetAsCurrent();
FakeSystemState fake_system_state;
- // TODO(adlr): see if we need a different file for build bots
ScopedTempFile output_temp_file;
TestDirectFileWriter writer;
EXPECT_EQ(
diff --git a/payload_consumer/extent_reader_unittest.cc b/payload_consumer/extent_reader_unittest.cc
index b7059bc..686f14d 100644
--- a/payload_consumer/extent_reader_unittest.cc
+++ b/payload_consumer/extent_reader_unittest.cc
@@ -72,7 +72,7 @@
}
FileDescriptorPtr fd_;
- test_utils::ScopedTempFile temp_file_{"ExtentReaderTest-file.XXXXXX"};
+ ScopedTempFile temp_file_{"ExtentReaderTest-file.XXXXXX"};
brillo::Blob sample_;
};
diff --git a/payload_consumer/extent_writer_unittest.cc b/payload_consumer/extent_writer_unittest.cc
index aef856b..afebb1a 100644
--- a/payload_consumer/extent_writer_unittest.cc
+++ b/payload_consumer/extent_writer_unittest.cc
@@ -59,7 +59,7 @@
void WriteAlignedExtents(size_t chunk_size, size_t first_chunk_size);
FileDescriptorPtr fd_;
- test_utils::ScopedTempFile temp_file_{"ExtentWriterTest-file.XXXXXX"};
+ ScopedTempFile temp_file_{"ExtentWriterTest-file.XXXXXX"};
};
TEST_F(ExtentWriterTest, SimpleTest) {
diff --git a/payload_consumer/file_descriptor_utils_unittest.cc b/payload_consumer/file_descriptor_utils_unittest.cc
index 48e610f..478893d 100644
--- a/payload_consumer/file_descriptor_utils_unittest.cc
+++ b/payload_consumer/file_descriptor_utils_unittest.cc
@@ -52,14 +52,13 @@
class FileDescriptorUtilsTest : public ::testing::Test {
protected:
void SetUp() override {
- EXPECT_TRUE(utils::MakeTempFile("fd_tgt.XXXXXX", &tgt_path_, nullptr));
- EXPECT_TRUE(target_->Open(tgt_path_.c_str(), O_RDWR));
+ EXPECT_TRUE(target_->Open(tgt_file_.path().c_str(), O_RDWR));
}
// Check that the |target_| file contains |expected_contents|.
void ExpectTarget(const std::string& expected_contents) {
std::string target_contents;
- EXPECT_TRUE(utils::ReadFile(tgt_path_, &target_contents));
+ EXPECT_TRUE(utils::ReadFile(tgt_file_.path(), &target_contents));
EXPECT_EQ(expected_contents.size(), target_contents.size());
if (target_contents != expected_contents) {
ADD_FAILURE() << "Contents don't match.";
@@ -70,8 +69,7 @@
}
}
- // Path to the target temporary file.
- std::string tgt_path_;
+ ScopedTempFile tgt_file_{"fd_tgt.XXXXXX"};
// Source and target file descriptor used for testing the tools.
FakeFileDescriptor* fake_source_{new FakeFileDescriptor()};
diff --git a/payload_consumer/file_writer_unittest.cc b/payload_consumer/file_writer_unittest.cc
index 59cfe2b..3b959f3 100644
--- a/payload_consumer/file_writer_unittest.cc
+++ b/payload_consumer/file_writer_unittest.cc
@@ -35,8 +35,7 @@
class FileWriterTest : public ::testing::Test {};
TEST(FileWriterTest, SimpleTest) {
- // Create a uniquely named file for testing.
- test_utils::ScopedTempFile file("FileWriterTest-XXXXXX");
+ ScopedTempFile file("FileWriterTest-XXXXXX");
DirectFileWriter file_writer;
EXPECT_EQ(0,
file_writer.Open(file.path().c_str(),
@@ -60,7 +59,7 @@
TEST(FileWriterTest, WriteErrorTest) {
// Create a uniquely named file for testing.
- test_utils::ScopedTempFile file("FileWriterTest-XXXXXX");
+ ScopedTempFile file("FileWriterTest-XXXXXX");
DirectFileWriter file_writer;
EXPECT_EQ(0,
file_writer.Open(file.path().c_str(),
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index 2971849..2c29b44 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -92,7 +92,7 @@
bool FilesystemVerifierActionTest::DoTest(bool terminate_early,
bool hash_fail) {
- test_utils::ScopedTempFile a_loop_file("a_loop_file.XXXXXX");
+ ScopedTempFile a_loop_file("a_loop_file.XXXXXX");
// Make random data for a.
const size_t kLoopFileSize = 10 * 1024 * 1024 + 512;
@@ -278,7 +278,7 @@
#ifdef __ANDROID__
TEST_F(FilesystemVerifierActionTest, RunAsRootWriteVerityTest) {
- test_utils::ScopedTempFile part_file("part_file.XXXXXX");
+ ScopedTempFile part_file("part_file.XXXXXX");
constexpr size_t filesystem_size = 200 * 4096;
constexpr size_t part_size = 256 * 4096;
brillo::Blob part_data(filesystem_size, 0x1);
@@ -340,7 +340,7 @@
#endif // __ANDROID__
TEST_F(FilesystemVerifierActionTest, RunAsRootSkipWriteVerityTest) {
- test_utils::ScopedTempFile part_file("part_file.XXXXXX");
+ ScopedTempFile part_file("part_file.XXXXXX");
constexpr size_t filesystem_size = 200 * 4096;
constexpr size_t part_size = 256 * 4096;
brillo::Blob part_data(part_size);
diff --git a/payload_consumer/verity_writer_android_unittest.cc b/payload_consumer/verity_writer_android_unittest.cc
index f943ce8..ec22ffb 100644
--- a/payload_consumer/verity_writer_android_unittest.cc
+++ b/payload_consumer/verity_writer_android_unittest.cc
@@ -39,7 +39,7 @@
VerityWriterAndroid verity_writer_;
InstallPlan::Partition partition_;
- test_utils::ScopedTempFile temp_file_;
+ ScopedTempFile temp_file_;
};
TEST_F(VerityWriterAndroidTest, SimpleTest) {
diff --git a/payload_generator/ab_generator_unittest.cc b/payload_generator/ab_generator_unittest.cc
index 7a95284..84eeb77 100644
--- a/payload_generator/ab_generator_unittest.cc
+++ b/payload_generator/ab_generator_unittest.cc
@@ -70,8 +70,7 @@
part_data.push_back(dis(gen));
}
ASSERT_EQ(part_size, part_data.size());
- test_utils::ScopedTempFile part_file(
- "SplitReplaceOrReplaceXzTest_part.XXXXXX");
+ ScopedTempFile part_file("SplitReplaceOrReplaceXzTest_part.XXXXXX");
ASSERT_TRUE(test_utils::WriteFileVector(part_file.path(), part_data));
// Create original operation and blob data.
@@ -107,8 +106,7 @@
aop.name = "SplitTestOp";
// Create the data file.
- test_utils::ScopedTempFile data_file(
- "SplitReplaceOrReplaceXzTest_data.XXXXXX");
+ ScopedTempFile data_file("SplitReplaceOrReplaceXzTest_data.XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(data_file.path(), op_blob));
int data_fd = open(data_file.path().c_str(), O_RDWR, 000);
EXPECT_GE(data_fd, 0);
@@ -220,8 +218,7 @@
part_data.push_back(dis(gen));
}
ASSERT_EQ(part_size, part_data.size());
- test_utils::ScopedTempFile part_file(
- "MergeReplaceOrReplaceXzTest_part.XXXXXX");
+ ScopedTempFile part_file("MergeReplaceOrReplaceXzTest_part.XXXXXX");
ASSERT_TRUE(test_utils::WriteFileVector(part_file.path(), part_data));
// Create original operations and blob data.
@@ -271,8 +268,7 @@
aops.push_back(second_aop);
// Create the data file.
- test_utils::ScopedTempFile data_file(
- "MergeReplaceOrReplaceXzTest_data.XXXXXX");
+ ScopedTempFile data_file("MergeReplaceOrReplaceXzTest_data.XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(data_file.path(), blob_data));
int data_fd = open(data_file.path().c_str(), O_RDWR, 000);
EXPECT_GE(data_fd, 0);
@@ -561,7 +557,7 @@
second_aop.op = second_op;
aops.push_back(second_aop);
- test_utils::ScopedTempFile src_part_file("AddSourceHashTest_src_part.XXXXXX");
+ ScopedTempFile src_part_file("AddSourceHashTest_src_part.XXXXXX");
brillo::Blob src_data(kBlockSize);
test_utils::FillWithData(&src_data);
ASSERT_TRUE(test_utils::WriteFileVector(src_part_file.path(), src_data));
diff --git a/payload_generator/blob_file_writer_unittest.cc b/payload_generator/blob_file_writer_unittest.cc
index 487bc73..f4dcafb 100644
--- a/payload_generator/blob_file_writer_unittest.cc
+++ b/payload_generator/blob_file_writer_unittest.cc
@@ -31,24 +31,21 @@
class BlobFileWriterTest : public ::testing::Test {};
TEST(BlobFileWriterTest, SimpleTest) {
- string blob_path;
- int blob_fd;
- EXPECT_TRUE(
- utils::MakeTempFile("BlobFileWriterTest.XXXXXX", &blob_path, &blob_fd));
+ ScopedTempFile blob_file("BlobFileWriterTest.XXXXXX", true);
off_t blob_file_size = 0;
- BlobFileWriter blob_file(blob_fd, &blob_file_size);
+ BlobFileWriter blob_file_writer(blob_file.fd(), &blob_file_size);
- off_t blob_size = 1024;
- brillo::Blob blob(blob_size);
+ const off_t kBlobSize = 1024;
+ brillo::Blob blob(kBlobSize);
FillWithData(&blob);
- EXPECT_EQ(0, blob_file.StoreBlob(blob));
- EXPECT_EQ(blob_size, blob_file.StoreBlob(blob));
+ EXPECT_EQ(0, blob_file_writer.StoreBlob(blob));
+ EXPECT_EQ(kBlobSize, blob_file_writer.StoreBlob(blob));
- brillo::Blob stored_blob(blob_size);
+ brillo::Blob stored_blob(kBlobSize);
ssize_t bytes_read;
- ASSERT_TRUE(
- utils::PReadAll(blob_fd, stored_blob.data(), blob_size, 0, &bytes_read));
- EXPECT_EQ(bytes_read, blob_size);
+ ASSERT_TRUE(utils::PReadAll(
+ blob_file.fd(), stored_blob.data(), kBlobSize, 0, &bytes_read));
+ EXPECT_EQ(bytes_read, kBlobSize);
EXPECT_EQ(blob, stored_blob);
}
diff --git a/payload_generator/block_mapping_unittest.cc b/payload_generator/block_mapping_unittest.cc
index 9b9b4f1..017548a 100644
--- a/payload_generator/block_mapping_unittest.cc
+++ b/payload_generator/block_mapping_unittest.cc
@@ -36,8 +36,8 @@
class BlockMappingTest : public ::testing::Test {
protected:
// Old new partition files used in testing.
- test_utils::ScopedTempFile old_part_{"BlockMappingTest_old.XXXXXX"};
- test_utils::ScopedTempFile new_part_{"BlockMappingTest_new.XXXXXX"};
+ ScopedTempFile old_part_{"BlockMappingTest_old.XXXXXX"};
+ ScopedTempFile new_part_{"BlockMappingTest_new.XXXXXX"};
size_t block_size_{1024};
BlockMapping bm_{block_size_}; // BlockMapping under test.
diff --git a/payload_generator/boot_img_filesystem_unittest.cc b/payload_generator/boot_img_filesystem_unittest.cc
index 0b115e0..7805156 100644
--- a/payload_generator/boot_img_filesystem_unittest.cc
+++ b/payload_generator/boot_img_filesystem_unittest.cc
@@ -63,7 +63,7 @@
return boot_img;
}
- test_utils::ScopedTempFile boot_file_;
+ ScopedTempFile boot_file_;
};
TEST_F(BootImgFilesystemTest, SimpleTest) {
diff --git a/payload_generator/delta_diff_generator.cc b/payload_generator/delta_diff_generator.cc
index c2b35ee..ff8b0da 100644
--- a/payload_generator/delta_diff_generator.cc
+++ b/payload_generator/delta_diff_generator.cc
@@ -119,18 +119,10 @@
PayloadFile payload;
TEST_AND_RETURN_FALSE(payload.Init(config));
- const string kTempFileTemplate("CrAU_temp_data.XXXXXX");
- string temp_file_path;
- int data_file_fd;
- TEST_AND_RETURN_FALSE(
- utils::MakeTempFile(kTempFileTemplate, &temp_file_path, &data_file_fd));
- ScopedPathUnlinker temp_file_unlinker(temp_file_path);
- TEST_AND_RETURN_FALSE(data_file_fd >= 0);
-
+ ScopedTempFile data_file("CrAU_temp_data.XXXXXX", true);
{
off_t data_file_size = 0;
- ScopedFdCloser data_file_fd_closer(&data_file_fd);
- BlobFileWriter blob_file(data_file_fd, &data_file_size);
+ BlobFileWriter blob_file(data_file.fd(), &data_file_size);
if (config.is_delta) {
TEST_AND_RETURN_FALSE(config.source.partitions.size() ==
config.target.partitions.size());
@@ -190,11 +182,12 @@
std::move(all_merge_sequences[i])));
}
}
+ data_file.CloseFd();
LOG(INFO) << "Writing payload file...";
// Write payload file to disk.
TEST_AND_RETURN_FALSE(payload.WritePayload(
- output_path, temp_file_path, private_key_path, metadata_size));
+ output_path, data_file.path(), private_key_path, metadata_size));
LOG(INFO) << "All done. Successfully created delta file with "
<< "metadata size = " << *metadata_size;
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 220c7ae..3c025e1 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -822,17 +822,13 @@
// Only Puffdiff if both files have at least one deflate left.
if (!src_deflates.empty() && !dst_deflates.empty()) {
brillo::Blob puffdiff_delta;
- string temp_file_path;
- TEST_AND_RETURN_FALSE(utils::MakeTempFile(
- "puffdiff-delta.XXXXXX", &temp_file_path, nullptr));
- ScopedPathUnlinker temp_file_unlinker(temp_file_path);
-
+ ScopedTempFile temp_file("puffdiff-delta.XXXXXX");
// Perform PuffDiff operation.
TEST_AND_RETURN_FALSE(puffin::PuffDiff(old_data,
new_data,
src_deflates,
dst_deflates,
- temp_file_path,
+ temp_file.path(),
&puffdiff_delta));
TEST_AND_RETURN_FALSE(puffdiff_delta.size() > 0);
if (IsDiffOperationBetter(operation,
diff --git a/payload_generator/delta_diff_utils_unittest.cc b/payload_generator/delta_diff_utils_unittest.cc
index 0857f9c..f2db1bd 100644
--- a/payload_generator/delta_diff_utils_unittest.cc
+++ b/payload_generator/delta_diff_utils_unittest.cc
@@ -69,13 +69,12 @@
// Create a fake filesystem of the given |size| and initialize the partition
// holding it in the PartitionConfig |part|.
void CreatePartition(PartitionConfig* part,
- const string& pattern,
+ ScopedTempFile* part_file,
uint64_t block_size,
off_t size) {
- int fd = -1;
- ASSERT_TRUE(utils::MakeTempFile(pattern.c_str(), &part->path, &fd));
- ASSERT_EQ(0, ftruncate(fd, size));
- ASSERT_EQ(0, close(fd));
+ part->path = part_file->path();
+ ASSERT_EQ(0, ftruncate(part_file->fd(), size));
+ part_file->CloseFd();
part->fs_interface.reset(new FakeFilesystem(block_size, size / block_size));
part->size = size;
}
@@ -112,30 +111,20 @@
void SetUp() override {
CreatePartition(&old_part_,
- "DeltaDiffUtilsTest-old_part-XXXXXX",
+ &old_part_file_,
block_size_,
block_size_ * kDefaultBlockCount);
CreatePartition(&new_part_,
- "DeltaDiffUtilsTest-old_part-XXXXXX",
+ &new_part_file_,
block_size_,
block_size_ * kDefaultBlockCount);
- ASSERT_TRUE(utils::MakeTempFile(
- "DeltaDiffUtilsTest-blob-XXXXXX", &blob_path_, &blob_fd_));
- }
-
- void TearDown() override {
- unlink(old_part_.path.c_str());
- unlink(new_part_.path.c_str());
- if (blob_fd_ != -1)
- close(blob_fd_);
- unlink(blob_path_.c_str());
}
// Helper function to call DeltaMovedAndZeroBlocks() using this class' data
// members. This simply avoids repeating all the arguments that never change.
bool RunDeltaMovedAndZeroBlocks(ssize_t chunk_blocks,
uint32_t minor_version) {
- BlobFileWriter blob_file(blob_fd_, &blob_size_);
+ BlobFileWriter blob_file(tmp_blob_file_.fd(), &blob_size_);
PayloadVersion version(kBrilloMajorPayloadVersion, minor_version);
ExtentRanges old_zero_blocks;
return diff_utils::DeltaMovedAndZeroBlocks(&aops_,
@@ -155,10 +144,11 @@
// with
PartitionConfig old_part_{"part"};
PartitionConfig new_part_{"part"};
+ ScopedTempFile old_part_file_{"DeltaDiffUtilsTest-old_part-XXXXXX", true};
+ ScopedTempFile new_part_file_{"DeltaDiffUtilsTest-new_part-XXXXXX", true};
// The file holding the output blob from the various diff utils functions.
- string blob_path_;
- int blob_fd_{-1};
+ ScopedTempFile tmp_blob_file_{"DeltaDiffUtilsTest-blob-XXXXXX", true};
off_t blob_size_{0};
size_t block_size_{kBlockSize};
@@ -173,7 +163,7 @@
new_part_.verity.hash_tree_extent = ExtentForRange(20, 30);
new_part_.verity.fec_extent = ExtentForRange(40, 50);
- BlobFileWriter blob_file(blob_fd_, &blob_size_);
+ BlobFileWriter blob_file(tmp_blob_file_.fd(), &blob_size_);
EXPECT_TRUE(diff_utils::DeltaReadPartition(
&aops_,
old_part_,
diff --git a/payload_generator/ext2_filesystem_unittest.cc b/payload_generator/ext2_filesystem_unittest.cc
index 54600e9..88e1538 100644
--- a/payload_generator/ext2_filesystem_unittest.cc
+++ b/payload_generator/ext2_filesystem_unittest.cc
@@ -62,7 +62,7 @@
class Ext2FilesystemTest : public ::testing::Test {};
TEST_F(Ext2FilesystemTest, InvalidFilesystem) {
- test_utils::ScopedTempFile fs_filename_{"Ext2FilesystemTest-XXXXXX"};
+ ScopedTempFile fs_filename_{"Ext2FilesystemTest-XXXXXX"};
ASSERT_EQ(0, truncate(fs_filename_.path().c_str(), kDefaultFilesystemSize));
unique_ptr<Ext2Filesystem> fs =
Ext2Filesystem::CreateFromFile(fs_filename_.path());
diff --git a/payload_generator/full_update_generator_unittest.cc b/payload_generator/full_update_generator_unittest.cc
index 5f39e8b..d3b3491 100644
--- a/payload_generator/full_update_generator_unittest.cc
+++ b/payload_generator/full_update_generator_unittest.cc
@@ -41,11 +41,9 @@
config_.block_size = 4096;
new_part_conf.path = part_file_.path();
- EXPECT_TRUE(utils::MakeTempFile(
- "FullUpdateTest_blobs.XXXXXX", &out_blobs_path_, &out_blobs_fd_));
- blob_file_.reset(new BlobFileWriter(out_blobs_fd_, &out_blobs_length_));
- out_blobs_unlinker_.reset(new ScopedPathUnlinker(out_blobs_path_));
+ blob_file_writer_.reset(
+ new BlobFileWriter(blob_file_.fd(), &out_blobs_length_));
}
PayloadGenerationConfig config_;
@@ -54,14 +52,11 @@
vector<AnnotatedOperation> aops;
// Output file holding the payload blobs.
- string out_blobs_path_;
- int out_blobs_fd_{-1};
off_t out_blobs_length_{0};
- ScopedFdCloser out_blobs_fd_closer_{&out_blobs_fd_};
- test_utils::ScopedTempFile part_file_{"FullUpdateTest_partition.XXXXXX"};
+ ScopedTempFile part_file_{"FullUpdateTest_partition.XXXXXX"};
- std::unique_ptr<BlobFileWriter> blob_file_;
- std::unique_ptr<ScopedPathUnlinker> out_blobs_unlinker_;
+ ScopedTempFile blob_file_{"FullUpdateTest_blobs.XXXXXX", true};
+ std::unique_ptr<BlobFileWriter> blob_file_writer_;
// FullUpdateGenerator under test.
FullUpdateGenerator generator_;
@@ -77,7 +72,7 @@
EXPECT_TRUE(generator_.GenerateOperations(config_,
new_part_conf, // this is ignored
new_part_conf,
- blob_file_.get(),
+ blob_file_writer_.get(),
&aops));
int64_t new_part_chunks = new_part_conf.size / config_.hard_chunk_size;
EXPECT_EQ(new_part_chunks, static_cast<int64_t>(aops.size()));
@@ -108,7 +103,7 @@
EXPECT_TRUE(generator_.GenerateOperations(config_,
new_part_conf, // this is ignored
new_part_conf,
- blob_file_.get(),
+ blob_file_writer_.get(),
&aops));
// new_part has one chunk and a half.
EXPECT_EQ(2U, aops.size());
@@ -129,7 +124,7 @@
EXPECT_TRUE(generator_.GenerateOperations(config_,
new_part_conf, // this is ignored
new_part_conf,
- blob_file_.get(),
+ blob_file_writer_.get(),
&aops));
// new_part has less than one chunk.
diff --git a/payload_generator/mapfile_filesystem_unittest.cc b/payload_generator/mapfile_filesystem_unittest.cc
index 36ae3bf..57b672b 100644
--- a/payload_generator/mapfile_filesystem_unittest.cc
+++ b/payload_generator/mapfile_filesystem_unittest.cc
@@ -55,8 +55,8 @@
class MapfileFilesystemTest : public ::testing::Test {
protected:
- test_utils::ScopedTempFile temp_file_{"mapfile_file.XXXXXX"};
- test_utils::ScopedTempFile temp_mapfile_{"mapfile_mapfile.XXXXXX"};
+ ScopedTempFile temp_file_{"mapfile_file.XXXXXX"};
+ ScopedTempFile temp_mapfile_{"mapfile_mapfile.XXXXXX"};
};
TEST_F(MapfileFilesystemTest, EmptyFilesystem) {
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index 2688e9a..74423d1 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -103,11 +103,9 @@
const string& private_key_path,
uint64_t* metadata_size_out) {
// Reorder the data blobs with the manifest_.
- string ordered_blobs_path;
- TEST_AND_RETURN_FALSE(utils::MakeTempFile(
- "CrAU_temp_data.ordered.XXXXXX", &ordered_blobs_path, nullptr));
- ScopedPathUnlinker ordered_blobs_unlinker(ordered_blobs_path);
- TEST_AND_RETURN_FALSE(ReorderDataBlobs(data_blobs_path, ordered_blobs_path));
+ ScopedTempFile ordered_blobs_file("CrAU_temp_data.ordered.XXXXXX");
+ TEST_AND_RETURN_FALSE(
+ ReorderDataBlobs(data_blobs_path, ordered_blobs_file.path()));
// Check that install op blobs are in order.
uint64_t next_blob_offset = 0;
@@ -231,7 +229,7 @@
// Append the data blobs.
LOG(INFO) << "Writing final delta file data blobs...";
- int blobs_fd = open(ordered_blobs_path.c_str(), O_RDONLY, 0);
+ int blobs_fd = open(ordered_blobs_file.path().c_str(), O_RDONLY, 0);
ScopedFdCloser blobs_fd_closer(&blobs_fd);
TEST_AND_RETURN_FALSE(blobs_fd >= 0);
for (;;) {
diff --git a/payload_generator/payload_file_unittest.cc b/payload_generator/payload_file_unittest.cc
index 45faebb9..1fd36f5 100644
--- a/payload_generator/payload_file_unittest.cc
+++ b/payload_generator/payload_file_unittest.cc
@@ -36,7 +36,7 @@
};
TEST_F(PayloadFileTest, ReorderBlobsTest) {
- test_utils::ScopedTempFile orig_blobs("ReorderBlobsTest.orig.XXXXXX");
+ ScopedTempFile orig_blobs("ReorderBlobsTest.orig.XXXXXX");
// The operations have three blob and one gap (the whitespace):
// Rootfs operation 1: [8, 3] bcd
@@ -45,7 +45,7 @@
string orig_data = "kernel abcd";
EXPECT_TRUE(test_utils::WriteFileString(orig_blobs.path(), orig_data));
- test_utils::ScopedTempFile new_blobs("ReorderBlobsTest.new.XXXXXX");
+ ScopedTempFile new_blobs("ReorderBlobsTest.new.XXXXXX");
payload_.part_vec_.resize(2);
diff --git a/payload_generator/payload_generation_config_android_unittest.cc b/payload_generator/payload_generation_config_android_unittest.cc
index 44eaf55..e87b034 100644
--- a/payload_generator/payload_generation_config_android_unittest.cc
+++ b/payload_generator/payload_generation_config_android_unittest.cc
@@ -138,8 +138,7 @@
}
ImageConfig image_config_;
- test_utils::ScopedTempFile temp_file_{
- "PayloadGenerationConfigAndroidTest.XXXXXX"};
+ ScopedTempFile temp_file_{"PayloadGenerationConfigAndroidTest.XXXXXX"};
};
TEST_F(PayloadGenerationConfigAndroidTest, LoadVerityConfigSimpleTest) {
diff --git a/payload_generator/payload_properties_unittest.cc b/payload_generator/payload_properties_unittest.cc
index 19bc2f8..ed936ff 100644
--- a/payload_generator/payload_properties_unittest.cc
+++ b/payload_generator/payload_properties_unittest.cc
@@ -40,7 +40,6 @@
#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/payload_generator/payload_generation_config.h"
-using chromeos_update_engine::test_utils::ScopedTempFile;
using std::string;
using std::unique_ptr;
using std::vector;
@@ -60,14 +59,6 @@
PayloadFile payload;
EXPECT_TRUE(payload.Init(config));
- const string kTempFileTemplate = "temp_data.XXXXXX";
- int data_file_fd;
- string temp_file_path;
- EXPECT_TRUE(
- utils::MakeTempFile(kTempFileTemplate, &temp_file_path, &data_file_fd));
- ScopedPathUnlinker temp_file_unlinker(temp_file_path);
- EXPECT_LE(0, data_file_fd);
-
const auto SetupPartitionConfig =
[](PartitionConfig* config, const string& path, size_t size) {
config->path = path;
@@ -77,8 +68,8 @@
string zeros(size, '\0');
EXPECT_TRUE(utils::WriteFile(path, zeros.c_str(), zeros.size()));
};
- ScopedTempFile old_part_file;
- ScopedTempFile new_part_file;
+ ScopedTempFile old_part_file("old_part.XXXXXX");
+ ScopedTempFile new_part_file("new_part.XXXXXX");
PartitionConfig old_part(kPartitionNameRoot);
PartitionConfig new_part(kPartitionNameRoot);
SetupPartitionConfig(&old_part, old_part_file.path(), 0);
@@ -91,7 +82,8 @@
vector<AnnotatedOperation> aops;
off_t data_file_size = 0;
- BlobFileWriter blob_file_writer(data_file_fd, &data_file_size);
+ ScopedTempFile data_file("temp_data.XXXXXX", true);
+ BlobFileWriter blob_file_writer(data_file.fd(), &data_file_size);
// Generate the operations using the strategy we selected above.
EXPECT_TRUE(strategy->GenerateOperations(
config, old_part, new_part, &blob_file_writer, &aops));
@@ -100,10 +92,10 @@
uint64_t metadata_size;
EXPECT_TRUE(payload.WritePayload(
- payload_file.path(), temp_file_path, "", &metadata_size));
+ payload_file_.path(), data_file.path(), "", &metadata_size));
}
- ScopedTempFile payload_file;
+ ScopedTempFile payload_file_{"payload_file.XXXXXX"};
};
// Validate the hash of file exists within the output.
@@ -119,7 +111,7 @@
"}";
string json;
EXPECT_TRUE(
- PayloadProperties(payload_file.path()).GetPropertiesAsJson(&json));
+ PayloadProperties(payload_file_.path()).GetPropertiesAsJson(&json));
EXPECT_EQ(kJsonProperties, json) << "JSON contents:\n" << json;
}
@@ -131,7 +123,7 @@
"METADATA_HASH=aEKYyzJt2E8Gz8fzB+gmekN5mriotZCSq6R+kDfdeV4=\n"
"METADATA_SIZE=165\n";
string key_value;
- EXPECT_TRUE(PayloadProperties{payload_file.path()}.GetPropertiesAsKeyValue(
+ EXPECT_TRUE(PayloadProperties{payload_file_.path()}.GetPropertiesAsKeyValue(
&key_value));
EXPECT_EQ(kKeyValueProperties, key_value) << "Key Value contents:\n"
<< key_value;
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index fe62997..2a0b394 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -167,7 +167,7 @@
}
TEST_F(PayloadSignerTest, SkipMetadataSignatureTest) {
- test_utils::ScopedTempFile payload_file("payload.XXXXXX");
+ ScopedTempFile payload_file("payload.XXXXXX");
PayloadGenerationConfig config;
config.version.major = kBrilloMajorPayloadVersion;
PayloadFile payload;
@@ -194,7 +194,7 @@
}
TEST_F(PayloadSignerTest, VerifySignedPayloadTest) {
- test_utils::ScopedTempFile payload_file("payload.XXXXXX");
+ ScopedTempFile payload_file("payload.XXXXXX");
PayloadGenerationConfig config;
config.version.major = kBrilloMajorPayloadVersion;
PayloadFile payload;
diff --git a/payload_generator/squashfs_filesystem.cc b/payload_generator/squashfs_filesystem.cc
index 6152d7d..a41e283 100644
--- a/payload_generator/squashfs_filesystem.cc
+++ b/payload_generator/squashfs_filesystem.cc
@@ -72,15 +72,10 @@
}
bool GetFileMapContent(const string& sqfs_path, string* map) {
- // Create a tmp file
- string map_file;
- TEST_AND_RETURN_FALSE(
- utils::MakeTempFile("squashfs_file_map.XXXXXX", &map_file, nullptr));
- ScopedPathUnlinker map_unlinker(map_file);
-
+ ScopedTempFile map_file("squashfs_file_map.XXXXXX");
// Run unsquashfs to get the system file map.
// unsquashfs -m <map-file> <squashfs-file>
- vector<string> cmd = {"unsquashfs", "-m", map_file, sqfs_path};
+ vector<string> cmd = {"unsquashfs", "-m", map_file.path(), sqfs_path};
string stdout, stderr;
int exit_code;
if (!Subprocess::SynchronousExec(cmd, &exit_code, &stdout, &stderr) ||
@@ -89,7 +84,7 @@
<< stdout << " and stderr content: " << stderr;
return false;
}
- TEST_AND_RETURN_FALSE(utils::ReadFile(map_file, map));
+ TEST_AND_RETURN_FALSE(utils::ReadFile(map_file.path(), map));
return true;
}