> For the other two changes, please refactor things to avoid > use of function-local classes. They will likely not work > on the older compilers KWSys supports. > > Also, please use the coding style of "this->Member" when > referencing class members from method code.
Attached are refactored patches. Thanks, Domen
From b7bd613c57b0d2cbe695792d58289a53757cc9f5 Mon Sep 17 00:00:00 2001 From: Domen Vrankar <[email protected]> Date: Sun, 1 Mar 2015 22:29:54 +0100 Subject: [PATCH 1/2] glob recurse cyclic recursion handling Prevent cyclic recursion of type a/b/c->a when glob recurse is used with follow symlinks so that each directory symbolic link is traversed only once and skipped on revisit. --- Source/kwsys/Glob.cxx | 76 ++++++++++++++++++++++++++++++++++++++++-------- Source/kwsys/Glob.hxx.in | 36 ++++++++++++++++++++--- 2 files changed, 96 insertions(+), 16 deletions(-) diff --git a/Source/kwsys/Glob.cxx b/Source/kwsys/Glob.cxx index 1476c25..423e586 100644 --- a/Source/kwsys/Glob.cxx +++ b/Source/kwsys/Glob.cxx @@ -19,6 +19,7 @@ #include KWSYS_HEADER(Directory.hxx) #include KWSYS_HEADER(stl/string) #include KWSYS_HEADER(stl/vector) +#include KWSYS_HEADER(stl/algorithm) // Work-around CMake dependency scanning limitation. This must // duplicate the above list of headers. @@ -30,6 +31,8 @@ # include "SystemTools.hxx.in" # include "kwsys_stl.hxx.in" # include "kwsys_stl_string.hxx.in" +# include "kwsys_stl_vector.hxx.in" +# include "kwsys_stl_algorithm.hxx.in" #endif #include <ctype.h> @@ -214,13 +217,13 @@ kwsys_stl::string Glob::PatternToRegex(const kwsys_stl::string& pattern, } //---------------------------------------------------------------------------- -void Glob::RecurseDirectory(kwsys_stl::string::size_type start, - const kwsys_stl::string& dir) +bool Glob::RecurseDirectory(kwsys_stl::string::size_type start, + const kwsys_stl::string& dir, GlobMessages* messages) { kwsys::Directory d; if ( !d.Load(dir) ) { - return; + return true; } unsigned long cc; kwsys_stl::string realname; @@ -255,8 +258,56 @@ void Glob::RecurseDirectory(kwsys_stl::string::size_type start, if (isSymLink) { ++this->FollowedSymlinkCount; + kwsys_stl::string realPathErrorMessage; + kwsys_stl::string canonicalPath(SystemTools::GetRealPath(dir, + &realPathErrorMessage)); + + if(!realPathErrorMessage.empty()) + { + if(messages) + { + messages->push_back(Message( + Glob::error, "Canonical path generation from path '" + + dir + "' failed! Reason: '" + realPathErrorMessage + "'")); + } + return false; + } + + if(kwsys_stl::find(this->VisitedSymlinks.begin(), + this->VisitedSymlinks.end(), + canonicalPath) == this->VisitedSymlinks.end()) + { + this->VisitedSymlinks.push_back(canonicalPath); + if(!this->RecurseDirectory(start+1, realname, messages)) + { + this->VisitedSymlinks.pop_back(); + + return false; + } + this->VisitedSymlinks.pop_back(); + } + // else we have already visited this symlink - prevent cyclic recursion + else if(messages) + { + kwsys_stl::string message; + kwsys_stl::vector<kwsys_stl::string>::const_iterator pathIt = + kwsys_stl::find(this->VisitedSymlinks.begin(), + this->VisitedSymlinks.end(), canonicalPath); + for(pathIt; pathIt != this->VisitedSymlinks.end(); ++pathIt) + { + message += *pathIt + "\n"; + } + message += canonicalPath + "/" + fname; + messages->push_back(Message(Glob::cyclicRecursion, message)); + } + } + else + { + if(!this->RecurseDirectory(start+1, realname, messages)) + { + return false; + } } - this->RecurseDirectory(start+1, realname); } else { @@ -267,17 +318,19 @@ void Glob::RecurseDirectory(kwsys_stl::string::size_type start, } } } + + return true; } //---------------------------------------------------------------------------- void Glob::ProcessDirectory(kwsys_stl::string::size_type start, - const kwsys_stl::string& dir) + const kwsys_stl::string& dir, GlobMessages* messages) { //kwsys_ios::cout << "ProcessDirectory: " << dir << kwsys_ios::endl; bool last = ( start == this->Internals->Expressions.size()-1 ); if ( last && this->Recurse ) { - this->RecurseDirectory(start, dir); + this->RecurseDirectory(start, dir, messages); return; } @@ -321,8 +374,7 @@ void Glob::ProcessDirectory(kwsys_stl::string::size_type start, // << this->Internals->TextExpressions[start].c_str() << kwsys_ios::endl; //kwsys_ios::cout << "Real name: " << realname << kwsys_ios::endl; - if ( !last && - !kwsys::SystemTools::FileIsDirectory(realname) ) + if( !last && !kwsys::SystemTools::FileIsDirectory(realname) ) { continue; } @@ -335,14 +387,14 @@ void Glob::ProcessDirectory(kwsys_stl::string::size_type start, } else { - this->ProcessDirectory(start+1, realname); + this->ProcessDirectory(start+1, realname, messages); } } } } //---------------------------------------------------------------------------- -bool Glob::FindFiles(const kwsys_stl::string& inexpr) +bool Glob::FindFiles(const kwsys_stl::string& inexpr, GlobMessages* messages) { kwsys_stl::string cexpr; kwsys_stl::string::size_type cc; @@ -438,11 +490,11 @@ bool Glob::FindFiles(const kwsys_stl::string& inexpr) // Handle network paths if ( skip > 0 ) { - this->ProcessDirectory(0, fexpr.substr(0, skip) + "/"); + this->ProcessDirectory(0, fexpr.substr(0, skip) + "/", messages); } else { - this->ProcessDirectory(0, "/"); + this->ProcessDirectory(0, "/", messages); } return true; } diff --git a/Source/kwsys/Glob.hxx.in b/Source/kwsys/Glob.hxx.in index d8b8491..0d40d02 100644 --- a/Source/kwsys/Glob.hxx.in +++ b/Source/kwsys/Glob.hxx.in @@ -40,11 +40,36 @@ class GlobInternals; class @KWSYS_NAMESPACE@_EXPORT Glob { public: + enum MessageType + { + error, + cyclicRecursion + }; + + struct Message + { + MessageType type; + kwsys_stl::string content; + + Message(MessageType t, const kwsys_stl::string& c) : + type(t), + content(c) + {} + Message(const Message& msg) : + type(msg.type), + content(msg.content) + {} + }; + + typedef kwsys_stl::vector<Message> GlobMessages; + typedef kwsys_stl::vector<Message>::iterator GlobMessagesIterator; +public: Glob(); ~Glob(); //! Find all files that match the pattern. - bool FindFiles(const kwsys_stl::string& inexpr); + bool FindFiles(const kwsys_stl::string& inexpr, + GlobMessages* messages = 0); //! Return the list of files that matched. kwsys_stl::vector<kwsys_stl::string>& GetFiles(); @@ -83,12 +108,14 @@ public: protected: //! Process directory void ProcessDirectory(kwsys_stl::string::size_type start, - const kwsys_stl::string& dir); + const kwsys_stl::string& dir, + GlobMessages* messages); //! Process last directory, but only when recurse flags is on. That is // effectively like saying: /path/to/file/**/file - void RecurseDirectory(kwsys_stl::string::size_type start, - const kwsys_stl::string& dir); + bool RecurseDirectory(kwsys_stl::string::size_type start, + const kwsys_stl::string& dir, + GlobMessages* messages); //! Add regular expression void AddExpression(const kwsys_stl::string& expr); @@ -101,6 +128,7 @@ protected: kwsys_stl::string Relative; bool RecurseThroughSymlinks; unsigned int FollowedSymlinkCount; + kwsys_stl::vector<kwsys_stl::string> VisitedSymlinks; private: Glob(const Glob&); // Not implemented. -- 2.1.4
From 664a48d7d0b148c34756cfc54ca09778e81b42ff Mon Sep 17 00:00:00 2001 From: Domen Vrankar <[email protected]> Date: Sun, 1 Mar 2015 22:35:37 +0100 Subject: [PATCH 2/2] glob support for directory listing Glob currently supports listing of directories in recursive mode and does not list directories in non recursive mode. The patch adds setting of listing directories mode for both recursive and non recursive globing. In recursive mode when directory listing is enabled directory symlinks are also listed as if they were directories. For back compatibility there is a separate flag for recursive and non recursive globing mode as pre patch functionality was inconsistent. --- Source/kwsys/Glob.cxx | 18 +++++++++++++++++- Source/kwsys/Glob.hxx.in | 11 +++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Source/kwsys/Glob.cxx b/Source/kwsys/Glob.cxx index 423e586..6e896b8 100644 --- a/Source/kwsys/Glob.cxx +++ b/Source/kwsys/Glob.cxx @@ -69,6 +69,10 @@ Glob::Glob() // RecurseThroughSymlinks is true by default for backwards compatibility, // not because it's a good idea... this->FollowedSymlinkCount = 0; + + // Keep separate variables for directory listing for back compatibility + this->ListDirs = true; + this->RecurseListDirs = false; } //---------------------------------------------------------------------------- @@ -277,6 +281,12 @@ bool Glob::RecurseDirectory(kwsys_stl::string::size_type start, this->VisitedSymlinks.end(), canonicalPath) == this->VisitedSymlinks.end()) { + if(RecurseListDirs) + { + // symlinks are treated as directories + this->AddFile(this->Internals->Files, realname); + } + this->VisitedSymlinks.push_back(canonicalPath); if(!this->RecurseDirectory(start+1, realname, messages)) { @@ -303,6 +313,10 @@ bool Glob::RecurseDirectory(kwsys_stl::string::size_type start, } else { + if(this->RecurseListDirs) + { + this->AddFile(this->Internals->Files, realname); + } if(!this->RecurseDirectory(start+1, realname, messages)) { return false; @@ -374,7 +388,9 @@ void Glob::ProcessDirectory(kwsys_stl::string::size_type start, // << this->Internals->TextExpressions[start].c_str() << kwsys_ios::endl; //kwsys_ios::cout << "Real name: " << realname << kwsys_ios::endl; - if( !last && !kwsys::SystemTools::FileIsDirectory(realname) ) + if( (!last && !kwsys::SystemTools::FileIsDirectory(realname)) + || (!this->ListDirs && last && + kwsys::SystemTools::FileIsDirectory(realname)) ) { continue; } diff --git a/Source/kwsys/Glob.hxx.in b/Source/kwsys/Glob.hxx.in index 0d40d02..39b7ce7 100644 --- a/Source/kwsys/Glob.hxx.in +++ b/Source/kwsys/Glob.hxx.in @@ -105,6 +105,15 @@ public: bool require_whole_string = true, bool preserve_case = false); + /** Getters and setters for enabling and disabling directory + listing in recursive and non recursive globbing mode. + If listing is enabled in recursive mode it also lists + directory symbolic links even if follow symlinks is enabled. */ + void SetListDirs(bool list) { this->ListDirs=list; } + bool GetListDirs() const { return this->ListDirs; } + void SetRecurseListDirs(bool list) { this->RecurseListDirs=list; } + bool GetRecurseListDirs() const { return this->RecurseListDirs; } + protected: //! Process directory void ProcessDirectory(kwsys_stl::string::size_type start, @@ -129,6 +138,8 @@ protected: bool RecurseThroughSymlinks; unsigned int FollowedSymlinkCount; kwsys_stl::vector<kwsys_stl::string> VisitedSymlinks; + bool ListDirs; + bool RecurseListDirs; private: Glob(const Glob&); // Not implemented. -- 2.1.4
-- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
