client: simplify/fix symlink loop check

Checking that we visit a symlink isn't correct; for example, the below is
valid, and we visit /b twice.

 /a/b -> c
 /a/c/d -> /a/b

In order to do this "correctly", I think we would need to track the pairs
of paths and symlinks we are resolving.  But, reading the man pages,
ELOOP is actually just defined as traversing more than MAXSYMLINKS syms.
(It appears to be 20 on my machine.)

Just do that instead.

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 251e40a23aa4ce754edb6afeaf97b2e9de9af469..a1c922f679c0dc8b8ad87111d9bd1e7582f31a68 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -20,6 +20,7 @@
 #include <time.h>
 #include <utime.h>
 #include <sys/stat.h>
+#include <sys/param.h>
 #include <fcntl.h>
 
 #include <sys/statvfs.h>
@@ -3736,7 +3737,8 @@ int Client::path_walk(const filepath& origpath, Inode **final, bool followsym)
 
   ldout(cct, 10) << "path_walk " << path << dendl;
 
-  set<Inode*,Inode::Compare> visited;
+  int symlinks = 0;
+
   unsigned i=0;
   while (i < path.depth() && cur) {
     const string &dname = path[i];
@@ -3749,15 +3751,13 @@ int Client::path_walk(const filepath& origpath, Inode **final, bool followsym)
     // only follow trailing symlink if followsym.  always follow
     // 'directory' symlinks.
     if (next && next->is_symlink()) {
-      ldout(cct, 20) << " symlink value is '" << next->symlink << "'" << dendl;
-      if (i < path.depth() - 1) {
-       // check for loops
-       if (visited.find(next) != visited.end()) {
-         // already hit this one, return error
-         return -ELOOP;
-       }
-       visited.insert(next);
+      symlinks++;
+      ldout(cct, 20) << " symlink count " << symlinks << ", value is '" << next->symlink << "'" << dendl;
+      if (symlinks > MAXSYMLINKS) {
+       return -ELOOP;
+      }
 
+      if (i < path.depth() - 1) {
        // dir symlink
        // replace consumed components of path with symlink dir target
        filepath resolved(next->symlink.c_str());