From: Rafael Espindola Date: Thu, 11 Sep 2014 20:30:02 +0000 (+0000) Subject: Misc cleanups to the FileSytem api. X-Git-Tag: android-x86-7.1-r4~57619 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=8d230cd536cfc00cc58e749f6e9cf58dd138b0bb;p=android-x86%2Fexternal-llvm.git Misc cleanups to the FileSytem api. The main difference is the removal of std::error_code exists(const Twine &path, bool &result); It was an horribly redundant interface since a file not existing is also a valid error_code. Now we have an access function that returns just an error_code. This is the only function that has to be implemented for Unix and Windows. The functions can_write, exists and can_execute an now just wrappers. One still has to be very careful using these function to avoid introducing race conditions (Time of check to time of use). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@217625 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Support/FileSystem.h b/include/llvm/Support/FileSystem.h index fd68d8741fd..87fc4875bd5 100644 --- a/include/llvm/Support/FileSystem.h +++ b/include/llvm/Support/FileSystem.h @@ -352,33 +352,37 @@ std::error_code resize_file(const Twine &path, uint64_t size); /// not. bool exists(file_status status); -/// @brief Does file exist? +/// @brief Can the file be accessed? /// /// @param path Input path. -/// @param result Set to true if the file represented by status exists, false if -/// it does not. Undefined otherwise. -/// @returns errc::success if result has been successfully set, otherwise a +/// @returns errc::success if the path can be accessed, otherwise a /// platform-specific error_code. -std::error_code exists(const Twine &path, bool &result); +enum class AccessMode { Exist, Write, Execute }; +std::error_code access(const Twine &Path, AccessMode Mode); -/// @brief Simpler version of exists for clients that don't need to -/// differentiate between an error and false. -inline bool exists(const Twine &path) { - bool result; - return !exists(path, result) && result; +/// @brief Does file exist? +/// +/// @param Path Input path. +/// @returns True if it exists, false otherwise. +inline bool exists(const Twine &Path) { + return !access(Path, AccessMode::Exist); } /// @brief Can we execute this file? /// /// @param Path Input path. /// @returns True if we can execute it, false otherwise. -bool can_execute(const Twine &Path); +inline bool can_execute(const Twine &Path) { + return !access(Path, AccessMode::Execute); +} /// @brief Can we write this file? /// /// @param Path Input path. /// @returns True if we can write to it, false otherwise. -bool can_write(const Twine &Path); +inline bool can_write(const Twine &Path) { + return !access(Path, AccessMode::Write); +} /// @brief Do file_status's represent the same thing? /// diff --git a/lib/Support/LockFileManager.cpp b/lib/Support/LockFileManager.cpp index 8fc58017cb0..5b82c367c0a 100644 --- a/lib/Support/LockFileManager.cpp +++ b/lib/Support/LockFileManager.cpp @@ -204,8 +204,8 @@ LockFileManager::WaitForUnlockResult LockFileManager::waitForUnlock() { // If the lock file is still expected to be there, check whether it still // is. if (!LockFileGone) { - bool Exists; - if (!sys::fs::exists(LockFileName.str(), Exists) && !Exists) { + if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist) == + errc::no_such_file_or_directory) { LockFileGone = true; LockFileJustDisappeared = true; } diff --git a/lib/Support/Path.cpp b/lib/Support/Path.cpp index bdaa12820b8..ef75fc1cd83 100644 --- a/lib/Support/Path.cpp +++ b/lib/Support/Path.cpp @@ -210,13 +210,13 @@ retry_random_path: } case FS_Name: { - bool Exists; - std::error_code EC = sys::fs::exists(ResultPath.begin(), Exists); + std::error_code EC = + sys::fs::access(ResultPath.begin(), sys::fs::AccessMode::Exist); + if (EC == errc::no_such_file_or_directory) + return std::error_code(); if (EC) return EC; - if (Exists) - goto retry_random_path; - return std::error_code(); + goto retry_random_path; } case FS_Dir: { diff --git a/lib/Support/Unix/Path.inc b/lib/Support/Unix/Path.inc index 2e0519b3ca6..634d4049d7d 100644 --- a/lib/Support/Unix/Path.inc +++ b/lib/Support/Unix/Path.inc @@ -321,38 +321,35 @@ std::error_code resize_file(const Twine &path, uint64_t size) { return std::error_code(); } -std::error_code exists(const Twine &path, bool &result) { - SmallString<128> path_storage; - StringRef p = path.toNullTerminatedStringRef(path_storage); - - if (::access(p.begin(), F_OK) == -1) { - if (errno != ENOENT) - return std::error_code(errno, std::generic_category()); - result = false; - } else - result = true; - - return std::error_code(); +static int convertAccessMode(AccessMode Mode) { + switch (Mode) { + case AccessMode::Exist: + return F_OK; + case AccessMode::Write: + return W_OK; + case AccessMode::Execute: + return R_OK | X_OK; // scripts also need R_OK. + } + llvm_unreachable("invalid enum"); } -bool can_write(const Twine &Path) { +std::error_code access(const Twine &Path, AccessMode Mode) { SmallString<128> PathStorage; StringRef P = Path.toNullTerminatedStringRef(PathStorage); - return 0 == access(P.begin(), W_OK); -} -bool can_execute(const Twine &Path) { - SmallString<128> PathStorage; - StringRef P = Path.toNullTerminatedStringRef(PathStorage); + if (::access(P.begin(), convertAccessMode(Mode)) == -1) + return std::error_code(errno, std::generic_category()); - if (0 != access(P.begin(), R_OK | X_OK)) - return false; - struct stat buf; - if (0 != stat(P.begin(), &buf)) - return false; - if (!S_ISREG(buf.st_mode)) - return false; - return true; + if (Mode == AccessMode::Execute) { + // Don't say that directories are executable. + struct stat buf; + if (0 != stat(P.begin(), &buf)) + return errc::permission_denied; + if (!S_ISREG(buf.st_mode)) + return errc::permission_denied; + } + + return std::error_code(); } bool equivalent(file_status A, file_status B) { diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc index bff82141b34..d90d408e661 100644 --- a/lib/Support/Windows/Path.inc +++ b/lib/Support/Windows/Path.inc @@ -251,49 +251,29 @@ std::error_code resize_file(const Twine &path, uint64_t size) { return std::error_code(error, std::generic_category()); } -std::error_code exists(const Twine &path, bool &result) { - SmallString<128> path_storage; - SmallVector path_utf16; +std::error_code access(const Twine &Path, AccessMode Mode) { + SmallString<128> PathStorage; + SmallVector PathUtf16; - if (std::error_code ec = - UTF8ToUTF16(path.toStringRef(path_storage), path_utf16)) - return ec; + if (std::error_code EC = + UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16)) + return EC; - DWORD attributes = ::GetFileAttributesW(path_utf16.begin()); + DWORD Attributes = ::GetFileAttributesW(PathUtf16.begin()); - if (attributes == INVALID_FILE_ATTRIBUTES) { + if (Attributes == INVALID_FILE_ATTRIBUTES) { // See if the file didn't actually exist. DWORD LastError = ::GetLastError(); if (LastError != ERROR_FILE_NOT_FOUND && LastError != ERROR_PATH_NOT_FOUND) return windows_error(LastError); - result = false; - } else - result = true; - return std::error_code(); -} - -bool can_write(const Twine &Path) { - // FIXME: take security attributes into account. - SmallString<128> PathStorage; - SmallVector PathUtf16; - - if (UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16)) - return false; - - DWORD Attr = ::GetFileAttributesW(PathUtf16.begin()); - return (Attr != INVALID_FILE_ATTRIBUTES) && !(Attr & FILE_ATTRIBUTE_READONLY); -} - -bool can_execute(const Twine &Path) { - SmallString<128> PathStorage; - SmallVector PathUtf16; + return errc::no_such_file_or_directory; + } - if (UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16)) - return false; + if (Mode == AccessMode::Write && (Attributes & FILE_ATTRIBUTE_READONLY)) + return errc::permission_denied; - DWORD Attr = ::GetFileAttributesW(PathUtf16.begin()); - return Attr != INVALID_FILE_ATTRIBUTES; + return std::error_code(); } bool equivalent(file_status A, file_status B) { diff --git a/unittests/Support/FileOutputBufferTest.cpp b/unittests/Support/FileOutputBufferTest.cpp index b086f1e118d..911d51613b1 100644 --- a/unittests/Support/FileOutputBufferTest.cpp +++ b/unittests/Support/FileOutputBufferTest.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/Support/Errc.h" #include "llvm/Support/FileOutputBuffer.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" @@ -65,9 +66,8 @@ TEST(FileOutputBuffer, Test) { // Do *not* commit buffer. } // Verify file does not exist (because buffer not committed). - bool Exists = false; - ASSERT_NO_ERROR(fs::exists(Twine(File2), Exists)); - EXPECT_FALSE(Exists); + ASSERT_EQ(fs::access(Twine(File2), fs::AccessMode::Exist), + errc::no_such_file_or_directory); ASSERT_NO_ERROR(fs::remove(File2.str())); // TEST 3: Verify sizing down case. diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp index a00920264a1..d4de2735d31 100644 --- a/unittests/Support/Path.cpp +++ b/unittests/Support/Path.cpp @@ -361,9 +361,8 @@ TEST_F(FileSystemTest, TempFiles) { EXPECT_EQ(B.type(), fs::file_type::file_not_found); // Make sure Temp2 doesn't exist. - bool TempFileExists; - ASSERT_NO_ERROR(fs::exists(Twine(TempPath2), TempFileExists)); - EXPECT_FALSE(TempFileExists); + ASSERT_EQ(fs::access(Twine(TempPath2), sys::fs::AccessMode::Exist), + errc::no_such_file_or_directory); SmallString<64> TempPath3; ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "", TempPath3)); @@ -386,8 +385,8 @@ TEST_F(FileSystemTest, TempFiles) { ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); // Make sure Temp1 doesn't exist. - ASSERT_NO_ERROR(fs::exists(Twine(TempPath), TempFileExists)); - EXPECT_FALSE(TempFileExists); + ASSERT_EQ(fs::access(Twine(TempPath), sys::fs::AccessMode::Exist), + errc::no_such_file_or_directory); #ifdef LLVM_ON_WIN32 // Path name > 260 chars should get an error.