From 589d6377251d39e5f4d866bcb495bf6e547b4372 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Sun, 23 Feb 2014 13:56:14 +0000 Subject: Simplify remove, create_directory and create_directories. Before this patch they would take an boolean argument to say if the path already existed. This was redundant with the returned error_code which is able to represent that. This allowed for callers to incorrectly check only the existed flag instead of first checking the error code. Instead, pass in a boolean flag to say if the previous (non-)existence should be an error or not. Callers of the of the old simple versions are not affected. They still ignore the previous (non-)existence as they did before. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@201979 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/FileSystem.h | 46 ++++++++-------------------- lib/Support/Path.cpp | 6 ++-- lib/Support/Unix/Path.inc | 28 +++++++---------- lib/Support/Windows/Path.inc | 52 ++++++++++++-------------------- unittests/Support/Path.cpp | 41 ++++++++++++++----------- unittests/Transforms/DebugIR/DebugIR.cpp | 7 +++-- 6 files changed, 74 insertions(+), 106 deletions(-) diff --git a/include/llvm/Support/FileSystem.h b/include/llvm/Support/FileSystem.h index 1eeaa02d97..176f935933 100644 --- a/include/llvm/Support/FileSystem.h +++ b/include/llvm/Support/FileSystem.h @@ -273,32 +273,18 @@ error_code make_absolute(SmallVectorImpl &path); /// @brief Create all the non-existent directories in path. /// /// @param path Directories to create. -/// @param existed Set to true if \a path already existed, false otherwise. -/// @returns errc::success if is_directory(path) and existed have been set, -/// otherwise a platform specific error_code. -error_code create_directories(const Twine &path, bool &existed); - -/// @brief Convenience function for clients that don't need to know if the -/// directory existed or not. -inline error_code create_directories(const Twine &Path) { - bool Existed; - return create_directories(Path, Existed); -} +/// @returns errc::success if is_directory(path), otherwise a platform +/// specific error_code. If IgnoreExisting is false, also returns +/// error if the directory already existed. +error_code create_directories(const Twine &path, bool IgnoreExisting = true); /// @brief Create the directory in path. /// /// @param path Directory to create. -/// @param existed Set to true if \a path already existed, false otherwise. -/// @returns errc::success if is_directory(path) and existed have been set, -/// otherwise a platform specific error_code. -error_code create_directory(const Twine &path, bool &existed); - -/// @brief Convenience function for clients that don't need to know if the -/// directory existed or not. -inline error_code create_directory(const Twine &Path) { - bool Existed; - return create_directory(Path, Existed); -} +/// @returns errc::success if is_directory(path), otherwise a platform +/// specific error_code. If IgnoreExisting is false, also returns +/// error if the directory already existed. +error_code create_directory(const Twine &path, bool IgnoreExisting = true); /// @brief Create a hard link from \a from to \a to. /// @@ -318,18 +304,10 @@ error_code current_path(SmallVectorImpl &result); /// @brief Remove path. Equivalent to POSIX remove(). /// /// @param path Input path. -/// @param existed Set to true if \a path existed, false if it did not. -/// undefined otherwise. -/// @returns errc::success if path has been removed and existed has been -/// successfully set, otherwise a platform specific error_code. -error_code remove(const Twine &path, bool &existed); - -/// @brief Convenience function for clients that don't need to know if the file -/// existed or not. -inline error_code remove(const Twine &Path) { - bool Existed; - return remove(Path, Existed); -} +/// @returns errc::success if path has been removed or didn't exist, otherwise a +/// platform specific error code. If IgnoreNonExisting is false, also +/// returns error if the file didn't exist. +error_code remove(const Twine &path, bool IgnoreNonExisting = true); /// @brief Rename \a from to \a to. Files are renamed as if by POSIX rename(). /// diff --git a/lib/Support/Path.cpp b/lib/Support/Path.cpp index b21846c548..535ff00bae 100644 --- a/lib/Support/Path.cpp +++ b/lib/Support/Path.cpp @@ -751,12 +751,12 @@ error_code make_absolute(SmallVectorImpl &path) { "occurred above!"); } -error_code create_directories(const Twine &Path, bool &Existed) { +error_code create_directories(const Twine &Path, bool IgnoreExisting) { SmallString<128> PathStorage; StringRef P = Path.toStringRef(PathStorage); // Be optimistic and try to create the directory - error_code EC = create_directory(P, Existed); + error_code EC = create_directory(P, IgnoreExisting); // If we succeeded, or had any error other than the parent not existing, just // return it. if (EC != errc::no_such_file_or_directory) @@ -771,7 +771,7 @@ error_code create_directories(const Twine &Path, bool &Existed) { if ((EC = create_directories(Parent))) return EC; - return create_directory(P, Existed); + return create_directory(P, IgnoreExisting); } bool exists(file_status status) { diff --git a/lib/Support/Unix/Path.inc b/lib/Support/Unix/Path.inc index 20e9642897..83d2b4284a 100644 --- a/lib/Support/Unix/Path.inc +++ b/lib/Support/Unix/Path.inc @@ -164,12 +164,11 @@ retry_random_path: } case FS_Dir: { - bool Existed; - error_code EC = sys::fs::create_directory(ResultPath.begin(), Existed); - if (EC) + if (error_code EC = sys::fs::create_directory(ResultPath.begin(), false)) { + if (EC == errc::file_exists) + goto retry_random_path; return EC; - if (Existed) - goto retry_random_path; + } return error_code::success(); } } @@ -333,16 +332,14 @@ error_code current_path(SmallVectorImpl &result) { return error_code::success(); } -error_code create_directory(const Twine &path, bool &existed) { +error_code create_directory(const Twine &path, bool IgnoreExisting) { SmallString<128> path_storage; StringRef p = path.toNullTerminatedStringRef(path_storage); if (::mkdir(p.begin(), S_IRWXU | S_IRWXG) == -1) { - if (errno != errc::file_exists) + if (errno != errc::file_exists || !IgnoreExisting) return error_code(errno, system_category()); - existed = true; - } else - existed = false; + } return error_code::success(); } @@ -360,15 +357,14 @@ error_code create_hard_link(const Twine &to, const Twine &from) { return error_code::success(); } -error_code remove(const Twine &path, bool &existed) { +error_code remove(const Twine &path, bool IgnoreNonExisting) { SmallString<128> path_storage; StringRef p = path.toNullTerminatedStringRef(path_storage); struct stat buf; if (stat(p.begin(), &buf) != 0) { - if (errno != errc::no_such_file_or_directory) + if (errno != errc::no_such_file_or_directory || !IgnoreNonExisting) return error_code(errno, system_category()); - existed = false; return error_code::success(); } @@ -381,11 +377,9 @@ error_code remove(const Twine &path, bool &existed) { return make_error_code(errc::operation_not_permitted); if (::remove(p.begin()) == -1) { - if (errno != errc::no_such_file_or_directory) + if (errno != errc::no_such_file_or_directory || !IgnoreNonExisting) return error_code(errno, system_category()); - existed = false; - } else - existed = true; + } return error_code::success(); } diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc index fa7d18a711..f3c7ba1a24 100644 --- a/lib/Support/Windows/Path.inc +++ b/lib/Support/Windows/Path.inc @@ -264,7 +264,7 @@ error_code current_path(SmallVectorImpl &result) { return UTF16ToUTF8(cur_path.begin(), cur_path.size(), result); } -error_code create_directory(const Twine &path, bool &existed) { +error_code create_directory(const Twine &path, bool IgnoreExisting) { SmallString<128> path_storage; SmallVector path_utf16; @@ -274,12 +274,9 @@ error_code create_directory(const Twine &path, bool &existed) { if (!::CreateDirectoryW(path_utf16.begin(), NULL)) { error_code ec = windows_error(::GetLastError()); - if (ec == windows_error::already_exists) - existed = true; - else + if (ec != windows_error::already_exists || !IgnoreExisting) return ec; - } else - existed = false; + } return error_code::success(); } @@ -303,43 +300,34 @@ error_code create_hard_link(const Twine &to, const Twine &from) { return error_code::success(); } -error_code remove(const Twine &path, bool &existed) { +error_code remove(const Twine &path, bool IgnoreNonExisting) { SmallString<128> path_storage; SmallVector path_utf16; - file_status st; - error_code EC = status(path, st); - if (EC) { - if (EC == windows_error::file_not_found || - EC == windows_error::path_not_found) { - existed = false; - return error_code::success(); - } - return EC; + file_status ST; + if (error_code EC = status(path, ST)) { + if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting) + return EC; + return error_code::success(); } if (error_code ec = UTF8ToUTF16(path.toStringRef(path_storage), path_utf16)) return ec; - if (st.type() == file_type::directory_file) { + if (ST.type() == file_type::directory_file) { if (!::RemoveDirectoryW(c_str(path_utf16))) { - error_code ec = windows_error(::GetLastError()); - if (ec != windows_error::file_not_found) - return ec; - existed = false; - } else - existed = true; - } else { - if (!::DeleteFileW(c_str(path_utf16))) { - error_code ec = windows_error(::GetLastError()); - if (ec != windows_error::file_not_found) - return ec; - existed = false; - } else - existed = true; + error_code EC = windows_error(::GetLastError()); + if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting) + return EC; + } + return error_code::success(); + } + if (!::DeleteFileW(c_str(path_utf16))) { + error_code EC = windows_error(::GetLastError()); + if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting) + return EC; } - return error_code::success(); } diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp index 197e1c2f86..2d8141f737 100644 --- a/unittests/Support/Path.cpp +++ b/unittests/Support/Path.cpp @@ -318,8 +318,10 @@ TEST_F(FileSystemTest, TempFiles) { ::close(FD2); // Remove Temp2. - ASSERT_NO_ERROR(fs::remove(Twine(TempPath2), TempFileExists)); - EXPECT_TRUE(TempFileExists); + ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); + ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); + ASSERT_EQ(fs::remove(Twine(TempPath2), false), + errc::no_such_file_or_directory); error_code EC = fs::status(TempPath2.c_str(), B); EXPECT_EQ(EC, errc::no_such_file_or_directory); @@ -344,12 +346,10 @@ TEST_F(FileSystemTest, TempFiles) { // Remove Temp1. ::close(FileDescriptor); - ASSERT_NO_ERROR(fs::remove(Twine(TempPath), TempFileExists)); - EXPECT_TRUE(TempFileExists); + ASSERT_NO_ERROR(fs::remove(Twine(TempPath))); // Remove the hard link. - ASSERT_NO_ERROR(fs::remove(Twine(TempPath2), TempFileExists)); - EXPECT_TRUE(TempFileExists); + ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); // Make sure Temp1 doesn't exist. ASSERT_NO_ERROR(fs::exists(Twine(TempPath), TempFileExists)); @@ -368,23 +368,30 @@ TEST_F(FileSystemTest, TempFiles) { #endif } +TEST_F(FileSystemTest, CreateDir) { + ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo")); + ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo")); + ASSERT_EQ(fs::create_directory(Twine(TestDirectory) + "foo", false), + errc::file_exists); + ASSERT_NO_ERROR(fs::remove(Twine(TestDirectory) + "foo")); +} + TEST_F(FileSystemTest, DirectoryIteration) { error_code ec; for (fs::directory_iterator i(".", ec), e; i != e; i.increment(ec)) ASSERT_NO_ERROR(ec); // Create a known hierarchy to recurse over. - bool existed; - ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) - + "/recursive/a0/aa1", existed)); - ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) - + "/recursive/a0/ab1", existed)); - ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) - + "/recursive/dontlookhere/da1", existed)); - ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) - + "/recursive/z0/za1", existed)); - ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) - + "/recursive/pop/p1", existed)); + ASSERT_NO_ERROR( + fs::create_directories(Twine(TestDirectory) + "/recursive/a0/aa1")); + ASSERT_NO_ERROR( + fs::create_directories(Twine(TestDirectory) + "/recursive/a0/ab1")); + ASSERT_NO_ERROR(fs::create_directories(Twine(TestDirectory) + + "/recursive/dontlookhere/da1")); + ASSERT_NO_ERROR( + fs::create_directories(Twine(TestDirectory) + "/recursive/z0/za1")); + ASSERT_NO_ERROR( + fs::create_directories(Twine(TestDirectory) + "/recursive/pop/p1")); typedef std::vector v_t; v_t visited; for (fs::recursive_directory_iterator i(Twine(TestDirectory) diff --git a/unittests/Transforms/DebugIR/DebugIR.cpp b/unittests/Transforms/DebugIR/DebugIR.cpp index a0916a21d2..affdd762ba 100644 --- a/unittests/Transforms/DebugIR/DebugIR.cpp +++ b/unittests/Transforms/DebugIR/DebugIR.cpp @@ -55,9 +55,10 @@ void insertCUDescriptor(Module *M, StringRef File, StringRef Dir, /// Attempts to remove file at Path and returns true if it existed, or false if /// it did not. bool removeIfExists(StringRef Path) { - bool existed = false; - sys::fs::remove(Path, existed); - return existed; + // This is an approximation, on error we don't know in general if the file + // existed or not. + llvm::error_code EC = sys::fs::remove(Path, false); + return EC != llvm::errc::no_such_file_or_directory; } char * current_dir() { -- cgit v1.2.3