[llvm] r343460 - [Support] Listing a directory containing dangling symlinks is not an error.
Sam McCall via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 1 05:17:05 PDT 2018
Author: sammccall
Date: Mon Oct 1 05:17:05 2018
New Revision: 343460
URL: http://llvm.org/viewvc/llvm-project?rev=343460&view=rev
Log:
[Support] Listing a directory containing dangling symlinks is not an error.
Summary:
Reporting this as an error required stat()ing every file, as well as seeming
semantically questionable.
Reviewers: vsk, bkramer
Subscribers: mgrang, kristina, llvm-commits, liaoyuke
Differential Revision: https://reviews.llvm.org/D52648
Modified:
llvm/trunk/include/llvm/Support/FileSystem.h
llvm/trunk/tools/llvm-cov/CodeCoverage.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=343460&r1=343459&r2=343460&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Mon Oct 1 05:17:05 2018
@@ -1182,7 +1182,6 @@ 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,
@@ -1191,7 +1190,6 @@ public:
State = std::make_shared<detail::DirIterState>();
ec = detail::directory_iterator_construct(
*State, de.path(), FollowSymlinks);
- update_error_code_for_current_entry(ec);
}
/// Construct end iterator.
@@ -1200,7 +1198,6 @@ 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;
}
@@ -1220,26 +1217,6 @@ public:
bool operator!=(const directory_iterator &RHS) const {
return !(*this == RHS);
}
- // 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 {
@@ -1277,8 +1254,15 @@ public:
if (State->HasNoPushRequest)
State->HasNoPushRequest = false;
else {
- ErrorOr<basic_file_status> status = State->Stack.top()->status();
- if (status && is_directory(*status)) {
+ file_type type = State->Stack.top()->type();
+ if (type == file_type::symlink_file && Follow) {
+ // Resolve the symlink: is it a directory to recurse into?
+ ErrorOr<basic_file_status> status = State->Stack.top()->status();
+ if (status)
+ type = status->type();
+ // Otherwise broken symlink, and we'll continue.
+ }
+ if (type == file_type::directory_file) {
State->Stack.push(directory_iterator(*State->Stack.top(), ec, Follow));
if (State->Stack.top() != end_itr) {
++State->Level;
@@ -1342,8 +1326,6 @@ public:
bool operator!=(const recursive_directory_iterator &RHS) const {
return !(*this == RHS);
}
- // Other members as required by
- // C++ Std, 24.1.1 Input iterators [input.iterators]
};
/// @}
Modified: llvm/trunk/tools/llvm-cov/CodeCoverage.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cov/CodeCoverage.cpp?rev=343460&r1=343459&r2=343460&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cov/CodeCoverage.cpp (original)
+++ llvm/trunk/tools/llvm-cov/CodeCoverage.cpp Mon Oct 1 05:17:05 2018
@@ -215,12 +215,13 @@ void CodeCoverageTool::collectPaths(cons
for (llvm::sys::fs::recursive_directory_iterator F(Path, EC), E;
F != E; F.increment(EC)) {
- if (EC) {
- warning(EC.message(), F->path());
+ auto Status = F->status();
+ if (!Status) {
+ warning(Status.getError().message(), F->path());
continue;
}
- if (llvm::sys::fs::is_regular_file(F->path()))
+ if (Status->type() == llvm::sys::fs::file_type::regular_file)
addCollectedPath(F->path());
}
}
Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=343460&r1=343459&r2=343460&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Mon Oct 1 05:17:05 2018
@@ -883,87 +883,62 @@ TEST_F(FileSystemTest, BrokenSymlinkDire
v_t VisitedNonBrokenSymlinks;
v_t VisitedBrokenSymlinks;
std::error_code ec;
+ using testing::UnorderedElementsAre;
+ using testing::UnorderedElementsAreArray;
// 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)) {
+ ASSERT_NO_ERROR(ec);
+ if (i->status().getError() ==
+ std::make_error_code(std::errc::no_such_file_or_directory)) {
VisitedBrokenSymlinks.push_back(path::filename(i->path()));
continue;
}
-
- ASSERT_NO_ERROR(ec);
VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- llvm::sort(VisitedNonBrokenSymlinks);
- llvm::sort(VisitedBrokenSymlinks);
- v_t ExpectedNonBrokenSymlinks = {"b", "d"};
- ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
- ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
- VisitedNonBrokenSymlinks.end(),
- ExpectedNonBrokenSymlinks.begin()));
+ EXPECT_THAT(VisitedNonBrokenSymlinks, UnorderedElementsAre("b", "d"));
VisitedNonBrokenSymlinks.clear();
- v_t ExpectedBrokenSymlinks = {"a", "c", "e"};
- ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
- ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
- VisitedBrokenSymlinks.end(),
- ExpectedBrokenSymlinks.begin()));
+ EXPECT_THAT(VisitedBrokenSymlinks, UnorderedElementsAre("a", "c", "e"));
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)) {
+ ASSERT_NO_ERROR(ec);
+ if (i->status().getError() ==
+ std::make_error_code(std::errc::no_such_file_or_directory)) {
VisitedBrokenSymlinks.push_back(path::filename(i->path()));
continue;
}
-
- ASSERT_NO_ERROR(ec);
VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- llvm::sort(VisitedNonBrokenSymlinks);
- llvm::sort(VisitedBrokenSymlinks);
- ExpectedNonBrokenSymlinks = {"b", "bb", "d", "da", "dd", "ddd", "ddd"};
- ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
- ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
- VisitedNonBrokenSymlinks.end(),
- ExpectedNonBrokenSymlinks.begin()));
+ EXPECT_THAT(VisitedNonBrokenSymlinks,
+ UnorderedElementsAre("b", "bb", "d", "da", "dd", "ddd", "ddd"));
VisitedNonBrokenSymlinks.clear();
- ExpectedBrokenSymlinks = {"a", "ba", "bc", "c", "e"};
- ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
- ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
- VisitedBrokenSymlinks.end(),
- ExpectedBrokenSymlinks.begin()));
+ EXPECT_THAT(VisitedBrokenSymlinks,
+ UnorderedElementsAre("a", "ba", "bc", "c", "e"));
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)) {
+ ASSERT_NO_ERROR(ec);
+ if (i->status().getError() ==
+ std::make_error_code(std::errc::no_such_file_or_directory)) {
VisitedBrokenSymlinks.push_back(path::filename(i->path()));
continue;
}
-
- ASSERT_NO_ERROR(ec);
VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- llvm::sort(VisitedNonBrokenSymlinks);
- llvm::sort(VisitedBrokenSymlinks);
- 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()));
+ EXPECT_THAT(VisitedNonBrokenSymlinks,
+ UnorderedElementsAreArray({"a", "b", "ba", "bb", "bc", "c", "d",
+ "da", "dd", "ddd", "e"}));
VisitedNonBrokenSymlinks.clear();
- ExpectedBrokenSymlinks = {};
- ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
- ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
- VisitedBrokenSymlinks.end(),
- ExpectedBrokenSymlinks.begin()));
+ EXPECT_THAT(VisitedBrokenSymlinks, UnorderedElementsAre());
VisitedBrokenSymlinks.clear();
ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/symlink"));
More information about the llvm-commits
mailing list