diff options
-rw-r--r-- | llvm/include/llvm/Support/FileSystem.h | 31 | ||||
-rw-r--r-- | llvm/tools/llvm-cov/CodeCoverage.cpp | 12 | ||||
-rw-r--r-- | llvm/unittests/Support/Path.cpp | 113 |
3 files changed, 106 insertions, 50 deletions
diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h index a9b02d9f4e4..766d332195d 100644 --- a/llvm/include/llvm/Support/FileSystem.h +++ b/llvm/include/llvm/Support/FileSystem.h @@ -956,14 +956,16 @@ public: SmallString<128> path_storage; ec = detail::directory_iterator_construct( *State, path.toStringRef(path_storage), FollowSymlinks); + update_error_code_for_current_entry(ec); } explicit directory_iterator(const directory_entry &de, std::error_code &ec, bool follow_symlinks = true) : FollowSymlinks(follow_symlinks) { State = std::make_shared<detail::DirIterState>(); - ec = - detail::directory_iterator_construct(*State, de.path(), FollowSymlinks); + ec = detail::directory_iterator_construct( + *State, de.path(), FollowSymlinks); + update_error_code_for_current_entry(ec); } /// Construct end iterator. @@ -972,6 +974,7 @@ public: // No operator++ because we need error_code. directory_iterator &increment(std::error_code &ec) { ec = directory_iterator_increment(*State); + update_error_code_for_current_entry(ec); return *this; } @@ -993,6 +996,24 @@ public: } // Other members as required by // C++ Std, 24.1.1 Input iterators [input.iterators] + +private: + // Checks if current entry is valid and populates error code. For example, + // current entry may not exist due to broken symbol links. + void update_error_code_for_current_entry(std::error_code &ec) { + // Bail out if error has already occured earlier to avoid overwriting it. + if (ec) + return; + + // Empty directory entry is used to mark the end of an interation, it's not + // an error. + if (State->CurrentEntry == directory_entry()) + return; + + ErrorOr<basic_file_status> status = State->CurrentEntry.status(); + if (!status) + ec = status.getError(); + } }; namespace detail { @@ -1030,11 +1051,9 @@ public: if (State->HasNoPushRequest) State->HasNoPushRequest = false; else { - ErrorOr<basic_file_status> st = State->Stack.top()->status(); - if (!st) return *this; - if (is_directory(*st)) { + ErrorOr<basic_file_status> status = State->Stack.top()->status(); + if (status && is_directory(*status)) { State->Stack.push(directory_iterator(*State->Stack.top(), ec, Follow)); - if (ec) return *this; if (State->Stack.top() != end_itr) { ++State->Level; return *this; diff --git a/llvm/tools/llvm-cov/CodeCoverage.cpp b/llvm/tools/llvm-cov/CodeCoverage.cpp index a1f2fd2b9d4..2cfe02b3eb0 100644 --- a/llvm/tools/llvm-cov/CodeCoverage.cpp +++ b/llvm/tools/llvm-cov/CodeCoverage.cpp @@ -199,7 +199,7 @@ void CodeCoverageTool::collectPaths(const std::string &Path) { if (PathRemapping) addCollectedPath(Path); else - error("Missing source file", Path); + warning("Source file doesn't exist, proceeded by ignoring it.", Path); return; } @@ -211,12 +211,16 @@ void CodeCoverageTool::collectPaths(const std::string &Path) { if (llvm::sys::fs::is_directory(Status)) { std::error_code EC; for (llvm::sys::fs::recursive_directory_iterator F(Path, EC), E; - F != E && !EC; F.increment(EC)) { + F != E; F.increment(EC)) { + + if (EC) { + warning(EC.message(), F->path()); + continue; + } + if (llvm::sys::fs::is_regular_file(F->path())) addCollectedPath(F->path()); } - if (EC) - warning(EC.message(), Path); } } diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index f30ef69b565..8b053b33ec2 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -885,58 +885,91 @@ TEST_F(FileSystemTest, BrokenSymlinkDirectoryIteration) { fs::create_link("no_such_file", Twine(TestDirectory) + "/symlink/e")); typedef std::vector<std::string> v_t; - v_t visited; - - // The directory iterator doesn't stat the file, so we should be able to - // iterate over the whole directory. + v_t VisitedNonBrokenSymlinks; + v_t VisitedBrokenSymlinks; std::error_code ec; + + // Broken symbol links are expected to throw an error. for (fs::directory_iterator i(Twine(TestDirectory) + "/symlink", ec), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); + continue; + } + ASSERT_NO_ERROR(ec); - visited.push_back(path::filename(i->path())); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - v_t expected = {"a", "b", "c", "d", "e"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); - visited.clear(); - - // The recursive directory iterator has to stat the file, so we need to skip - // the broken symlinks. - for (fs::recursive_directory_iterator - i(Twine(TestDirectory) + "/symlink", ec), - e; - i != e; i.increment(ec)) { - ASSERT_NO_ERROR(ec); - - ErrorOr<fs::basic_file_status> status = i->status(); - if (status.getError() == - std::make_error_code(std::errc::no_such_file_or_directory)) { - i.no_push(); + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + v_t ExpectedNonBrokenSymlinks = {"b", "d"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + v_t ExpectedBrokenSymlinks = {"a", "c", "e"}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); + + // Broken symbol links are expected to throw an error. + for (fs::recursive_directory_iterator i( + Twine(TestDirectory) + "/symlink", ec), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); continue; } - visited.push_back(path::filename(i->path())); + ASSERT_NO_ERROR(ec); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - expected = {"b", "bb", "d", "da", "dd", "ddd", "ddd"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); - visited.clear(); - - // This recursive directory iterator doesn't follow symlinks, so we don't need - // to skip them. - for (fs::recursive_directory_iterator - i(Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false), - e; + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + ExpectedNonBrokenSymlinks = {"b", "bb", "d", "da", "dd", "ddd", "ddd"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + ExpectedBrokenSymlinks = {"a", "ba", "bc", "c", "e"}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); + + for (fs::recursive_directory_iterator i( + Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); + continue; + } + ASSERT_NO_ERROR(ec); - visited.push_back(path::filename(i->path())); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - expected = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd", "ddd", "e"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + ExpectedNonBrokenSymlinks = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd", + "ddd", "e"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + ExpectedBrokenSymlinks = {}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/symlink")); } |