update_engine: Remove directory iteration usage of glib.
P2PManager used g_dir_* functions to iterate over the files in a
directory. This patch uses the POSIX equivalent functions instead.
BUG=chromium:499886
TEST=Unittest still pass.
Change-Id: Ia603dcdd5095db8aadf6fbe431b9a92c9d220d04
Reviewed-on: https://chromium-review.googlesource.com/284891
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
diff --git a/p2p_manager.cc b/p2p_manager.cc
index ee0f583..c72a023 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -11,10 +11,8 @@
#include "update_engine/p2p_manager.h"
#include <attr/xattr.h>
-#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
-#include <glib.h>
#include <linux/falloc.h>
#include <signal.h>
#include <string.h>
@@ -30,8 +28,10 @@
#include <vector>
#include <base/bind.h>
+#include <base/files/file_enumerator.h>
#include <base/files/file_path.h>
#include <base/logging.h>
+#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include "update_engine/glib_utils.h"
@@ -318,46 +318,32 @@
bool P2PManagerImpl::PerformHousekeeping() {
// Open p2p dir.
FilePath p2p_dir = configuration_->GetP2PDir();
- GError* error = nullptr;
- GDir* dir = g_dir_open(p2p_dir.value().c_str(), 0, &error);
- if (dir == nullptr) {
- LOG(ERROR) << "Error opening directory " << p2p_dir.value() << ": "
- << utils::GetAndFreeGError(&error);
- return false;
- }
+ const string ext_visible = GetExt(kVisible);
+ const string ext_non_visible = GetExt(kNonVisible);
- // Go through all files and collect their mtime.
- string ext_visible = GetExt(kVisible);
- string ext_non_visible = GetExt(kNonVisible);
bool deletion_failed = false;
- const char* name = nullptr;
vector<pair<FilePath, Time>> matches;
- while ((name = g_dir_read_name(dir)) != nullptr) {
- if (!(g_str_has_suffix(name, ext_visible.c_str()) ||
- g_str_has_suffix(name, ext_non_visible.c_str())))
+
+ base::FileEnumerator dir(p2p_dir, false, base::FileEnumerator::FILES);
+ // Go through all files and collect their mtime.
+ for (FilePath name = dir.Next(); !name.empty(); name = dir.Next()) {
+ if (!(base::EndsWith(name.value(), ext_visible, true) ||
+ base::EndsWith(name.value(), ext_non_visible, true)))
continue;
- struct stat statbuf;
- FilePath file = p2p_dir.Append(name);
- if (stat(file.value().c_str(), &statbuf) != 0) {
- PLOG(ERROR) << "Error getting file status for " << file.value();
- continue;
- }
-
- Time time = utils::TimeFromStructTimespec(&statbuf.st_mtim);
+ Time time = dir.GetInfo().GetLastModifiedTime();
// If instructed to keep only files younger than a given age
// (|max_file_age_| != 0), delete files satisfying this criteria
// right now. Otherwise add it to a list we'll consider for later.
if (clock_ != nullptr && max_file_age_ != TimeDelta() &&
clock_->GetWallclockTime() - time > max_file_age_) {
- if (!DeleteP2PFile(file, "file too old"))
+ if (!DeleteP2PFile(name, "file too old"))
deletion_failed = true;
} else {
- matches.push_back(std::make_pair(file, time));
+ matches.push_back(std::make_pair(name, time));
}
}
- g_dir_close(dir);
// If instructed to only keep N files (|max_files_to_keep_ != 0),
// sort list of matches, newest (biggest time) to oldest (lowest
@@ -670,28 +656,18 @@
}
int P2PManagerImpl::CountSharedFiles() {
- GDir* dir;
- GError* error = nullptr;
- const char* name;
int num_files = 0;
FilePath p2p_dir = configuration_->GetP2PDir();
- dir = g_dir_open(p2p_dir.value().c_str(), 0, &error);
- if (dir == nullptr) {
- LOG(ERROR) << "Error opening directory " << p2p_dir.value() << ": "
- << utils::GetAndFreeGError(&error);
- return -1;
- }
+ const string ext_visible = GetExt(kVisible);
+ const string ext_non_visible = GetExt(kNonVisible);
- string ext_visible = GetExt(kVisible);
- string ext_non_visible = GetExt(kNonVisible);
- while ((name = g_dir_read_name(dir)) != nullptr) {
- if (g_str_has_suffix(name, ext_visible.c_str()) ||
- g_str_has_suffix(name, ext_non_visible.c_str())) {
+ base::FileEnumerator dir(p2p_dir, false, base::FileEnumerator::FILES);
+ for (FilePath name = dir.Next(); !name.empty(); name = dir.Next()) {
+ if (base::EndsWith(name.value(), ext_visible, true) ||
+ base::EndsWith(name.value(), ext_non_visible, true))
num_files += 1;
- }
}
- g_dir_close(dir);
return num_files;
}
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index 3926f93..8667578 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -6,7 +6,6 @@
#include <dirent.h>
#include <fcntl.h>
-#include <glib.h>
#include <sys/stat.h>
#include <unistd.h>
#include <attr/xattr.h> // NOLINT - requires typed defined in unistd.h
@@ -17,6 +16,7 @@
#include <base/bind.h>
#include <base/callback.h>
+#include <base/files/file_util.h>
#include <base/strings/stringprintf.h>
#include <chromeos/message_loops/glib_message_loop.h>
#include <chromeos/message_loops/message_loop.h>
@@ -36,10 +36,9 @@
using base::TimeDelta;
using chromeos::MessageLoop;
-using chromeos_update_engine::test_utils::System;
using std::string;
-using std::vector;
using std::unique_ptr;
+using std::vector;
using testing::DoAll;
using testing::Return;
using testing::SetArgPointee;
@@ -129,39 +128,20 @@
TimeDelta() /* max_file_age */));
EXPECT_EQ(manager_->CountSharedFiles(), 0);
+ base::Time start_time = base::Time::FromDoubleT(1246996800.);
// Generate files with different timestamps matching our pattern and generate
// other files not matching the pattern.
- double last_timestamp = -1;
for (int n = 0; n < 5; n++) {
- double current_timestamp;
- do {
- base::FilePath file = test_conf_->GetP2PDir().Append(base::StringPrintf(
- "file_%d.cros_au.p2p", n));
- EXPECT_EQ(0, System(base::StringPrintf("touch %s",
- file.value().c_str())));
+ base::FilePath path = test_conf_->GetP2PDir().Append(base::StringPrintf(
+ "file_%d.cros_au.p2p", n));
+ base::Time file_time = start_time + TimeDelta::FromMinutes(n);
+ EXPECT_EQ(0, base::WriteFile(path, nullptr, 0));
+ EXPECT_TRUE(base::TouchFile(path, file_time, file_time));
- // Check that the current timestamp on the file is different from the
- // previous generated file. This timestamp depends on the file system
- // time resolution, for example, ext2/ext3 have a time resolution of one
- // second while ext4 has a resolution of one nanosecond. If the assigned
- // timestamp is the same, we introduce a bigger sleep and call touch
- // again.
- struct stat statbuf;
- EXPECT_EQ(stat(file.value().c_str(), &statbuf), 0);
- current_timestamp = utils::TimeFromStructTimespec(&statbuf.st_ctim)
- .ToDoubleT();
- if (current_timestamp == last_timestamp)
- sleep(1);
- } while (current_timestamp == last_timestamp);
- last_timestamp = current_timestamp;
-
- EXPECT_EQ(0, System(base::StringPrintf(
- "touch %s/file_%d.OTHER.p2p",
- test_conf_->GetP2PDir().value().c_str(), n)));
-
- // A sleep of one micro-second is enough to have a different "Change" time
- // on the file on newer file systems.
- g_usleep(1);
+ path = test_conf_->GetP2PDir().Append(base::StringPrintf(
+ "file_%d.OTHER.p2p", n));
+ EXPECT_EQ(0, base::WriteFile(path, nullptr, 0));
+ EXPECT_TRUE(base::TouchFile(path, file_time, file_time));
}
// CountSharedFiles() only counts 'cros_au' files.
EXPECT_EQ(manager_->CountSharedFiles(), 5);
@@ -178,12 +158,12 @@
file_name = base::StringPrintf(
"%s/file_%d.cros_au.p2p",
test_conf_->GetP2PDir().value().c_str(), n);
- EXPECT_EQ(!!g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS), expect);
+ EXPECT_EQ(expect, utils::FileExists(file_name.c_str()));
file_name = base::StringPrintf(
"%s/file_%d.OTHER.p2p",
test_conf_->GetP2PDir().value().c_str(), n);
- EXPECT_TRUE(g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS));
+ EXPECT_TRUE(utils::FileExists(file_name.c_str()));
}
// CountSharedFiles() only counts 'cros_au' files.
EXPECT_EQ(manager_->CountSharedFiles(), 3);
@@ -196,14 +176,14 @@
// September 2001 - arbitrary number, but constant to avoid test
// flakiness) since the epoch and then we put two files before that
// date and three files after.
- time_t cutoff_time = 1000000000;
+ base::Time cutoff_time = base::Time::FromTimeT(1000000000);
TimeDelta age_limit = TimeDelta::FromDays(5);
// Set the clock just so files with a timestamp before |cutoff_time|
// will be deleted at housekeeping.
- fake_clock_.SetWallclockTime(base::Time::FromTimeT(cutoff_time) + age_limit);
+ fake_clock_.SetWallclockTime(cutoff_time + age_limit);
- // Specifically pass 0 for |num_files_to_keep| to allow files of any age.
+ // Specifically pass 0 for |num_files_to_keep| to allow any number of files.
// Note that we need to reallocate the test_conf_ member, whose currently
// aliased object will be freed.
test_conf_ = new FakeP2PManagerConfiguration();
@@ -215,7 +195,7 @@
// Generate files with different timestamps matching our pattern and generate
// other files not matching the pattern.
for (int n = 0; n < 5; n++) {
- base::FilePath file = test_conf_->GetP2PDir().Append(base::StringPrintf(
+ base::FilePath path = test_conf_->GetP2PDir().Append(base::StringPrintf(
"file_%d.cros_au.p2p", n));
// With five files and aiming for two of them to be before
@@ -225,32 +205,16 @@
// |
// cutoff_time
//
- base::Time file_date = base::Time::FromTimeT(cutoff_time) +
- (n - 2)*TimeDelta::FromDays(1) + TimeDelta::FromHours(12);
+ base::Time file_date = cutoff_time + (n - 2) * TimeDelta::FromDays(1)
+ + TimeDelta::FromHours(12);
- // The touch(1) command expects input like this
- // --date="2004-02-27 14:19:13.489392193 +0530"
- base::Time::Exploded exploded;
- file_date.UTCExplode(&exploded);
- string file_date_string = base::StringPrintf(
- "%d-%02d-%02d %02d:%02d:%02d +0000",
- exploded.year, exploded.month, exploded.day_of_month,
- exploded.hour, exploded.minute, exploded.second);
+ EXPECT_EQ(0, base::WriteFile(path, nullptr, 0));
+ EXPECT_TRUE(base::TouchFile(path, file_date, file_date));
- // Sanity check that we generated the correct string.
- base::Time parsed_time;
- EXPECT_TRUE(base::Time::FromUTCString(file_date_string.c_str(),
- &parsed_time));
- EXPECT_EQ(parsed_time, file_date);
-
- EXPECT_EQ(0, System(base::StringPrintf("touch --date=\"%s\" %s",
- file_date_string.c_str(),
- file.value().c_str())));
-
- EXPECT_EQ(0, System(base::StringPrintf(
- "touch --date=\"%s\" %s/file_%d.OTHER.p2p",
- file_date_string.c_str(),
- test_conf_->GetP2PDir().value().c_str(), n)));
+ path = test_conf_->GetP2PDir().Append(base::StringPrintf(
+ "file_%d.OTHER.p2p", n));
+ EXPECT_EQ(0, base::WriteFile(path, nullptr, 0));
+ EXPECT_TRUE(base::TouchFile(path, file_date, file_date));
}
// CountSharedFiles() only counts 'cros_au' files.
EXPECT_EQ(manager_->CountSharedFiles(), 5);
@@ -267,12 +231,12 @@
file_name = base::StringPrintf(
"%s/file_%d.cros_au.p2p",
test_conf_->GetP2PDir().value().c_str(), n);
- EXPECT_EQ(!!g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS), expect);
+ EXPECT_EQ(expect, utils::FileExists(file_name.c_str()));
file_name = base::StringPrintf(
"%s/file_%d.OTHER.p2p",
test_conf_->GetP2PDir().value().c_str(), n);
- EXPECT_TRUE(g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS));
+ EXPECT_TRUE(utils::FileExists(file_name.c_str()));
}
// CountSharedFiles() only counts 'cros_au' files.
EXPECT_EQ(manager_->CountSharedFiles(), 3);