Move DeleteAfterReboot, Move, and ReplaceFile to base namespace
Rename ReplaceFileAndGetError (only used once) to ReplaceFile (used 5 times) and have each of those callers specify NULL for the output error if they don't care.
Remove InsertBeforeExtension from file_util.cc which seems to be unused.
BUG=
Review URL: https://codereview.chromium.org/18383003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209532 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 5553d5bcd8c1f0d90d74a2a52a4c9af51d730d01
diff --git a/base/file_util.cc b/base/file_util.cc
index 049bb36..bf5ce23 100644
--- a/base/file_util.cc
+++ b/base/file_util.cc
@@ -44,6 +44,12 @@
return running_size;
}
+bool Move(const FilePath& from_path, const FilePath& to_path) {
+ if (from_path.ReferencesParent() || to_path.ReferencesParent())
+ return false;
+ return MoveUnsafe(from_path, to_path);
+}
+
} // namespace base
// -----------------------------------------------------------------------------
@@ -55,37 +61,6 @@
using base::kExtensionSeparator;
using base::kMaxUniqueFiles;
-void InsertBeforeExtension(FilePath* path, const FilePath::StringType& suffix) {
- FilePath::StringType& value =
- const_cast<FilePath::StringType&>(path->value());
-
- const FilePath::StringType::size_type last_dot =
- value.rfind(kExtensionSeparator);
- const FilePath::StringType::size_type last_separator =
- value.find_last_of(FilePath::StringType(FilePath::kSeparators));
-
- if (last_dot == FilePath::StringType::npos ||
- (last_separator != std::wstring::npos && last_dot < last_separator)) {
- // The path looks something like "C:\pics.old\jojo" or "C:\pics\jojo".
- // We should just append the suffix to the entire path.
- value.append(suffix);
- return;
- }
-
- value.insert(last_dot, suffix);
-}
-
-bool Move(const FilePath& from_path, const FilePath& to_path) {
- if (from_path.ReferencesParent() || to_path.ReferencesParent())
- return false;
- return MoveUnsafe(from_path, to_path);
-}
-
-bool ReplaceFile(const base::FilePath& from_path,
- const base::FilePath& to_path) {
- return ReplaceFileAndGetError(from_path, to_path, NULL);
-}
-
bool CopyFile(const FilePath& from_path, const FilePath& to_path) {
if (from_path.ReferencesParent() || to_path.ReferencesParent())
return false;
diff --git a/base/file_util.h b/base/file_util.h
index 0fa7017..bdef48a 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -71,19 +71,13 @@
// TO "rm -rf", SO USE WITH CAUTION.
BASE_EXPORT bool Delete(const FilePath& path, bool recursive);
-} // namespace base
-
-// -----------------------------------------------------------------------------
-
-namespace file_util {
-
#if defined(OS_WIN)
// Schedules to delete the given path, whether it's a file or a directory, until
// the operating system is restarted.
// Note:
// 1) The file/directory to be deleted should exist in a temp folder.
// 2) The directory to be deleted must be empty.
-BASE_EXPORT bool DeleteAfterReboot(const base::FilePath& path);
+BASE_EXPORT bool DeleteAfterReboot(const FilePath& path);
#endif
// Moves the given path, whether it's a file or a directory.
@@ -91,13 +85,12 @@
// on different volumes, this will attempt to copy and delete. Returns
// true for success.
// This function fails if either path contains traversal components ('..').
-BASE_EXPORT bool Move(const base::FilePath& from_path,
- const base::FilePath& to_path);
+BASE_EXPORT bool Move(const FilePath& from_path, const FilePath& to_path);
// Same as Move but allows paths with traversal components.
// Use only with extreme care.
-BASE_EXPORT bool MoveUnsafe(const base::FilePath& from_path,
- const base::FilePath& to_path);
+BASE_EXPORT bool MoveUnsafe(const FilePath& from_path,
+ const FilePath& to_path);
// Renames file |from_path| to |to_path|. Both paths must be on the same
// volume, or the function will fail. Destination file will be created
@@ -105,13 +98,15 @@
// temporary files. On Windows it preserves attributes of the target file.
// Returns true on success, leaving *error unchanged.
// Returns false on failure and sets *error appropriately, if it is non-NULL.
-BASE_EXPORT bool ReplaceFileAndGetError(const base::FilePath& from_path,
- const base::FilePath& to_path,
- base::PlatformFileError* error);
+BASE_EXPORT bool ReplaceFile(const FilePath& from_path,
+ const FilePath& to_path,
+ PlatformFileError* error);
-// Backward-compatible convenience method for the above.
-BASE_EXPORT bool ReplaceFile(const base::FilePath& from_path,
- const base::FilePath& to_path);
+} // namespace base
+
+// -----------------------------------------------------------------------------
+
+namespace file_util {
// Copies a single file. Use CopyDirectory to copy directories.
// This function fails if either path contains traversal components ('..').
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index afb34b4..899bdb9 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -65,28 +65,28 @@
#if defined(OS_BSD) || defined(OS_MACOSX)
typedef struct stat stat_wrapper_t;
static int CallStat(const char *path, stat_wrapper_t *sb) {
- base::ThreadRestrictions::AssertIOAllowed();
+ ThreadRestrictions::AssertIOAllowed();
return stat(path, sb);
}
static int CallLstat(const char *path, stat_wrapper_t *sb) {
- base::ThreadRestrictions::AssertIOAllowed();
+ ThreadRestrictions::AssertIOAllowed();
return lstat(path, sb);
}
#else
typedef struct stat64 stat_wrapper_t;
static int CallStat(const char *path, stat_wrapper_t *sb) {
- base::ThreadRestrictions::AssertIOAllowed();
+ ThreadRestrictions::AssertIOAllowed();
return stat64(path, sb);
}
static int CallLstat(const char *path, stat_wrapper_t *sb) {
- base::ThreadRestrictions::AssertIOAllowed();
+ ThreadRestrictions::AssertIOAllowed();
return lstat64(path, sb);
}
#endif
// Helper for NormalizeFilePath(), defined below.
bool RealPath(const FilePath& path, FilePath* real_path) {
- base::ThreadRestrictions::AssertIOAllowed(); // For realpath().
+ ThreadRestrictions::AssertIOAllowed(); // For realpath().
FilePath::CharType buf[PATH_MAX];
if (!realpath(path.value().c_str(), buf))
return false;
@@ -134,6 +134,18 @@
return true;
}
+std::string TempFileName() {
+#if defined(OS_MACOSX)
+ return StringPrintf(".%s.XXXXXX", base::mac::BaseBundleID());
+#endif
+
+#if defined(GOOGLE_CHROME_BUILD)
+ return std::string(".com.google.Chrome.XXXXXX");
+#else
+ return std::string(".org.chromium.Chromium.XXXXXX");
+#endif
+}
+
} // namespace
FilePath MakeAbsoluteFilePath(const FilePath& input) {
@@ -185,35 +197,8 @@
return success;
}
-} // namespace base
-
-// -----------------------------------------------------------------------------
-
-namespace file_util {
-
-using base::stat_wrapper_t;
-using base::CallStat;
-using base::CallLstat;
-using base::FileEnumerator;
-using base::FilePath;
-using base::MakeAbsoluteFilePath;
-using base::RealPath;
-using base::VerifySpecificPathControlledByUser;
-
-static std::string TempFileName() {
-#if defined(OS_MACOSX)
- return base::StringPrintf(".%s.XXXXXX", base::mac::BaseBundleID());
-#endif
-
-#if defined(GOOGLE_CHROME_BUILD)
- return std::string(".com.google.Chrome.XXXXXX");
-#else
- return std::string(".org.chromium.Chromium.XXXXXX");
-#endif
-}
-
bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) {
- base::ThreadRestrictions::AssertIOAllowed();
+ ThreadRestrictions::AssertIOAllowed();
// Windows compatibility: if to_path exists, from_path and to_path
// must be the same type, either both files, or both directories.
stat_wrapper_t to_file_info;
@@ -230,24 +215,39 @@
if (rename(from_path.value().c_str(), to_path.value().c_str()) == 0)
return true;
- if (!CopyDirectory(from_path, to_path, true))
+ if (!file_util::CopyDirectory(from_path, to_path, true))
return false;
Delete(from_path, true);
return true;
}
-bool ReplaceFileAndGetError(const FilePath& from_path,
- const FilePath& to_path,
- base::PlatformFileError* error) {
- base::ThreadRestrictions::AssertIOAllowed();
+bool ReplaceFile(const FilePath& from_path,
+ const FilePath& to_path,
+ PlatformFileError* error) {
+ ThreadRestrictions::AssertIOAllowed();
if (rename(from_path.value().c_str(), to_path.value().c_str()) == 0)
return true;
if (error)
- *error = base::ErrnoToPlatformFileError(errno);
+ *error = ErrnoToPlatformFileError(errno);
return false;
}
+} // namespace base
+
+// -----------------------------------------------------------------------------
+
+namespace file_util {
+
+using base::stat_wrapper_t;
+using base::CallStat;
+using base::CallLstat;
+using base::FileEnumerator;
+using base::FilePath;
+using base::MakeAbsoluteFilePath;
+using base::RealPath;
+using base::VerifySpecificPathControlledByUser;
+
bool CopyDirectory(const FilePath& from_path,
const FilePath& to_path,
bool recursive) {
@@ -442,7 +442,7 @@
// This function does NOT unlink() the file.
int CreateAndOpenFdForTemporaryFile(FilePath directory, FilePath* path) {
base::ThreadRestrictions::AssertIOAllowed(); // For call to mkstemp().
- *path = directory.Append(TempFileName());
+ *path = directory.Append(base::TempFileName());
const std::string& tmpdir_string = path->value();
// this should be OK since mkstemp just replaces characters in place
char* buffer = const_cast<char*>(tmpdir_string.c_str());
@@ -522,7 +522,8 @@
if (!GetTempDir(&tmpdir))
return false;
- return CreateTemporaryDirInDirImpl(tmpdir, TempFileName(), new_temp_path);
+ return CreateTemporaryDirInDirImpl(tmpdir, base::TempFileName(),
+ new_temp_path);
}
bool CreateDirectoryAndGetError(const FilePath& full_path,
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index 7626957..aab56a9 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -1069,7 +1069,7 @@
FILE_PATH_LITERAL("Move_Test_File_Destination.txt"));
ASSERT_FALSE(file_util::PathExists(file_name_to));
- EXPECT_TRUE(file_util::Move(file_name_from, file_name_to));
+ EXPECT_TRUE(base::Move(file_name_from, file_name_to));
// Check everything has been moved.
EXPECT_FALSE(file_util::PathExists(file_name_from));
@@ -1089,7 +1089,7 @@
CreateTextFile(file_name_to, L"Old file content");
ASSERT_TRUE(file_util::PathExists(file_name_to));
- EXPECT_TRUE(file_util::Move(file_name_from, file_name_to));
+ EXPECT_TRUE(base::Move(file_name_from, file_name_to));
// Check everything has been moved.
EXPECT_FALSE(file_util::PathExists(file_name_from));
@@ -1110,7 +1110,7 @@
file_util::CreateDirectory(dir_name_to);
ASSERT_TRUE(file_util::PathExists(dir_name_to));
- EXPECT_FALSE(file_util::Move(file_name_from, dir_name_to));
+ EXPECT_FALSE(base::Move(file_name_from, dir_name_to));
}
@@ -1135,7 +1135,7 @@
ASSERT_FALSE(file_util::PathExists(dir_name_to));
- EXPECT_TRUE(file_util::Move(dir_name_from, dir_name_to));
+ EXPECT_TRUE(base::Move(dir_name_from, dir_name_to));
// Check everything has been moved.
EXPECT_FALSE(file_util::PathExists(dir_name_from));
@@ -1147,10 +1147,10 @@
file_name_from = dir_name_to.Append(txt_file_name);
file_name_to = dir_name_to.Append(FILE_PATH_LITERAL(".."));
file_name_to = file_name_to.Append(txt_file_name);
- EXPECT_FALSE(file_util::Move(file_name_from, file_name_to));
+ EXPECT_FALSE(base::Move(file_name_from, file_name_to));
EXPECT_TRUE(file_util::PathExists(file_name_from));
EXPECT_FALSE(file_util::PathExists(file_name_to));
- EXPECT_TRUE(file_util::MoveUnsafe(file_name_from, file_name_to));
+ EXPECT_TRUE(base::MoveUnsafe(file_name_from, file_name_to));
EXPECT_FALSE(file_util::PathExists(file_name_from));
EXPECT_TRUE(file_util::PathExists(file_name_to));
}
@@ -1181,7 +1181,7 @@
file_util::CreateDirectory(dir_name_exists);
ASSERT_TRUE(file_util::PathExists(dir_name_exists));
- EXPECT_TRUE(file_util::Move(dir_name_from, dir_name_to));
+ EXPECT_TRUE(base::Move(dir_name_from, dir_name_to));
// Check everything has been moved.
EXPECT_FALSE(file_util::PathExists(dir_name_from));
diff --git a/base/files/file_path_watcher_browsertest.cc b/base/files/file_path_watcher_browsertest.cc
index 2a8b0cf..a0b4319 100644
--- a/base/files/file_path_watcher_browsertest.cc
+++ b/base/files/file_path_watcher_browsertest.cc
@@ -261,7 +261,7 @@
ASSERT_TRUE(SetupWatch(test_file(), &watcher, delegate.get(), false));
// Now make sure we get notified if the file is modified.
- ASSERT_TRUE(file_util::Move(source_file, test_file()));
+ ASSERT_TRUE(base::Move(source_file, test_file()));
ASSERT_TRUE(WaitForEvents());
DeleteDelegateOnFileThread(delegate.release());
}
@@ -496,7 +496,7 @@
ASSERT_TRUE(WaitForEvents());
// Move the parent directory.
- file_util::Move(dir, dest);
+ base::Move(dir, dest);
VLOG(1) << "Waiting for directory move";
ASSERT_TRUE(WaitForEvents());
DeleteDelegateOnFileThread(file_delegate.release());
@@ -588,7 +588,7 @@
false));
// Move the directory into place, s.t. the watched file appears.
- ASSERT_TRUE(file_util::Move(source_dir, dest_dir));
+ ASSERT_TRUE(base::Move(source_dir, dest_dir));
ASSERT_TRUE(WaitForEvents());
DeleteDelegateOnFileThread(file_delegate.release());
DeleteDelegateOnFileThread(subdir_delegate.release());
diff --git a/base/files/file_util_proxy_unittest.cc b/base/files/file_util_proxy_unittest.cc
index a4ba571..f524ba4 100644
--- a/base/files/file_util_proxy_unittest.cc
+++ b/base/files/file_util_proxy_unittest.cc
@@ -169,7 +169,7 @@
#if defined(OS_WIN)
// This fails on Windows if the file is not closed.
- EXPECT_FALSE(file_util::Move(test_path(),
+ EXPECT_FALSE(base::Move(test_path(),
test_dir_path().AppendASCII("new")));
#endif
@@ -181,7 +181,7 @@
EXPECT_EQ(PLATFORM_FILE_OK, error_);
// Now it should pass on all platforms.
- EXPECT_TRUE(file_util::Move(test_path(), test_dir_path().AppendASCII("new")));
+ EXPECT_TRUE(base::Move(test_path(), test_dir_path().AppendASCII("new")));
}
TEST_F(FileUtilProxyTest, CreateTemporary) {
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc
index f10e7e6..b3564ef 100644
--- a/base/files/important_file_writer.cc
+++ b/base/files/important_file_writer.cc
@@ -84,7 +84,7 @@
return false;
}
- if (!file_util::ReplaceFile(tmp_file_path, path)) {
+ if (!base::ReplaceFile(tmp_file_path, path, NULL)) {
LogFailure(path, FAILED_RENAMING, "could not rename temporary file");
base::Delete(tmp_file_path, false);
return false;
diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc
index a753dae..ff3c232 100644
--- a/base/prefs/json_pref_store.cc
+++ b/base/prefs/json_pref_store.cc
@@ -130,7 +130,7 @@
// want to differentiate between recent and long ago errors.
if (file_util::PathExists(bad))
*error = PersistentPrefStore::PREF_READ_ERROR_JSON_REPEAT;
- file_util::Move(path, bad);
+ base::Move(path, bad);
break;
}
} else if (!value->IsType(base::Value::TYPE_DICTIONARY)) {