<div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">- bool Exists;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">- if (!sys::fs::exists(</span><span style="font-family:arial,sans-serif;font-size:13px">LockFileName.str(), Exists) && !Exists) {</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">+ if (sys::fs::access(LockFileName.</span><span style="font-family:arial,sans-serif;font-size:13px">c_str(), sys::fs::AccessMode::Exist) ==</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">+ errc::no_such_file_or_</span><span style="font-family:arial,sans-serif;font-size:13px">directory) {</span><br><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">Is there a reason you didn't just use the `</span><span style="font-family:arial,sans-serif;font-size:13px">bool exists(const Twine &Path)` helper here?</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">-- Sean Silva</span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 11, 2014 at 1:30 PM, Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Thu Sep 11 15:30:02 2014<br>
New Revision: 217625<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217625&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217625&view=rev</a><br>
Log:<br>
Misc cleanups to the FileSytem api.<br>
<br>
The main difference is the removal of<br>
<br>
std::error_code exists(const Twine &path, bool &result);<br>
<br>
It was an horribly redundant interface since a file not existing is also a valid<br>
error_code. Now we have an access function that returns just an error_code. This<br>
is the only function that has to be implemented for Unix and Windows. The<br>
functions can_write, exists and can_execute an now just wrappers.<br>
<br>
One still has to be very careful using these function to avoid introducing<br>
race conditions (Time of check to time of use).<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Support/FileSystem.h<br>
llvm/trunk/lib/Support/LockFileManager.cpp<br>
llvm/trunk/lib/Support/Path.cpp<br>
llvm/trunk/lib/Support/Unix/Path.inc<br>
llvm/trunk/lib/Support/Windows/Path.inc<br>
llvm/trunk/unittests/Support/FileOutputBufferTest.cpp<br>
llvm/trunk/unittests/Support/Path.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Support/FileSystem.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)<br>
+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Sep 11 15:30:02 2014<br>
@@ -352,33 +352,37 @@ std::error_code resize_file(const Twine<br>
/// not.<br>
bool exists(file_status status);<br>
<br>
-/// @brief Does file exist?<br>
+/// @brief Can the file be accessed?<br>
///<br>
/// @param path Input path.<br>
-/// @param result Set to true if the file represented by status exists, false if<br>
-/// it does not. Undefined otherwise.<br>
-/// @returns errc::success if result has been successfully set, otherwise a<br>
+/// @returns errc::success if the path can be accessed, otherwise a<br>
/// platform-specific error_code.<br>
-std::error_code exists(const Twine &path, bool &result);<br>
+enum class AccessMode { Exist, Write, Execute };<br>
+std::error_code access(const Twine &Path, AccessMode Mode);<br>
<br>
-/// @brief Simpler version of exists for clients that don't need to<br>
-/// differentiate between an error and false.<br>
-inline bool exists(const Twine &path) {<br>
- bool result;<br>
- return !exists(path, result) && result;<br>
+/// @brief Does file exist?<br>
+///<br>
+/// @param Path Input path.<br>
+/// @returns True if it exists, false otherwise.<br>
+inline bool exists(const Twine &Path) {<br>
+ return !access(Path, AccessMode::Exist);<br>
}<br>
<br>
/// @brief Can we execute this file?<br>
///<br>
/// @param Path Input path.<br>
/// @returns True if we can execute it, false otherwise.<br>
-bool can_execute(const Twine &Path);<br>
+inline bool can_execute(const Twine &Path) {<br>
+ return !access(Path, AccessMode::Execute);<br>
+}<br>
<br>
/// @brief Can we write this file?<br>
///<br>
/// @param Path Input path.<br>
/// @returns True if we can write to it, false otherwise.<br>
-bool can_write(const Twine &Path);<br>
+inline bool can_write(const Twine &Path) {<br>
+ return !access(Path, AccessMode::Write);<br>
+}<br>
<br>
/// @brief Do file_status's represent the same thing?<br>
///<br>
<br>
Modified: llvm/trunk/lib/Support/LockFileManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/LockFileManager.cpp (original)<br>
+++ llvm/trunk/lib/Support/LockFileManager.cpp Thu Sep 11 15:30:02 2014<br>
@@ -204,8 +204,8 @@ LockFileManager::WaitForUnlockResult Loc<br>
// If the lock file is still expected to be there, check whether it still<br>
// is.<br>
if (!LockFileGone) {<br>
- bool Exists;<br>
- if (!sys::fs::exists(LockFileName.str(), Exists) && !Exists) {<br>
+ if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist) ==<br>
+ errc::no_such_file_or_directory) {<br>
LockFileGone = true;<br>
LockFileJustDisappeared = true;<br>
}<br>
<br>
Modified: llvm/trunk/lib/Support/Path.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/Path.cpp (original)<br>
+++ llvm/trunk/lib/Support/Path.cpp Thu Sep 11 15:30:02 2014<br>
@@ -210,13 +210,13 @@ retry_random_path:<br>
}<br>
<br>
case FS_Name: {<br>
- bool Exists;<br>
- std::error_code EC = sys::fs::exists(ResultPath.begin(), Exists);<br>
+ std::error_code EC =<br>
+ sys::fs::access(ResultPath.begin(), sys::fs::AccessMode::Exist);<br>
+ if (EC == errc::no_such_file_or_directory)<br>
+ return std::error_code();<br>
if (EC)<br>
return EC;<br>
- if (Exists)<br>
- goto retry_random_path;<br>
- return std::error_code();<br>
+ goto retry_random_path;<br>
}<br>
<br>
case FS_Dir: {<br>
<br>
Modified: llvm/trunk/lib/Support/Unix/Path.inc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/Unix/Path.inc (original)<br>
+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Sep 11 15:30:02 2014<br>
@@ -321,38 +321,35 @@ std::error_code resize_file(const Twine<br>
return std::error_code();<br>
}<br>
<br>
-std::error_code exists(const Twine &path, bool &result) {<br>
- SmallString<128> path_storage;<br>
- StringRef p = path.toNullTerminatedStringRef(path_storage);<br>
-<br>
- if (::access(p.begin(), F_OK) == -1) {<br>
- if (errno != ENOENT)<br>
- return std::error_code(errno, std::generic_category());<br>
- result = false;<br>
- } else<br>
- result = true;<br>
-<br>
- return std::error_code();<br>
+static int convertAccessMode(AccessMode Mode) {<br>
+ switch (Mode) {<br>
+ case AccessMode::Exist:<br>
+ return F_OK;<br>
+ case AccessMode::Write:<br>
+ return W_OK;<br>
+ case AccessMode::Execute:<br>
+ return R_OK | X_OK; // scripts also need R_OK.<br>
+ }<br>
+ llvm_unreachable("invalid enum");<br>
}<br>
<br>
-bool can_write(const Twine &Path) {<br>
+std::error_code access(const Twine &Path, AccessMode Mode) {<br>
SmallString<128> PathStorage;<br>
StringRef P = Path.toNullTerminatedStringRef(PathStorage);<br>
- return 0 == access(P.begin(), W_OK);<br>
-}<br>
<br>
-bool can_execute(const Twine &Path) {<br>
- SmallString<128> PathStorage;<br>
- StringRef P = Path.toNullTerminatedStringRef(PathStorage);<br>
+ if (::access(P.begin(), convertAccessMode(Mode)) == -1)<br>
+ return std::error_code(errno, std::generic_category());<br>
+<br>
+ if (Mode == AccessMode::Execute) {<br>
+ // Don't say that directories are executable.<br>
+ struct stat buf;<br>
+ if (0 != stat(P.begin(), &buf))<br>
+ return errc::permission_denied;<br>
+ if (!S_ISREG(buf.st_mode))<br>
+ return errc::permission_denied;<br>
+ }<br>
<br>
- if (0 != access(P.begin(), R_OK | X_OK))<br>
- return false;<br>
- struct stat buf;<br>
- if (0 != stat(P.begin(), &buf))<br>
- return false;<br>
- if (!S_ISREG(buf.st_mode))<br>
- return false;<br>
- return true;<br>
+ return std::error_code();<br>
}<br>
<br>
bool equivalent(file_status A, file_status B) {<br>
<br>
Modified: llvm/trunk/lib/Support/Windows/Path.inc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/Windows/Path.inc (original)<br>
+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Sep 11 15:30:02 2014<br>
@@ -251,49 +251,29 @@ std::error_code resize_file(const Twine<br>
return std::error_code(error, std::generic_category());<br>
}<br>
<br>
-std::error_code exists(const Twine &path, bool &result) {<br>
- SmallString<128> path_storage;<br>
- SmallVector<wchar_t, 128> path_utf16;<br>
+std::error_code access(const Twine &Path, AccessMode Mode) {<br>
+ SmallString<128> PathStorage;<br>
+ SmallVector<wchar_t, 128> PathUtf16;<br>
<br>
- if (std::error_code ec =<br>
- UTF8ToUTF16(path.toStringRef(path_storage), path_utf16))<br>
- return ec;<br>
+ if (std::error_code EC =<br>
+ UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16))<br>
+ return EC;<br>
<br>
- DWORD attributes = ::GetFileAttributesW(path_utf16.begin());<br>
+ DWORD Attributes = ::GetFileAttributesW(PathUtf16.begin());<br>
<br>
- if (attributes == INVALID_FILE_ATTRIBUTES) {<br>
+ if (Attributes == INVALID_FILE_ATTRIBUTES) {<br>
// See if the file didn't actually exist.<br>
DWORD LastError = ::GetLastError();<br>
if (LastError != ERROR_FILE_NOT_FOUND &&<br>
LastError != ERROR_PATH_NOT_FOUND)<br>
return windows_error(LastError);<br>
- result = false;<br>
- } else<br>
- result = true;<br>
- return std::error_code();<br>
-}<br>
-<br>
-bool can_write(const Twine &Path) {<br>
- // FIXME: take security attributes into account.<br>
- SmallString<128> PathStorage;<br>
- SmallVector<wchar_t, 128> PathUtf16;<br>
+ return errc::no_such_file_or_directory;<br>
+ }<br>
<br>
- if (UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16))<br>
- return false;<br>
+ if (Mode == AccessMode::Write && (Attributes & FILE_ATTRIBUTE_READONLY))<br>
+ return errc::permission_denied;<br>
<br>
- DWORD Attr = ::GetFileAttributesW(PathUtf16.begin());<br>
- return (Attr != INVALID_FILE_ATTRIBUTES) && !(Attr & FILE_ATTRIBUTE_READONLY);<br>
-}<br>
-<br>
-bool can_execute(const Twine &Path) {<br>
- SmallString<128> PathStorage;<br>
- SmallVector<wchar_t, 128> PathUtf16;<br>
-<br>
- if (UTF8ToUTF16(Path.toStringRef(PathStorage), PathUtf16))<br>
- return false;<br>
-<br>
- DWORD Attr = ::GetFileAttributesW(PathUtf16.begin());<br>
- return Attr != INVALID_FILE_ATTRIBUTES;<br>
+ return std::error_code();<br>
}<br>
<br>
bool equivalent(file_status A, file_status B) {<br>
<br>
Modified: llvm/trunk/unittests/Support/FileOutputBufferTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FileOutputBufferTest.cpp?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FileOutputBufferTest.cpp?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Support/FileOutputBufferTest.cpp (original)<br>
+++ llvm/trunk/unittests/Support/FileOutputBufferTest.cpp Thu Sep 11 15:30:02 2014<br>
@@ -7,6 +7,7 @@<br>
//<br>
//===----------------------------------------------------------------------===//<br>
<br>
+#include "llvm/Support/Errc.h"<br>
#include "llvm/Support/FileOutputBuffer.h"<br>
#include "llvm/Support/ErrorHandling.h"<br>
#include "llvm/Support/FileSystem.h"<br>
@@ -65,9 +66,8 @@ TEST(FileOutputBuffer, Test) {<br>
// Do *not* commit buffer.<br>
}<br>
// Verify file does not exist (because buffer not committed).<br>
- bool Exists = false;<br>
- ASSERT_NO_ERROR(fs::exists(Twine(File2), Exists));<br>
- EXPECT_FALSE(Exists);<br>
+ ASSERT_EQ(fs::access(Twine(File2), fs::AccessMode::Exist),<br>
+ errc::no_such_file_or_directory);<br>
ASSERT_NO_ERROR(fs::remove(File2.str()));<br>
<br>
// TEST 3: Verify sizing down case.<br>
<br>
Modified: llvm/trunk/unittests/Support/Path.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=217625&r1=217624&r2=217625&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Support/Path.cpp (original)<br>
+++ llvm/trunk/unittests/Support/Path.cpp Thu Sep 11 15:30:02 2014<br>
@@ -361,9 +361,8 @@ TEST_F(FileSystemTest, TempFiles) {<br>
EXPECT_EQ(B.type(), fs::file_type::file_not_found);<br>
<br>
// Make sure Temp2 doesn't exist.<br>
- bool TempFileExists;<br>
- ASSERT_NO_ERROR(fs::exists(Twine(TempPath2), TempFileExists));<br>
- EXPECT_FALSE(TempFileExists);<br>
+ ASSERT_EQ(fs::access(Twine(TempPath2), sys::fs::AccessMode::Exist),<br>
+ errc::no_such_file_or_directory);<br>
<br>
SmallString<64> TempPath3;<br>
ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "", TempPath3));<br>
@@ -386,8 +385,8 @@ TEST_F(FileSystemTest, TempFiles) {<br>
ASSERT_NO_ERROR(fs::remove(Twine(TempPath2)));<br>
<br>
// Make sure Temp1 doesn't exist.<br>
- ASSERT_NO_ERROR(fs::exists(Twine(TempPath), TempFileExists));<br>
- EXPECT_FALSE(TempFileExists);<br>
+ ASSERT_EQ(fs::access(Twine(TempPath), sys::fs::AccessMode::Exist),<br>
+ errc::no_such_file_or_directory);<br>
<br>
#ifdef LLVM_ON_WIN32<br>
// Path name > 260 chars should get an error.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>