CreateDirectory() should check if an existing path is actually a directory before skipping it. Also update a couple instances and comments to reflect current behaviour (see also http://codereview.chromium.org/1681).
Review URL: http://codereview.chromium.org/1709
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2060 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 806b9c6f00c6d21259d049d21ebea52de9b47663
diff --git a/base/file_util.h b/base/file_util.h
index 304d3cb..45d4a33 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -151,6 +151,9 @@
// Returns true if the given path is writable by the user, false otherwise.
bool PathIsWritable(const std::wstring& path);
+// Returns true if the given path exists and is a directory, false otherwise.
+bool DirectoryExists(const std::wstring& path);
+
#if defined(OS_WIN)
// Gets the creation time of the given file (expressed in the local timezone),
// and returns it via the creation_time parameter. Returns true if successful,
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index 201a2c0..f6beb57 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -111,6 +111,13 @@
return (stat64(WideToUTF8(path).c_str(), &file_info) == 0);
}
+bool DirectoryExists(const std::wstring& path) {
+ struct stat64 file_info;
+ if (stat64(WideToUTF8(path).c_str(), &file_info) == 0)
+ return S_ISDIR(file_info.st_mode);
+ return false;
+}
+
// TODO(erikkay): implement
#if 0
bool GetFileCreationLocalTimeFromHandle(int fd,
@@ -179,7 +186,7 @@
path = *i;
else
AppendToPath(&path, *i);
- if (!PathExists(path)) {
+ if (!DirectoryExists(path)) {
if (mkdir(WideToUTF8(path).c_str(), 0777) != 0)
return false;
}
@@ -324,5 +331,3 @@
} // namespace file_util
-
-
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index a93eda3..142967e 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -517,7 +517,7 @@
std::wstring dest_file(dir_name_from);
file_util::AppendToPath(&dest_file, L"DestFile.txt");
ASSERT_TRUE(file_util::CopyFile(file_name_from, dest_file));
-
+
// Copy the file to another location using '..' in the path.
std::wstring dest_file2(dir_name_from);
file_util::AppendToPath(&dest_file2, L"..");
@@ -697,15 +697,15 @@
TEST_F(FileUtilTest, CreateTemporaryFileNameTest) {
std::wstring temp_file;
file_util::CreateTemporaryFileName(&temp_file);
- EXPECT_EQ(file_util::PathExists(temp_file), true);
- EXPECT_EQ(file_util::Delete(temp_file, false), true);
+ EXPECT_TRUE(file_util::PathExists(temp_file));
+ EXPECT_TRUE(file_util::Delete(temp_file, false));
}
TEST_F(FileUtilTest, CreateNewTempDirectoryTest) {
std::wstring temp_dir;
file_util::CreateNewTempDirectory(std::wstring(), &temp_dir);
- EXPECT_EQ(file_util::PathExists(temp_dir), true);
- EXPECT_EQ(file_util::Delete(temp_dir, false), true);
+ EXPECT_TRUE(file_util::PathExists(temp_dir));
+ EXPECT_TRUE(file_util::Delete(temp_dir, false));
}
TEST_F(FileUtilTest, CreateDirectoryTest) {
@@ -717,13 +717,44 @@
#elif defined(OS_POSIX)
file_util::AppendToPath(&test_path, L"dir/tree/likely/doesnt/exist/");
#endif
-
- EXPECT_EQ(file_util::PathExists(test_path), false);
- EXPECT_EQ(file_util::CreateDirectory(test_path), true);
- EXPECT_EQ(file_util::PathExists(test_path), true);
- EXPECT_EQ(file_util::Delete(test_root, true), true);
- EXPECT_EQ(file_util::PathExists(test_root), false);
- EXPECT_EQ(file_util::PathExists(test_path), false);
+
+ EXPECT_FALSE(file_util::PathExists(test_path));
+ EXPECT_TRUE(file_util::CreateDirectory(test_path));
+ EXPECT_TRUE(file_util::PathExists(test_path));
+ // CreateDirectory returns true if the DirectoryExists returns true.
+ EXPECT_TRUE(file_util::CreateDirectory(test_path));
+
+ // Doesn't work to create it on top of a non-dir
+ file_util::AppendToPath(&test_path, L"foobar.txt");
+ EXPECT_FALSE(file_util::PathExists(test_path));
+ CreateTextFile(test_path, L"test file");
+ EXPECT_TRUE(file_util::PathExists(test_path));
+ EXPECT_FALSE(file_util::CreateDirectory(test_path));
+
+ EXPECT_TRUE(file_util::Delete(test_root, true));
+ EXPECT_FALSE(file_util::PathExists(test_root));
+ EXPECT_FALSE(file_util::PathExists(test_path));
+}
+
+TEST_F(FileUtilTest, DetectDirectoryTest) {
+ // Check a directory
+ std::wstring test_root = test_dir_;
+ file_util::AppendToPath(&test_root, L"detect_directory_test");
+ EXPECT_FALSE(file_util::PathExists(test_root));
+ EXPECT_TRUE(file_util::CreateDirectory(test_root));
+ EXPECT_TRUE(file_util::PathExists(test_root));
+ EXPECT_TRUE(file_util::DirectoryExists(test_root));
+
+ // Check a file
+ std::wstring test_path(test_root);
+ file_util::AppendToPath(&test_path, L"foobar.txt");
+ EXPECT_FALSE(file_util::PathExists(test_path));
+ CreateTextFile(test_path, L"test file");
+ EXPECT_TRUE(file_util::PathExists(test_path));
+ EXPECT_FALSE(file_util::DirectoryExists(test_path));
+ EXPECT_TRUE(file_util::Delete(test_path, false));
+
+ EXPECT_TRUE(file_util::Delete(test_root, true));
}
static const struct goodbad_pair {
diff --git a/base/path_service.cc b/base/path_service.cc
index 37ac433..078815d 100644
--- a/base/path_service.cc
+++ b/base/path_service.cc
@@ -205,9 +205,7 @@
return false;
// make sure the directory exists:
- if (!file_util::PathExists(file_path) &&
- // TODO(darin): what if this path is not that of a directory?
- !file_util::CreateDirectory(file_path))
+ if (!file_util::CreateDirectory(file_path))
return false;
file_util::TrimTrailingSeparator(&file_path);
@@ -251,4 +249,3 @@
#endif
path_data->providers = p;
}
-