From b946e380d4e414cec85082ebe67c8ffed6579277 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 16 May 2018 22:20:49 +0300 Subject: Add ignore_dangling parameter to dir_iterator() ctor --- libbutl/filesystem.cxx | 69 +++++++++++++++++++++++++++++++------------ libbutl/filesystem.ixx | 4 +-- libbutl/filesystem.mxx | 25 +++++++++++++--- tests/dir-iterator/driver.cxx | 54 ++++++++++++++++++++++++--------- tests/dir-iterator/testscript | 19 ++++++++++-- tests/link/driver.cxx | 4 +-- tests/wildcard/testscript | 16 +++++++++- 7 files changed, 147 insertions(+), 44 deletions(-) diff --git a/libbutl/filesystem.cxx b/libbutl/filesystem.cxx index 2650c25..d1b0424 100644 --- a/libbutl/filesystem.cxx +++ b/libbutl/filesystem.cxx @@ -285,7 +285,7 @@ namespace butl // An nftw()-based implementation (for platforms that support it) // might be a faster way. // - for (const dir_entry& de: dir_iterator (p)) + for (const dir_entry& de: dir_iterator (p, false /* ignore_dangling */)) { path ep (p / de.path ()); //@@ Would be good to reuse the buffer. @@ -1083,10 +1083,25 @@ namespace butl h_ = x.h_; x.h_ = nullptr; + + ignore_dangling_ = x.ignore_dangling_; } return *this; } + static inline entry_type + type (const struct stat& s) noexcept + { + if (S_ISREG (s.st_mode)) + return entry_type::regular; + else if (S_ISDIR (s.st_mode)) + return entry_type::directory; + else if (S_ISLNK (s.st_mode)) + return entry_type::symlink; + else + return entry_type::other; + } + entry_type dir_entry:: type (bool link) const { @@ -1095,22 +1110,9 @@ namespace butl if ((link ? stat (p.string ().c_str (), &s) : lstat (p.string ().c_str (), &s)) != 0) - { throw_generic_error (errno); - } - - entry_type r; - - if (S_ISREG (s.st_mode)) - r = entry_type::regular; - else if (S_ISDIR (s.st_mode)) - r = entry_type::directory; - else if (S_ISLNK (s.st_mode)) - r = entry_type::symlink; - else - r = entry_type::other; - return r; + return butl::type (s); } // dir_iterator @@ -1121,7 +1123,8 @@ namespace butl }; dir_iterator:: - dir_iterator (const dir_path& d) + dir_iterator (const dir_path& d, bool ignore_dangling) + : ignore_dangling_ (ignore_dangling) { unique_ptr h (opendir (d.string ().c_str ())); h_ = h.get (); @@ -1189,12 +1192,33 @@ namespace butl // Skip '.' and '..'. // - if (p.current () || p.parent ()) + if (p.current () || p.parent ()) continue; e_.p_ = move (p); e_.t_ = d_type (de, nullptr); e_.lt_ = entry_type::unknown; + + // If requested, we ignore dangling symlinks, skipping ones with + // non-existing or inaccessible targets. + // + // Note that ltype () can potentially lstat() and so throw. + // + if (ignore_dangling_ && e_.ltype () == entry_type::symlink) + { + struct stat s; + path p (e_.base () / e_.path ()); + + if (stat (p.string ().c_str (), &s) != 0) + { + if (errno == ENOENT || errno == ENOTDIR || errno == EACCES) + continue; + + throw_generic_error (errno); + } + + e_.lt_ = type (s); // While at it, set the target type. + } } else if (errno == 0) { @@ -1233,6 +1257,8 @@ namespace butl h_ = x.h_; x.h_ = -1; + + ignore_dangling_ = x.ignore_dangling_; } return *this; } @@ -1283,7 +1309,8 @@ namespace butl }; dir_iterator:: - dir_iterator (const dir_path& d) + dir_iterator (const dir_path& d, bool ignore_dangling) + : ignore_dangling_ (ignore_dangling) { auto_dir h (h_); e_.b_ = d; // Used by next() to call _findfirst(). @@ -1801,7 +1828,11 @@ namespace butl if (!preopen || preopen_ (p)) { dir_path d (start_ / p); - i = dir_iterator (!d.empty () ? d : dir_path (".")); + + // If we follow symlinks, then we ignore the dangling ones. + // + i = dir_iterator (!d.empty () ? d : dir_path ("."), + follow_symlinks_); } iters_.emplace_back (move (i), move (p)); diff --git a/libbutl/filesystem.ixx b/libbutl/filesystem.ixx index 170c108..e5c1298 100644 --- a/libbutl/filesystem.ixx +++ b/libbutl/filesystem.ixx @@ -9,7 +9,7 @@ namespace butl { // @@ Could 0 size be a valid and faster way? // - return dir_iterator (d) == dir_iterator (); + return dir_iterator (d, false /* ignore_dangling */) == dir_iterator (); } inline bool @@ -126,7 +126,7 @@ namespace butl // inline dir_iterator:: dir_iterator (dir_iterator&& x) noexcept - : e_ (std::move (x.e_)), h_ (x.h_) + : e_ (std::move (x.e_)), h_ (x.h_), ignore_dangling_ (x.ignore_dangling_) { #ifndef _WIN32 x.h_ = nullptr; diff --git a/libbutl/filesystem.mxx b/libbutl/filesystem.mxx index ff9a4a6..da5ab59 100644 --- a/libbutl/filesystem.mxx +++ b/libbutl/filesystem.mxx @@ -251,9 +251,17 @@ LIBBUTL_MODEXPORT namespace butl // deleted at a later stage, then the filesystem API functions may fail // when encounter such a symlink. This includes rmsymlink(). // - // - Symlinks are not visible when iterating over a directory with - // dir_iterator. As a result, a directory that contains symlinks can not - // be recursively deleted. + // - Dangling symlinks are not visible when iterating over a directory with + // dir_iterator. As a result, a directory that contains such symlinks can + // not be recursively deleted. + // + // @@ Note that the above restrictions seems to be Wine-specific (as of + // 2.20). It is probably make sense to properly support directory + // symlinks when run natively. + // + // - Symlinks that refer to existing targets are recognized as ordinary + // directories by dir_iterator. As a result rmdir_r() function removes the + // target directories content, rather then symlinks entries. // LIBBUTL_SYMEXPORT void mksymlink (const path& target, const path& link, bool dir = false); @@ -629,8 +637,15 @@ LIBBUTL_MODEXPORT namespace butl ~dir_iterator (); dir_iterator () = default; + // If it is requested to ignore dangling symlinks, then the increment + // operator will skip symlinks that refer to non-existing or inaccessible + // targets. That implies that it will always try to stat() symlinks. + // + // Note that we currently do not fully support symlinks on Windows, so the + // ignore_dangling argument is noop there (see mksymlink() for details). + // explicit - dir_iterator (const dir_path&); + dir_iterator (const dir_path&, bool ignore_dangling); dir_iterator (const dir_iterator&) = delete; dir_iterator& operator= (const dir_iterator&) = delete; @@ -658,6 +673,8 @@ LIBBUTL_MODEXPORT namespace butl #else intptr_t h_ = -1; #endif + + bool ignore_dangling_; }; // Range-based for loop support. diff --git a/tests/dir-iterator/driver.cxx b/tests/dir-iterator/driver.cxx index 1adef6f..bad9e58 100644 --- a/tests/dir-iterator/driver.cxx +++ b/tests/dir-iterator/driver.cxx @@ -41,45 +41,71 @@ operator<< (ostream& os, entry_type e) return os << entry_type_string[static_cast (e)]; } -// @@ Should we make the test silent unless -v arg passed. In silen mode could -// compare the output with a set of predefined dir entries. +// Usage: argv[0] [-v] [-i] +// +// Iterates over a directory filesystem sub-entries, obtains their types and +// target types for symlinks. +// +// -v +// Print the filesystem entries types and names to STDOUT. +// +// -i +// Ignore dangling symlinks, rather than fail trying to obtain the target +// type. // int main (int argc, const char* argv[]) { - if (!(argc == 2 || (argc == 3 && argv[1] == string ("-v")))) + assert (argc > 0); + + bool verbose (false); + bool ignore_dangling (false); + + int i (1); + for (; i != argc; ++i) + { + string v (argv[i]); + + if (v == "-v") + verbose = true; + else if (v == "-i") + ignore_dangling = true; + else + break; + } + + if (i != argc - 1) { - cerr << "usage: " << argv[0] << " [-v] " << endl; + cerr << "usage: " << argv[0] << " [-v] [-i] " << endl; return 1; } - bool v (argc == 3); - const char* d (argv[argc - 1]); + const char* d (argv[i]); try { - for (const dir_entry& de: dir_iterator (dir_path (d))) + for (const dir_entry& de: dir_iterator (dir_path (d), ignore_dangling)) { entry_type lt (de.ltype ()); - entry_type t (lt == entry_type::symlink ? de.ltype () : lt); + entry_type t (lt == entry_type::symlink ? de.type () : lt); const path& p (de.path ()); - if (v) + if (verbose) { - cerr << lt << " "; + cout << lt << " "; if (lt == entry_type::symlink) - cerr << t; + cout << t; else - cerr << " "; + cout << " "; - cerr << " " << p << endl; + cout << " " << p << endl; } } } catch (const exception& e) { - cerr << argv[1] << ": " << e << endl; + cerr << e << endl; return 1; } } diff --git a/tests/dir-iterator/testscript b/tests/dir-iterator/testscript index 75a1ca6..f592c78 100644 --- a/tests/dir-iterator/testscript +++ b/tests/dir-iterator/testscript @@ -8,9 +8,24 @@ test.options = -v : mkdir a; touch a/b; -$* a 2>"reg b" +$* a >"reg b" : dir : mkdir -p a/b; -$* a 2>"dir b" +$* a >"dir b" + +: dangling-link +: +if ($cxx.target.class != 'windows') +{ + +mkdir a + +touch --no-cleanup a/b + +ln -s a/b a/l + +rm a/b + + +touch a/c + + $* ../a >! 2>! != 0 : keep + $* -i ../a >'reg c' : skip +} diff --git a/tests/link/driver.cxx b/tests/link/driver.cxx index 463ad23..7ee8c3b 100644 --- a/tests/link/driver.cxx +++ b/tests/link/driver.cxx @@ -96,11 +96,11 @@ link_dir (const dir_path& target, dir_path tp (target.absolute () ? target : link.directory () / target); set> te; - for (const dir_entry& de: dir_iterator (tp)) + for (const dir_entry& de: dir_iterator (tp, false /* ignore_dangling */)) te.emplace (de.ltype (), de.path ()); set> le; - for (const dir_entry& de: dir_iterator (link)) + for (const dir_entry& de: dir_iterator (link, false /* ignore_dangling */)) le.emplace (de.ltype (), de.path ()); return te == le; diff --git a/tests/wildcard/testscript b/tests/wildcard/testscript index 489f724..85f5a33 100644 --- a/tests/wildcard/testscript +++ b/tests/wildcard/testscript @@ -113,7 +113,7 @@ : absolute : - : When cross-testing we can't guarantee that host absolute paths are + : When cross-testing we cannot guarantee that host absolute paths are : recognized by the target process. : if ($test.target == $build.host) @@ -566,6 +566,20 @@ } } } + + : dangling-link + : + if ($cxx.target.class != 'windows') + { + mkdir a; + touch --no-cleanup a/b; + ln -s a/b a/l; + rm a/b; + + touch a/c; + + $* a/* >/'a/c' + } } : path-entry-search -- cgit v1.1