[llvm] r185126 - Improvements to unique_file and createUniqueDirectory.
Rafael Espindola
rafael.espindola at gmail.com
Thu Jun 27 20:48:48 PDT 2013
Author: rafael
Date: Thu Jun 27 22:48:47 2013
New Revision: 185126
URL: http://llvm.org/viewvc/llvm-project?rev=185126&view=rev
Log:
Improvements to unique_file and createUniqueDirectory.
* Don't try to create parent directories in unique_file. It had two problem:
* It violates the contract that it is atomic. If the directory creation
success and the file creation fails, we would return an error but the
file system was modified.
* When creating a temporary file clang would have to first check if the
parent directory existed or not to avoid creating one when it was not
supposed to.
* More efficient implementations of createUniqueDirectory and the unique_file
that produces only the file name. Now all 3 just call into a static
function passing what they want (name, file or directory).
Clang also has to be updated, so tests might fail if a bot picks up this commit
and not the corresponding clang one.
Modified:
llvm/trunk/lib/Support/Path.cpp
llvm/trunk/lib/Support/Unix/Path.inc
llvm/trunk/lib/Support/Windows/Path.inc
Modified: llvm/trunk/lib/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=185126&r1=185125&r2=185126&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Path.cpp (original)
+++ llvm/trunk/lib/Support/Path.cpp Thu Jun 27 22:48:47 2013
@@ -153,6 +153,18 @@ namespace {
}
} // end unnamed namespace
+enum FSEntity {
+ FS_Dir,
+ FS_File,
+ FS_Name
+};
+
+// Implemented in Unix/Path.inc and Windows/Path.inc.
+static llvm::error_code
+createUniqueEntity(const llvm::Twine &Model, int &ResultFD,
+ llvm::SmallVectorImpl<char> &ResultPath,
+ bool MakeAbsolute, unsigned Mode, FSEntity Type);
+
namespace llvm {
namespace sys {
namespace path {
@@ -625,32 +637,31 @@ bool is_relative(const Twine &path) {
namespace fs {
+// This is a mkostemps with a different pattern. Unfortunatelly OS X (ond *BSD)
+// don't have it. It might be worth experimenting with mkostemps on systems
+// that have it.
+error_code unique_file(const Twine &Model, int &ResultFD,
+ SmallVectorImpl<char> &ResultPath, bool MakeAbsolute,
+ unsigned Mode) {
+ return createUniqueEntity(Model, ResultFD, ResultPath, MakeAbsolute, Mode,
+ FS_File);
+}
+
+// This is a mktemp with a differet pattern. We use createUniqueEntity mostly
+// for consistency. It might be worth it experimenting with mktemp.
error_code unique_file(const Twine &Model, SmallVectorImpl<char> &ResultPath,
bool MakeAbsolute) {
- // FIXME: This is really inefficient. unique_path creates a path an tries to
- // open it. We should factor the code so that we just don't create/open the
- // file when we don't need it.
- int FD;
- error_code Ret = unique_file(Model, FD, ResultPath, MakeAbsolute, all_read);
- if (Ret)
- return Ret;
-
- if (close(FD))
- return error_code(errno, system_category());
-
- StringRef P(ResultPath.begin(), ResultPath.size());
- return fs::remove(P);
+ int Dummy;
+ return createUniqueEntity(Model, Dummy, ResultPath, MakeAbsolute, 0, FS_Name);
}
+// This is a mkdtemp with a different pattern. We use createUniqueEntity mostly
+// for consistency. It might be worth it experimenting with mkdtemp.
error_code createUniqueDirectory(const Twine &Prefix,
SmallVectorImpl<char> &ResultPath) {
- // FIXME: This is double inefficient. We compute a unique file name, created
- // it, delete it and keep only the directory.
- error_code EC = unique_file(Prefix + "-%%%%%%/dummy", ResultPath);
- if (EC)
- return EC;
- path::remove_filename(ResultPath);
- return error_code::success();
+ int Dummy;
+ return createUniqueEntity(Prefix + "-%%%%%%", Dummy, ResultPath,
+ true, 0, FS_Dir);
}
error_code make_absolute(SmallVectorImpl<char> &path) {
Modified: llvm/trunk/lib/Support/Unix/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=185126&r1=185125&r2=185126&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Path.inc (original)
+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Jun 27 22:48:47 2013
@@ -110,6 +110,76 @@ namespace {
}
}
+static error_code createUniqueEntity(const Twine &Model, int &ResultFD,
+ SmallVectorImpl<char> &ResultPath,
+ bool MakeAbsolute, unsigned Mode,
+ FSEntity Type) {
+ SmallString<128> ModelStorage;
+ Model.toVector(ModelStorage);
+
+ if (MakeAbsolute) {
+ // Make model absolute by prepending a temp directory if it's not already.
+ bool absolute = sys::path::is_absolute(Twine(ModelStorage));
+ if (!absolute) {
+ SmallString<128> TDir;
+ if (error_code ec = TempDir(TDir)) return ec;
+ sys::path::append(TDir, Twine(ModelStorage));
+ ModelStorage.swap(TDir);
+ }
+ }
+
+ // From here on, DO NOT modify model. It may be needed if the randomly chosen
+ // path already exists.
+ ResultPath = ModelStorage;
+ // Null terminate.
+ ResultPath.push_back(0);
+ ResultPath.pop_back();
+
+retry_random_path:
+ // Replace '%' with random chars.
+ for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
+ if (ModelStorage[i] == '%')
+ ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
+ }
+
+ // Try to open + create the file.
+ switch (Type) {
+ case FS_File: {
+ int RandomFD = ::open(ResultPath.begin(), O_RDWR | O_CREAT | O_EXCL, Mode);
+ if (RandomFD == -1) {
+ int SavedErrno = errno;
+ // If the file existed, try again, otherwise, error.
+ if (SavedErrno == errc::file_exists)
+ goto retry_random_path;
+ return error_code(SavedErrno, system_category());
+ }
+
+ ResultFD = RandomFD;
+ return error_code::success();
+ }
+
+ case FS_Name: {
+ bool Exists;
+ error_code EC = sys::fs::exists(ResultPath.begin(), Exists);
+ if (EC)
+ return EC;
+ if (Exists)
+ goto retry_random_path;
+ return error_code::success();
+ }
+
+ case FS_Dir: {
+ bool Existed;
+ error_code EC = sys::fs::create_directory(ResultPath.begin(), Existed);
+ if (EC)
+ return EC;
+ if (Existed)
+ goto retry_random_path;
+ return error_code::success();
+ }
+ }
+}
+
namespace llvm {
namespace sys {
namespace fs {
@@ -564,92 +634,6 @@ error_code setLastModificationAndAccessT
return error_code::success();
}
-error_code unique_file(const Twine &model, int &result_fd,
- SmallVectorImpl<char> &result_path,
- bool makeAbsolute, unsigned mode) {
- SmallString<128> Model;
- model.toVector(Model);
- // Null terminate.
- Model.c_str();
-
- if (makeAbsolute) {
- // Make model absolute by prepending a temp directory if it's not already.
- bool absolute = path::is_absolute(Twine(Model));
- if (!absolute) {
- SmallString<128> TDir;
- if (error_code ec = TempDir(TDir)) return ec;
- path::append(TDir, Twine(Model));
- Model.swap(TDir);
- }
- }
-
- // From here on, DO NOT modify model. It may be needed if the randomly chosen
- // path already exists.
- SmallString<128> RandomPath = Model;
-
-retry_random_path:
- // Replace '%' with random chars.
- for (unsigned i = 0, e = Model.size(); i != e; ++i) {
- if (Model[i] == '%')
- RandomPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
- }
-
- // Make sure we don't fall into an infinite loop by constantly trying
- // to create the parent path.
- bool TriedToCreateParent = false;
-
- // Try to open + create the file.
-rety_open_create:
- int RandomFD = ::open(RandomPath.c_str(), O_RDWR | O_CREAT | O_EXCL, mode);
- if (RandomFD == -1) {
- int SavedErrno = errno;
- // If the file existed, try again, otherwise, error.
- if (SavedErrno == errc::file_exists)
- goto retry_random_path;
- // If path prefix doesn't exist, try to create it.
- if (SavedErrno == errc::no_such_file_or_directory && !TriedToCreateParent) {
- TriedToCreateParent = true;
- StringRef p(RandomPath);
- SmallString<64> dir_to_create;
- for (path::const_iterator i = path::begin(p),
- e = --path::end(p); i != e; ++i) {
- path::append(dir_to_create, *i);
- bool Exists;
- if (error_code ec = exists(Twine(dir_to_create), Exists)) return ec;
- if (!Exists) {
- // Don't try to create network paths.
- if (i->size() > 2 && (*i)[0] == '/' &&
- (*i)[1] == '/' &&
- (*i)[2] != '/')
- return make_error_code(errc::no_such_file_or_directory);
- if (::mkdir(dir_to_create.c_str(), 0700) == -1 &&
- errno != errc::file_exists)
- return error_code(errno, system_category());
- }
- }
- goto rety_open_create;
- }
-
- return error_code(SavedErrno, system_category());
- }
-
- // Make the path absolute.
- char real_path_buff[PATH_MAX + 1];
- if (realpath(RandomPath.c_str(), real_path_buff) == NULL) {
- int error = errno;
- ::close(RandomFD);
- ::unlink(RandomPath.c_str());
- return error_code(error, system_category());
- }
-
- result_path.clear();
- StringRef d(real_path_buff);
- result_path.append(d.begin(), d.end());
-
- result_fd = RandomFD;
- return error_code::success();
-}
-
error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
AutoFD ScopedFD(FD);
if (!CloseFD)
Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=185126&r1=185125&r2=185126&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Jun 27 22:48:47 2013
@@ -124,6 +124,140 @@ namespace {
}
}
+// FIXME: mode should be used here and default to user r/w only,
+// it currently comes in as a UNIX mode.
+static error_code createUniqueEntity(const Twine &model, int &result_fd,
+ SmallVectorImpl<char> &result_path,
+ bool makeAbsolute, unsigned mode,
+ FSEntity Type) {
+ // Use result_path as temp storage.
+ result_path.set_size(0);
+ StringRef m = model.toStringRef(result_path);
+
+ SmallVector<wchar_t, 128> model_utf16;
+ if (error_code ec = UTF8ToUTF16(m, model_utf16)) return ec;
+
+ if (makeAbsolute) {
+ // Make model absolute by prepending a temp directory if it's not already.
+ bool absolute = sys::path::is_absolute(m);
+
+ if (!absolute) {
+ SmallVector<wchar_t, 64> temp_dir;
+ if (error_code ec = TempDir(temp_dir)) return ec;
+ // Handle c: by removing it.
+ if (model_utf16.size() > 2 && model_utf16[1] == L':') {
+ model_utf16.erase(model_utf16.begin(), model_utf16.begin() + 2);
+ }
+ model_utf16.insert(model_utf16.begin(), temp_dir.begin(), temp_dir.end());
+ }
+ }
+
+ // Replace '%' with random chars. From here on, DO NOT modify model. It may be
+ // needed if the randomly chosen path already exists.
+ SmallVector<wchar_t, 128> random_path_utf16;
+
+ // Get a Crypto Provider for CryptGenRandom.
+ HCRYPTPROV HCPC;
+ if (!::CryptAcquireContextW(&HCPC,
+ NULL,
+ NULL,
+ PROV_RSA_FULL,
+ CRYPT_VERIFYCONTEXT))
+ return windows_error(::GetLastError());
+ ScopedCryptContext CryptoProvider(HCPC);
+
+retry_random_path:
+ random_path_utf16.set_size(0);
+ for (SmallVectorImpl<wchar_t>::const_iterator i = model_utf16.begin(),
+ e = model_utf16.end();
+ i != e; ++i) {
+ if (*i == L'%') {
+ BYTE val = 0;
+ if (!::CryptGenRandom(CryptoProvider, 1, &val))
+ return windows_error(::GetLastError());
+ random_path_utf16.push_back("0123456789abcdef"[val & 15]);
+ }
+ else
+ random_path_utf16.push_back(*i);
+ }
+ // Make random_path_utf16 null terminated.
+ random_path_utf16.push_back(0);
+ random_path_utf16.pop_back();
+
+ HANDLE TempFileHandle;
+
+ switch (Type) {
+ case FS_File: {
+ // Try to create + open the path.
+ TempFileHandle =
+ ::CreateFileW(random_path_utf16.begin(), GENERIC_READ | GENERIC_WRITE,
+ FILE_SHARE_READ, NULL,
+ // Return ERROR_FILE_EXISTS if the file
+ // already exists.
+ CREATE_NEW, FILE_ATTRIBUTE_TEMPORARY, NULL);
+ if (TempFileHandle == INVALID_HANDLE_VALUE) {
+ // If the file existed, try again, otherwise, error.
+ error_code ec = windows_error(::GetLastError());
+ if (ec == windows_error::file_exists)
+ goto retry_random_path;
+
+ return ec;
+ }
+
+ // Convert the Windows API file handle into a C-runtime handle.
+ int fd = ::_open_osfhandle(intptr_t(TempFileHandle), 0);
+ if (fd == -1) {
+ ::CloseHandle(TempFileHandle);
+ ::DeleteFileW(random_path_utf16.begin());
+ // MSDN doesn't say anything about _open_osfhandle setting errno or
+ // GetLastError(), so just return invalid_handle.
+ return windows_error::invalid_handle;
+ }
+
+ result_fd = fd;
+ break;
+ }
+
+ case FS_Name: {
+ DWORD attributes = ::GetFileAttributesW(random_path_utf16.begin());
+ if (attributes != INVALID_FILE_ATTRIBUTES)
+ goto retry_random_path;
+ error_code EC = make_error_code(windows_error(::GetLastError()));
+ if (EC != windows_error::file_not_found &&
+ EC != windows_error::path_not_found)
+ return EC;
+ break;
+ }
+
+ case FS_Dir:
+ if (!::CreateDirectoryW(random_path_utf16.begin(), NULL)) {
+ error_code EC = windows_error(::GetLastError());
+ if (EC != windows_error::already_exists)
+ return EC;
+ goto retry_random_path;
+ }
+ break;
+ }
+
+ // Set result_path to the utf-8 representation of the path.
+ if (error_code ec = UTF16ToUTF8(random_path_utf16.begin(),
+ random_path_utf16.size(), result_path)) {
+ switch (Type) {
+ case FS_File:
+ ::CloseHandle(TempFileHandle);
+ ::DeleteFileW(random_path_utf16.begin());
+ case FS_Name:
+ break;
+ case FS_Dir:
+ ::RemoveDirectoryW(random_path_utf16.begin());
+ break;
+ }
+ return ec;
+ }
+
+ return error_code::success();
+}
+
namespace llvm {
namespace sys {
namespace fs {
@@ -604,141 +738,6 @@ error_code setLastModificationAndAccessT
return error_code::success();
}
-// FIXME: mode should be used here and default to user r/w only,
-// it currently comes in as a UNIX mode.
-error_code unique_file(const Twine &model, int &result_fd,
- SmallVectorImpl<char> &result_path,
- bool makeAbsolute, unsigned mode) {
- // Use result_path as temp storage.
- result_path.set_size(0);
- StringRef m = model.toStringRef(result_path);
-
- SmallVector<wchar_t, 128> model_utf16;
- if (error_code ec = UTF8ToUTF16(m, model_utf16)) return ec;
-
- if (makeAbsolute) {
- // Make model absolute by prepending a temp directory if it's not already.
- bool absolute = path::is_absolute(m);
-
- if (!absolute) {
- SmallVector<wchar_t, 64> temp_dir;
- if (error_code ec = TempDir(temp_dir)) return ec;
- // Handle c: by removing it.
- if (model_utf16.size() > 2 && model_utf16[1] == L':') {
- model_utf16.erase(model_utf16.begin(), model_utf16.begin() + 2);
- }
- model_utf16.insert(model_utf16.begin(), temp_dir.begin(), temp_dir.end());
- }
- }
-
- // Replace '%' with random chars. From here on, DO NOT modify model. It may be
- // needed if the randomly chosen path already exists.
- SmallVector<wchar_t, 128> random_path_utf16;
-
- // Get a Crypto Provider for CryptGenRandom.
- HCRYPTPROV HCPC;
- if (!::CryptAcquireContextW(&HCPC,
- NULL,
- NULL,
- PROV_RSA_FULL,
- CRYPT_VERIFYCONTEXT))
- return windows_error(::GetLastError());
- ScopedCryptContext CryptoProvider(HCPC);
-
-retry_random_path:
- random_path_utf16.set_size(0);
- for (SmallVectorImpl<wchar_t>::const_iterator i = model_utf16.begin(),
- e = model_utf16.end();
- i != e; ++i) {
- if (*i == L'%') {
- BYTE val = 0;
- if (!::CryptGenRandom(CryptoProvider, 1, &val))
- return windows_error(::GetLastError());
- random_path_utf16.push_back("0123456789abcdef"[val & 15]);
- }
- else
- random_path_utf16.push_back(*i);
- }
- // Make random_path_utf16 null terminated.
- random_path_utf16.push_back(0);
- random_path_utf16.pop_back();
-
- // Make sure we don't fall into an infinite loop by constantly trying
- // to create the parent path.
- bool TriedToCreateParent = false;
-
- // Try to create + open the path.
-retry_create_file:
- HANDLE TempFileHandle = ::CreateFileW(random_path_utf16.begin(),
- GENERIC_READ | GENERIC_WRITE,
- FILE_SHARE_READ,
- NULL,
- // Return ERROR_FILE_EXISTS if the file
- // already exists.
- CREATE_NEW,
- FILE_ATTRIBUTE_TEMPORARY,
- NULL);
- if (TempFileHandle == INVALID_HANDLE_VALUE) {
- // If the file existed, try again, otherwise, error.
- error_code ec = windows_error(::GetLastError());
- if (ec == windows_error::file_exists)
- goto retry_random_path;
- // Check for non-existing parent directories.
- if (ec == windows_error::path_not_found && !TriedToCreateParent) {
- TriedToCreateParent = true;
-
- // Create the directories using result_path as temp storage.
- if (error_code ec = UTF16ToUTF8(random_path_utf16.begin(),
- random_path_utf16.size(), result_path))
- return ec;
- StringRef p(result_path.begin(), result_path.size());
- SmallString<64> dir_to_create;
- for (path::const_iterator i = path::begin(p),
- e = --path::end(p); i != e; ++i) {
- path::append(dir_to_create, *i);
- bool Exists;
- if (error_code ec = exists(Twine(dir_to_create), Exists)) return ec;
- if (!Exists) {
- // If c: doesn't exist, bail.
- if (i->endswith(":"))
- return ec;
-
- SmallVector<wchar_t, 64> dir_to_create_utf16;
- if (error_code ec = UTF8ToUTF16(dir_to_create, dir_to_create_utf16))
- return ec;
-
- // Create the directory.
- if (!::CreateDirectoryW(dir_to_create_utf16.begin(), NULL))
- return windows_error(::GetLastError());
- }
- }
- goto retry_create_file;
- }
- return ec;
- }
-
- // Set result_path to the utf-8 representation of the path.
- if (error_code ec = UTF16ToUTF8(random_path_utf16.begin(),
- random_path_utf16.size(), result_path)) {
- ::CloseHandle(TempFileHandle);
- ::DeleteFileW(random_path_utf16.begin());
- return ec;
- }
-
- // Convert the Windows API file handle into a C-runtime handle.
- int fd = ::_open_osfhandle(intptr_t(TempFileHandle), 0);
- if (fd == -1) {
- ::CloseHandle(TempFileHandle);
- ::DeleteFileW(random_path_utf16.begin());
- // MSDN doesn't say anything about _open_osfhandle setting errno or
- // GetLastError(), so just return invalid_handle.
- return windows_error::invalid_handle;
- }
-
- result_fd = fd;
- return error_code::success();
-}
-
error_code get_magic(const Twine &path, uint32_t len,
SmallVectorImpl<char> &result) {
SmallString<128> path_storage;
More information about the llvm-commits
mailing list