Hi everyone,
I am having problems with a patch for Glob functionality because Glob
is part of KWSys repository.
I tried to register to http://review.source.kitware.com so that I
would be able to commit patches to KWSys repository but after I put in
my OpenID I got a "Not Found" error.
How can I contribute this patch?
Attached are KWSys patch and KWSys+CMake patch.
Thanks,
Domen
From 9c113fe546669a064d461004ae7e74be07744e40 Mon Sep 17 00:00:00 2001
From: Domen Vrankar <domen.vran...@gmail.com>
Date: Sun, 15 Feb 2015 20:42:16 +0100
Subject: [PATCH] handle cyclic recursion when using glob recurse with follow
symlinks
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/Directory.cxx | 35 ++++++++++++++++++++++
Source/kwsys/Directory.hxx.in | 7 +++++
Source/kwsys/Glob.cxx | 32 +++++++++++++++++++-
Source/kwsys/Glob.hxx.in | 2 ++
.../file/GLOB_RECURSE-symlink-recursion-stderr.txt | 2 ++
.../file/GLOB_RECURSE-symlink-recursion.cmake | 10 +++++++
Tests/RunCMake/file/RunCMakeTest.cmake | 1 +
7 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644 Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion-stderr.txt
create mode 100644 Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion.cmake
diff --git a/Source/kwsys/Directory.cxx b/Source/kwsys/Directory.cxx
index 58cea63..5494908 100644
--- a/Source/kwsys/Directory.cxx
+++ b/Source/kwsys/Directory.cxx
@@ -18,6 +18,7 @@
#include KWSYS_HEADER(stl/string)
#include KWSYS_HEADER(stl/vector)
+#include KWSYS_HEADER(stl/stdexcept)
// Work-around CMake dependency scanning limitation. This must
// duplicate the above list of headers.
@@ -28,8 +29,11 @@
# include "kwsys_stl.hxx.in"
# include "kwsys_stl_string.hxx.in"
# include "kwsys_stl_vector.hxx.in"
+# include "kwsys_stl_stdexcept.hxx.in"
#endif
+#include <stdlib.h>
+
namespace KWSYS_NAMESPACE
{
@@ -79,6 +83,37 @@ const char* Directory::GetPath() const
}
//----------------------------------------------------------------------------
+kwsys_stl::string Directory::GetCanonicalPath() const
+{
+/* The maximum length of a file name. */
+#if defined(PATH_MAX)
+ #define CANONICAL_PATH_MAX_LEN PATH_MAX
+#elif defined(MAXPATHLEN)
+ #define CANONICAL_PATH_MAX_LEN MAXPATHLEN
+#else
+ #define CANONICAL_PATH_MAX_LEN 16384
+#endif
+
+ char out_path[CANONICAL_PATH_MAX_LEN];
+
+#if defined(_WIN32) && !defined(__CYGWIN__)
+ /* Implementation for Windows. */
+ DWORD n = GetFullPathNameA(this->Internal->Path.c_str(),
+ CANONICAL_PATH_MAX_LEN], out_path, 0);
+ if( n > 0 && n <= CANONICAL_PATH_MAX_LEN )
+#else
+ /* Implementation for UNIX. */
+ if( !realpath(this->Internal->Path.c_str(), out_path) )
+#endif
+ {
+ throw kwsys_stl::runtime_error("Failed to convert path '"
+ + this->Internal->Path + "' to canonical form.");
+ }
+
+ return out_path;
+}
+
+//----------------------------------------------------------------------------
void Directory::Clear()
{
this->Internal->Path.resize(0);
diff --git a/Source/kwsys/Directory.hxx.in b/Source/kwsys/Directory.hxx.in
index 1bcf90e..453b578 100644
--- a/Source/kwsys/Directory.hxx.in
+++ b/Source/kwsys/Directory.hxx.in
@@ -68,6 +68,13 @@ public:
const char* GetPath() const;
/**
+ * Return path to Open'ed directory without symbolic links, dot or
+ * dot-dot elements. runtime_error is thrown if canonical path
+ * could not be created.
+ */
+ kwsys_stl::string GetCanonicalPath() const;
+
+ /**
* Clear the internal structure. Used internally at beginning of Load(...)
* to clear the cache.
*/
diff --git a/Source/kwsys/Glob.cxx b/Source/kwsys/Glob.cxx
index 5a96aed..22bab9f 100644
--- a/Source/kwsys/Glob.cxx
+++ b/Source/kwsys/Glob.cxx
@@ -30,11 +30,14 @@
# include "SystemTools.hxx.in"
# include "kwsys_stl.hxx.in"
# include "kwsys_stl_string.hxx.in"
+# include "kwsys_stl_vector.hxx.in"
#endif
#include <ctype.h>
#include <stdio.h>
#include <string.h>
+#include <stdlib.h>
+#include <iostream>
namespace KWSYS_NAMESPACE
{
#if defined(_WIN32) || defined(__APPLE__) || defined(__CYGWIN__)
@@ -265,8 +268,35 @@ void Glob::RecurseDirectory(kwsys_stl::string::size_type start,
if (isSymLink)
{
++this->FollowedSymlinkCount;
+ kwsys_stl::string canonicalPath(d.GetCanonicalPath());
+
+ if(VisitedSymlinks.find(canonicalPath) == VisitedSymlinks.end())
+ {
+ // structure for handling symbolic link path even if an exception
+ // is thrown
+ struct ScopedSymlinkPath
+ {
+ ScopedSymlinkPath(
+ kwsys_stl::set<kwsys_stl::string>& visitedSymlinks,
+ const std::string& canonicalPath)
+ : RefVisitedSymlinks(visitedSymlinks)
+ , RefCanonicalPath(canonicalPath)
+ {
+ RefVisitedSymlinks.insert(RefCanonicalPath);
+ }
+ ~ScopedSymlinkPath() {RefVisitedSymlinks.erase(RefCanonicalPath);}
+ kwsys_stl::set<kwsys_stl::string>& RefVisitedSymlinks;
+ const kwsys_stl::string& RefCanonicalPath;
+ } scopedSymlinkPath(VisitedSymlinks, canonicalPath);
+
+ this->RecurseDirectory(start+1, realname);
+ }
+ // else we have already visited this symlink - prevent cyclic recursion
+ }
+ else
+ {
+ this->RecurseDirectory(start+1, realname);
}
- this->RecurseDirectory(start+1, realname);
}
else
{
diff --git a/Source/kwsys/Glob.hxx.in b/Source/kwsys/Glob.hxx.in
index d8b8491..61bbd35 100644
--- a/Source/kwsys/Glob.hxx.in
+++ b/Source/kwsys/Glob.hxx.in
@@ -17,6 +17,7 @@
#include <@KWSYS_NAMESPACE@/stl/string>
#include <@KWSYS_NAMESPACE@/stl/vector>
+#include <@KWSYS_NAMESPACE@/stl/set>
/* Define this macro temporarily to keep the code readable. */
#if !defined (KWSYS_NAMESPACE) && !@KWSYS_NAMESPACE@_NAME_IS_KWSYS
@@ -101,6 +102,7 @@ protected:
kwsys_stl::string Relative;
bool RecurseThroughSymlinks;
unsigned int FollowedSymlinkCount;
+ kwsys_stl::set<kwsys_stl::string> VisitedSymlinks;
private:
Glob(const Glob&); // Not implemented.
diff --git a/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion-stderr.txt b/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion-stderr.txt
new file mode 100644
index 0000000..865545a
--- /dev/null
+++ b/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion-stderr.txt
@@ -0,0 +1,2 @@
+.*files: 4 .*
+.*/test/abc;.*/test/depth1/depth2/depth3/recursion/abc;.*/test/depth1/depth2/depth3/recursion/depth1/depth2/depth3/file_symlink;.*/test/depth1/depth2/depth3/file_symlink.*
diff --git a/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion.cmake b/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion.cmake
new file mode 100644
index 0000000..d114737
--- /dev/null
+++ b/Tests/RunCMake/file/GLOB_RECURSE-symlink-recursion.cmake
@@ -0,0 +1,10 @@
+file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/test/depth1/depth2/depth3")
+execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_BINARY_DIR}/test" "${CMAKE_CURRENT_BINARY_DIR}/test/depth1/depth2/depth3/recursion")
+
+file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test/abc" "message to write")
+execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_BINARY_DIR}/test/abc" "${CMAKE_CURRENT_BINARY_DIR}/test/depth1/depth2/depth3/file_symlink")
+
+file(GLOB_RECURSE FILE_LIST FOLLOW_SYMLINKS "${CMAKE_CURRENT_BINARY_DIR}/test/*")
+list(LENGTH FILE_LIST FILES_COUNT)
+message("files: ${FILES_COUNT} ")
+message("${FILE_LIST}")
diff --git a/Tests/RunCMake/file/RunCMakeTest.cmake b/Tests/RunCMake/file/RunCMakeTest.cmake
index 14819e7..1afae62 100644
--- a/Tests/RunCMake/file/RunCMakeTest.cmake
+++ b/Tests/RunCMake/file/RunCMakeTest.cmake
@@ -17,3 +17,4 @@ run_cmake(LOCK-error-no-result-variable)
run_cmake(LOCK-error-no-timeout)
run_cmake(LOCK-error-timeout)
run_cmake(LOCK-error-unknown-option)
+run_cmake(GLOB_RECURSE-symlink-recursion)
--
2.1.0
From d70ceb156cf801026086e3ba009c68a029523baf Mon Sep 17 00:00:00 2001
From: Domen Vrankar <domen.vran...@gmail.com>
Date: Sun, 15 Feb 2015 20:58:48 +0100
Subject: [PATCH] handle glob symlink cyclic recursion
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.
---
Directory.cxx | 35 +++++++++++++++++++++++++++++++++++
Directory.hxx.in | 7 +++++++
Glob.cxx | 32 +++++++++++++++++++++++++++++++-
Glob.hxx.in | 2 ++
4 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/Directory.cxx b/Directory.cxx
index 58cea63..5494908 100644
--- a/Directory.cxx
+++ b/Directory.cxx
@@ -18,6 +18,7 @@
#include KWSYS_HEADER(stl/string)
#include KWSYS_HEADER(stl/vector)
+#include KWSYS_HEADER(stl/stdexcept)
// Work-around CMake dependency scanning limitation. This must
// duplicate the above list of headers.
@@ -28,8 +29,11 @@
# include "kwsys_stl.hxx.in"
# include "kwsys_stl_string.hxx.in"
# include "kwsys_stl_vector.hxx.in"
+# include "kwsys_stl_stdexcept.hxx.in"
#endif
+#include <stdlib.h>
+
namespace KWSYS_NAMESPACE
{
@@ -79,6 +83,37 @@ const char* Directory::GetPath() const
}
//----------------------------------------------------------------------------
+kwsys_stl::string Directory::GetCanonicalPath() const
+{
+/* The maximum length of a file name. */
+#if defined(PATH_MAX)
+ #define CANONICAL_PATH_MAX_LEN PATH_MAX
+#elif defined(MAXPATHLEN)
+ #define CANONICAL_PATH_MAX_LEN MAXPATHLEN
+#else
+ #define CANONICAL_PATH_MAX_LEN 16384
+#endif
+
+ char out_path[CANONICAL_PATH_MAX_LEN];
+
+#if defined(_WIN32) && !defined(__CYGWIN__)
+ /* Implementation for Windows. */
+ DWORD n = GetFullPathNameA(this->Internal->Path.c_str(),
+ CANONICAL_PATH_MAX_LEN], out_path, 0);
+ if( n > 0 && n <= CANONICAL_PATH_MAX_LEN )
+#else
+ /* Implementation for UNIX. */
+ if( !realpath(this->Internal->Path.c_str(), out_path) )
+#endif
+ {
+ throw kwsys_stl::runtime_error("Failed to convert path '"
+ + this->Internal->Path + "' to canonical form.");
+ }
+
+ return out_path;
+}
+
+//----------------------------------------------------------------------------
void Directory::Clear()
{
this->Internal->Path.resize(0);
diff --git a/Directory.hxx.in b/Directory.hxx.in
index 1bcf90e..453b578 100644
--- a/Directory.hxx.in
+++ b/Directory.hxx.in
@@ -68,6 +68,13 @@ public:
const char* GetPath() const;
/**
+ * Return path to Open'ed directory without symbolic links, dot or
+ * dot-dot elements. runtime_error is thrown if canonical path
+ * could not be created.
+ */
+ kwsys_stl::string GetCanonicalPath() const;
+
+ /**
* Clear the internal structure. Used internally at beginning of Load(...)
* to clear the cache.
*/
diff --git a/Glob.cxx b/Glob.cxx
index 5a96aed..22bab9f 100644
--- a/Glob.cxx
+++ b/Glob.cxx
@@ -30,11 +30,14 @@
# include "SystemTools.hxx.in"
# include "kwsys_stl.hxx.in"
# include "kwsys_stl_string.hxx.in"
+# include "kwsys_stl_vector.hxx.in"
#endif
#include <ctype.h>
#include <stdio.h>
#include <string.h>
+#include <stdlib.h>
+#include <iostream>
namespace KWSYS_NAMESPACE
{
#if defined(_WIN32) || defined(__APPLE__) || defined(__CYGWIN__)
@@ -265,8 +268,35 @@ void Glob::RecurseDirectory(kwsys_stl::string::size_type start,
if (isSymLink)
{
++this->FollowedSymlinkCount;
+ kwsys_stl::string canonicalPath(d.GetCanonicalPath());
+
+ if(VisitedSymlinks.find(canonicalPath) == VisitedSymlinks.end())
+ {
+ // structure for handling symbolic link path even if an exception
+ // is thrown
+ struct ScopedSymlinkPath
+ {
+ ScopedSymlinkPath(
+ kwsys_stl::set<kwsys_stl::string>& visitedSymlinks,
+ const std::string& canonicalPath)
+ : RefVisitedSymlinks(visitedSymlinks)
+ , RefCanonicalPath(canonicalPath)
+ {
+ RefVisitedSymlinks.insert(RefCanonicalPath);
+ }
+ ~ScopedSymlinkPath() {RefVisitedSymlinks.erase(RefCanonicalPath);}
+ kwsys_stl::set<kwsys_stl::string>& RefVisitedSymlinks;
+ const kwsys_stl::string& RefCanonicalPath;
+ } scopedSymlinkPath(VisitedSymlinks, canonicalPath);
+
+ this->RecurseDirectory(start+1, realname);
+ }
+ // else we have already visited this symlink - prevent cyclic recursion
+ }
+ else
+ {
+ this->RecurseDirectory(start+1, realname);
}
- this->RecurseDirectory(start+1, realname);
}
else
{
diff --git a/Glob.hxx.in b/Glob.hxx.in
index d8b8491..61bbd35 100644
--- a/Glob.hxx.in
+++ b/Glob.hxx.in
@@ -17,6 +17,7 @@
#include <@KWSYS_NAMESPACE@/stl/string>
#include <@KWSYS_NAMESPACE@/stl/vector>
+#include <@KWSYS_NAMESPACE@/stl/set>
/* Define this macro temporarily to keep the code readable. */
#if !defined (KWSYS_NAMESPACE) && !@KWSYS_NAMESPACE@_NAME_IS_KWSYS
@@ -101,6 +102,7 @@ protected:
kwsys_stl::string Relative;
bool RecurseThroughSymlinks;
unsigned int FollowedSymlinkCount;
+ kwsys_stl::set<kwsys_stl::string> VisitedSymlinks;
private:
Glob(const Glob&); // Not implemented.
--
2.1.0
--
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
