[llvm] r217625 - Misc cleanups to the FileSytem api.
Rafael Espindola
rafael.espindola at gmail.com
Thu Sep 11 13:30:02 PDT 2014
Author: rafael
Date: Thu Sep 11 15:30:02 2014
New Revision: 217625
URL: http://llvm.org/viewvc/llvm-project?rev=217625&view=rev
Log:
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).
Modified:
llvm/trunk/include/llvm/Support/FileSystem.h
llvm/trunk/lib/Support/LockFileManager.cpp
llvm/trunk/lib/Support/Path.cpp
llvm/trunk/lib/Support/Unix/Path.inc
llvm/trunk/lib/Support/Windows/Path.inc
llvm/trunk/unittests/Support/FileOutputBufferTest.cpp
llvm/trunk/unittests/Support/Path.cpp
Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Sep 11 15:30:02 2014
@@ -352,33 +352,37 @@ std::error_code resize_file(const Twine
/// 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?
///
Modified: llvm/trunk/lib/Support/LockFileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/lib/Support/LockFileManager.cpp (original)
+++ llvm/trunk/lib/Support/LockFileManager.cpp Thu Sep 11 15:30:02 2014
@@ -204,8 +204,8 @@ LockFileManager::WaitForUnlockResult Loc
// 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;
}
Modified: llvm/trunk/lib/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Path.cpp (original)
+++ llvm/trunk/lib/Support/Path.cpp Thu Sep 11 15:30:02 2014
@@ -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: {
Modified: llvm/trunk/lib/Support/Unix/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Path.inc (original)
+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Sep 11 15:30:02 2014
@@ -321,38 +321,35 @@ std::error_code resize_file(const Twine
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 (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;
+ }
- 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;
+ return std::error_code();
}
bool equivalent(file_status A, file_status B) {
Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Sep 11 15:30:02 2014
@@ -251,49 +251,29 @@ std::error_code resize_file(const Twine
return std::error_code(error, std::generic_category());
}
-std::error_code exists(const Twine &path, bool &result) {
- SmallString<128> path_storage;
- SmallVector<wchar_t, 128> path_utf16;
+std::error_code access(const Twine &Path, AccessMode Mode) {
+ SmallString<128> PathStorage;
+ SmallVector<wchar_t, 128> 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<wchar_t, 128> 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) && !(Attr & FILE_ATTRIBUTE_READONLY);
-}
-
-bool can_execute(const Twine &Path) {
- SmallString<128> PathStorage;
- SmallVector<wchar_t, 128> PathUtf16;
-
- if (UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16))
- return false;
-
- DWORD Attr = ::GetFileAttributesW(PathUtf16.begin());
- return Attr != INVALID_FILE_ATTRIBUTES;
+ return std::error_code();
}
bool equivalent(file_status A, file_status B) {
Modified: llvm/trunk/unittests/Support/FileOutputBufferTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FileOutputBufferTest.cpp?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/FileOutputBufferTest.cpp (original)
+++ llvm/trunk/unittests/Support/FileOutputBufferTest.cpp Thu Sep 11 15:30:02 2014
@@ -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.
Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Thu Sep 11 15:30:02 2014
@@ -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.
More information about the llvm-commits
mailing list