Remove utils::MakeTempDirectory().
In favor of base::ScopedTempDir, except for PostinstallRunnerAction,
where the temp directory needs to be removed for every partition.
ScopedDirRemover is also removed because it's no longer used.
Test: ./update_engine_unittests
Test: cros_workon_make update_engine --test
Bug: 26955860
Change-Id: I954e6e892aff0cf9f8434a77408dc3c9eb64c1b5
diff --git a/common/prefs_unittest.cc b/common/prefs_unittest.cc
index d94623a..1000131 100644
--- a/common/prefs_unittest.cc
+++ b/common/prefs_unittest.cc
@@ -21,6 +21,7 @@
#include <string>
#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
#include <base/macros.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
@@ -41,19 +42,17 @@
class PrefsTest : public ::testing::Test {
protected:
void SetUp() override {
- ASSERT_TRUE(base::CreateNewTempDirectory("auprefs", &prefs_dir_));
+ ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+ prefs_dir_ = temp_dir_.path();
ASSERT_TRUE(prefs_.Init(prefs_dir_));
}
- void TearDown() override {
- base::DeleteFile(prefs_dir_, true); // recursive
- }
-
bool SetValue(const string& key, const string& value) {
return base::WriteFile(prefs_dir_.Append(key), value.data(),
value.length()) == static_cast<int>(value.length());
}
+ base::ScopedTempDir temp_dir_;
base::FilePath prefs_dir_;
Prefs prefs_;
};
diff --git a/common/test_utils.cc b/common/test_utils.cc
index 13ce6f9..dfdc6b8 100644
--- a/common/test_utils.cc
+++ b/common/test_utils.cc
@@ -246,8 +246,8 @@
ScopedLoopMounter::ScopedLoopMounter(const string& file_path,
string* mnt_path,
unsigned long flags) { // NOLINT - long
- EXPECT_TRUE(utils::MakeTempDirectory("mnt.XXXXXX", mnt_path));
- dir_remover_.reset(new ScopedDirRemover(*mnt_path));
+ EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
+ *mnt_path = temp_dir_.path().value();
string loop_dev;
loop_binder_.reset(
diff --git a/common/test_utils.h b/common/test_utils.h
index 952cbea..363801f 100644
--- a/common/test_utils.h
+++ b/common/test_utils.h
@@ -30,6 +30,7 @@
#include <base/callback.h>
#include <base/files/file_path.h>
+#include <base/files/scoped_temp_dir.h>
#include <gtest/gtest.h>
#include "update_engine/common/action.h"
@@ -182,7 +183,7 @@
// ScopedFilesystemUnmounter (the file system must be unmounted first)
// ScopedLoopbackDeviceBinder (then the loop device can be deleted)
// ScopedDirRemover (then the mount point can be deleted)
- std::unique_ptr<ScopedDirRemover> dir_remover_;
+ base::ScopedTempDir temp_dir_;
std::unique_ptr<ScopedLoopbackDeviceBinder> loop_binder_;
std::unique_ptr<ScopedFilesystemUnmounter> unmounter_;
};
diff --git a/common/utils.cc b/common/utils.cc
index 49502ff..f7d4585 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -622,22 +622,6 @@
return true;
}
-bool MakeTempDirectory(const string& base_dirname_template,
- string* dirname) {
- base::FilePath dirname_template;
- TEST_AND_RETURN_FALSE(GetTempName(base_dirname_template, &dirname_template));
- DCHECK(dirname);
- vector<char> buf(dirname_template.value().size() + 1);
- memcpy(buf.data(), dirname_template.value().data(),
- dirname_template.value().size());
- buf[dirname_template.value().size()] = '\0';
-
- char* return_code = mkdtemp(buf.data());
- TEST_AND_RETURN_FALSE_ERRNO(return_code != nullptr);
- *dirname = buf.data();
- return true;
-}
-
bool SetBlockDeviceReadOnly(const string& device, bool read_only) {
int fd = HANDLE_EINTR(open(device.c_str(), O_RDONLY | O_CLOEXEC));
if (fd < 0) {
diff --git a/common/utils.h b/common/utils.h
index 3af6e71..e950b15 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -142,14 +142,6 @@
std::string* filename,
int* fd);
-// If |base_dirname_template| is neither absolute (starts with "/") nor
-// explicitly relative to the current working directory (starts with "./" or
-// "../"), then it is prepended the system's temporary directory. On success,
-// stores the name of the new temporary directory in |dirname|. The template
-// must end with "XXXXXX". Returns true on success.
-bool MakeTempDirectory(const std::string& base_dirname_template,
- std::string* dirname);
-
// Splits the partition device name into the block device name and partition
// number. For example, "/dev/sda3" will be split into {"/dev/sda", 3} and
// "/dev/mmcblk0p2" into {"/dev/mmcblk0", 2}
@@ -366,27 +358,6 @@
DISALLOW_COPY_AND_ASSIGN(ScopedPathUnlinker);
};
-// Utility class to delete an empty directory when it goes out of scope.
-class ScopedDirRemover {
- public:
- explicit ScopedDirRemover(const std::string& path)
- : path_(path),
- should_remove_(true) {}
- ~ScopedDirRemover() {
- if (should_remove_ && (rmdir(path_.c_str()) < 0)) {
- PLOG(ERROR) << "Unable to remove dir " << path_;
- }
- }
- void set_should_remove(bool should_remove) { should_remove_ = should_remove; }
-
- protected:
- const std::string path_;
-
- private:
- bool should_remove_;
- DISALLOW_COPY_AND_ASSIGN(ScopedDirRemover);
-};
-
// 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 09d7f75..b5887e3 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -25,6 +25,7 @@
#include <base/files/file_path.h>
#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
#include <gtest/gtest.h>
#include "update_engine/common/test_utils.h"
@@ -80,17 +81,16 @@
}
TEST(UtilsTest, IsSymlinkTest) {
- string temp_dir;
- EXPECT_TRUE(utils::MakeTempDirectory("symlink-test.XXXXXX", &temp_dir));
- string temp_file = temp_dir + "/temp-file";
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ string temp_file = temp_dir.path().Append("temp-file").value();
EXPECT_TRUE(utils::WriteFile(temp_file.c_str(), "", 0));
- string temp_symlink = temp_dir + "/temp-symlink";
+ string temp_symlink = temp_dir.path().Append("temp-symlink").value();
EXPECT_EQ(0, symlink(temp_file.c_str(), temp_symlink.c_str()));
- EXPECT_FALSE(utils::IsSymlink(temp_dir.c_str()));
+ EXPECT_FALSE(utils::IsSymlink(temp_dir.path().value().c_str()));
EXPECT_FALSE(utils::IsSymlink(temp_file.c_str()));
EXPECT_TRUE(utils::IsSymlink(temp_symlink.c_str()));
EXPECT_FALSE(utils::IsSymlink("/non/existent/path"));
- EXPECT_TRUE(base::DeleteFile(base::FilePath(temp_dir), true));
}
TEST(UtilsTest, SplitPartitionNameTest) {
diff --git a/fake_p2p_manager_configuration.h b/fake_p2p_manager_configuration.h
index 2f05ba3..1bc1dc8 100644
--- a/fake_p2p_manager_configuration.h
+++ b/fake_p2p_manager_configuration.h
@@ -17,17 +17,14 @@
#ifndef UPDATE_ENGINE_FAKE_P2P_MANAGER_CONFIGURATION_H_
#define UPDATE_ENGINE_FAKE_P2P_MANAGER_CONFIGURATION_H_
-#include "update_engine/common/test_utils.h"
-#include "update_engine/common/utils.h"
#include "update_engine/p2p_manager.h"
#include <string>
#include <vector>
-#include <base/files/file_util.h>
-#include <base/logging.h>
-#include <base/strings/string_number_conversions.h>
+#include <base/files/scoped_temp_dir.h>
#include <base/strings/string_util.h>
+#include <gtest/gtest.h>
namespace chromeos_update_engine {
@@ -36,19 +33,12 @@
class FakeP2PManagerConfiguration : public P2PManager::Configuration {
public:
FakeP2PManagerConfiguration() {
- EXPECT_TRUE(utils::MakeTempDirectory("p2p-tc.XXXXXX", &p2p_dir_));
- }
-
- ~FakeP2PManagerConfiguration() {
- if (p2p_dir_.size() > 0 &&
- !base::DeleteFile(base::FilePath(p2p_dir_), true)) {
- PLOG(ERROR) << "Unable to unlink files and directory in " << p2p_dir_;
- }
+ EXPECT_TRUE(p2p_dir_.CreateUniqueTempDir());
}
// P2PManager::Configuration override
base::FilePath GetP2PDir() override {
- return base::FilePath(p2p_dir_);
+ return p2p_dir_.path();
}
// P2PManager::Configuration override
@@ -95,7 +85,7 @@
private:
// The temporary directory used for p2p.
- std::string p2p_dir_;
+ base::ScopedTempDir p2p_dir_;
// Argument vector for starting p2p.
std::vector<std::string> initctl_start_args_{"initctl", "start", "p2p"};
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 3a811f0..09bbb7c 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -24,6 +24,7 @@
#include <base/files/file_path.h>
#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
@@ -832,11 +833,10 @@
// a. it's not an official build; and
// b. there is no key in the root filesystem.
- string temp_dir;
- EXPECT_TRUE(utils::MakeTempDirectory("PublicKeyFromResponseTests.XXXXXX",
- &temp_dir));
- string non_existing_file = temp_dir + "/non-existing";
- string existing_file = temp_dir + "/existing";
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ string non_existing_file = temp_dir.path().Append("non-existing").value();
+ string existing_file = temp_dir.path().Append("existing").value();
EXPECT_EQ(0, System(base::StringPrintf("touch %s", existing_file.c_str())));
// Non-official build, non-existing public-key, key in response -> true
@@ -883,8 +883,6 @@
performer_.public_key_path_ = non_existing_file;
install_plan_.public_key_rsa = "not-valid-base64";
EXPECT_FALSE(performer_.GetPublicKeyFromResponse(&key_path));
-
- EXPECT_TRUE(base::DeleteFile(base::FilePath(temp_dir), true));
}
TEST_F(DeltaPerformerTest, ConfVersionsMatch) {
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index fa89857..c24590e 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -113,8 +113,9 @@
#ifdef __ANDROID__
fs_mount_dir_ = "/postinstall";
#else // __ANDROID__
- TEST_AND_RETURN(
- utils::MakeTempDirectory("au_postint_mount.XXXXXX", &fs_mount_dir_));
+ base::FilePath temp_dir;
+ TEST_AND_RETURN(base::CreateNewTempDirectory("au_postint_mount", &temp_dir));
+ fs_mount_dir_ = temp_dir.value();
#endif // __ANDROID__
base::FilePath postinstall_path(partition.postinstall_path);
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index da5a1d4..3b6b49a 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -26,6 +26,7 @@
#include <base/bind.h>
#include <base/files/file_util.h>
+#include <base/files/scoped_temp_dir.h>
#include <base/message_loop/message_loop.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
@@ -87,20 +88,16 @@
loop_.SetAsCurrent();
async_signal_handler_.Init();
subprocess_.Init(&async_signal_handler_);
- ASSERT_TRUE(utils::MakeTempDirectory(
- "postinstall_runner_action_unittest-XXXXXX", &working_dir_));
+ ASSERT_TRUE(working_dir_.CreateUniqueTempDir());
// We use a test-specific powerwash marker file, to avoid race conditions.
- powerwash_marker_file_ = working_dir_ + "/factory_install_reset";
+ powerwash_marker_file_ =
+ working_dir_.path().Append("factory_install_reset").value();
// These tests use the postinstall files generated by "generate_images.sh"
// stored in the "disk_ext2_unittest.img" image.
postinstall_image_ =
test_utils::GetBuildArtifactsPath("gen/disk_ext2_unittest.img");
}
- void TearDown() override {
- EXPECT_TRUE(base::DeleteFile(base::FilePath(working_dir_), true));
- }
-
// Setup an action processor and run the PostinstallRunnerAction with a single
// partition |device_path|, running the |postinstall_program| command from
// there.
@@ -158,7 +155,7 @@
Subprocess subprocess_;
// A temporary working directory used for the test.
- string working_dir_;
+ base::ScopedTempDir working_dir_;
string powerwash_marker_file_;
// The path to the postinstall sample image.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index eb8f16b..99cd2c8 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -131,8 +131,6 @@
}
void SetUp() override {
- CHECK(utils::MakeTempDirectory("UpdateAttempterTest-XXXXXX", &test_dir_));
-
EXPECT_NE(nullptr, attempter_.system_state_);
EXPECT_EQ(0, attempter_.http_response_code_);
EXPECT_EQ(UpdateStatus::IDLE, attempter_.status_);
@@ -161,10 +159,6 @@
.WillRepeatedly(ReturnPointee(&actual_using_p2p_for_sharing_));
}
- void TearDown() override {
- base::DeleteFile(base::FilePath(test_dir_), true);
- }
-
public:
void ScheduleQuitMainLoop();
@@ -209,8 +203,6 @@
NiceMock<MockPrefs>* prefs_; // Shortcut to fake_system_state_->mock_prefs().
NiceMock<MockConnectionManager> mock_connection_manager;
- string test_dir_;
-
bool actual_using_p2p_for_downloading_;
bool actual_using_p2p_for_sharing_;
};