Add some code to the tree walking to better handle file descriptors.

Merge of master-melb:xfs-cmds:30192a by kenmcd.

6 files changed, 287 insertions, 69 deletions

diff --git a/VERSION b/VERSION
index a1d6280..d38a86c 100644
--- a/VERSION
+++ b/VERSION
@@ -3,5 +3,5 @@
 #
 PKG_MAJOR=2
 PKG_MINOR=4
-PKG_REVISION=40
+PKG_REVISION=41
 PKG_BUILD=0
diff --git a/doc/CHANGES b/doc/CHANGES
index 29e367f..7bb81ab 100644
--- a/doc/CHANGES
+++ b/doc/CHANGES
@@ -1,7 +1,8 @@
-attr-2.4.41
-	- remove outdated doc/ea-conv thanks to Andreas Gruenbacher
-	- Update getfattr man page for symlink related options
-	  thanks to Andreas G.
+attr-2.4.41 (4 December 2007)
+	- A number of changes from Andreas Gruenbacher:
+	    - remove outdated doc/ea-conv
+	    - fix issues for tree walking with file descriptors
+		- fd duplicates and running out
 
 attr-2.4.40 (21 November 2007)
 	- Address compilation warning about signedness in libattr.c
diff --git a/getfattr/getfattr.c b/getfattr/getfattr.c
index 9cabe43..ddd856e 100644
--- a/getfattr/getfattr.c
+++ b/getfattr/getfattr.c
@@ -346,12 +346,14 @@ int list_attributes(const char *path, int *header_printed)
 	return 0;
 }
 
-int do_print(const char *path, const struct stat *stat, int walk_flags, void *unused)
+int do_print(const char *path, const struct stat *stat, int walk_flags,
+	     void *unused)
 {
 	int header_printed = 0;
 
 	if (walk_flags & WALK_TREE_FAILED) {
-		fprintf(stderr, "%s: %s: %s\n", progname, xquote(path), strerror(errno));
+		fprintf(stderr, "%s: %s: %s\n", progname, xquote(path),
+			strerror(errno));
 		return 1;
 	}
 
@@ -474,7 +476,8 @@ int main(int argc, char *argv[])
 	}
 
 	while (optind < argc) {
-		had_errors += walk_tree(argv[optind], walk_flags, do_print, NULL);
+		had_errors += walk_tree(argv[optind], walk_flags, 0,
+					do_print, NULL);
 		optind++;
 	}
 
diff --git a/include/walk_tree.h b/include/walk_tree.h
index 96bb7a3..73cbc7c 100644
--- a/include/walk_tree.h
+++ b/include/walk_tree.h
@@ -1,3 +1,23 @@
+/*
+  File: walk_tree.h
+
+  Copyright (C) 2007 Andreas Gruenbacher <a.gruenbacher@computer.org>
+
+  This program is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public
+  License along with this library; if not, write to the Free Software
+  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+*/
+
 #ifndef __WALK_TREE_H
 #define __WALK_TREE_H
 
@@ -6,13 +26,14 @@
 #define WALK_TREE_LOGICAL	0x4
 #define WALK_TREE_DEREFERENCE	0x8
 
-#define WALK_TREE_SYMLINK	0x10
-#define WALK_TREE_FAILED	0x20
+#define WALK_TREE_TOPLEVEL	0x100
+#define WALK_TREE_SYMLINK	0x200
+#define WALK_TREE_FAILED	0x400
 
 struct stat;
 
-extern int walk_tree(const char *path, int walk_flags,
-		     int (*func)(const char *, const struct stat *, int, void *),
-		     void *arg);
+extern int walk_tree(const char *path, int walk_flags, unsigned int num,
+		     int (*func)(const char *, const struct stat *, int,
+				 void *), void *arg);
 
 #endif
diff --git a/libmisc/walk_tree.c b/libmisc/walk_tree.c
index 62bc74d..bd02d9e 100644
--- a/libmisc/walk_tree.c
+++ b/libmisc/walk_tree.c
@@ -21,6 +21,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <sys/time.h>
+#include <sys/resource.h>
 #include <dirent.h>
 #include <stdio.h>
 #include <string.h>
@@ -28,73 +30,193 @@
 
 #include "walk_tree.h"
 
+struct entry_handle {
+	struct entry_handle *prev, *next;
+	dev_t dev;
+	ino_t ino;
+	DIR *stream;
+	off_t pos;
+};
+
+struct entry_handle head = {
+	.next = &head,
+	.prev = &head,
+	/* The other fields are unused. */
+};
+struct entry_handle *closed = &head;
+unsigned int num_dir_handles;
+
+static int walk_tree_visited(dev_t dev, ino_t ino)
+{
+	struct entry_handle *i;
+
+	for (i = head.next; i != &head; i = i->next)
+		if (i->dev == dev && i->ino == ino)
+			return 1;
+	return 0;
+}
+
 static int walk_tree_rec(const char *path, int walk_flags,
-			 int (*func)(const char *, const struct stat *, int, void *),
-			 void *arg, int depth)
+			 int (*func)(const char *, const struct stat *, int,
+			 	     void *), void *arg, int depth)
 {
-	int (*xstat)(const char *, struct stat *) = lstat;
+	int follow_symlinks = (walk_flags & WALK_TREE_LOGICAL) ||
+			      (!(walk_flags & WALK_TREE_PHYSICAL) &&
+			       depth == 0);
+	int have_dir_stat = 0, flags = walk_flags, err;
+	struct entry_handle dir;
 	struct stat st;
-	int local_walk_flags = walk_flags, err;
 
-	/* Default to traversing symlinks on the command line, traverse all symlinks
-	 * with -L, and do not traverse symlinks with -P. (This is similar to chown.)
+	/*
+	 * If (walk_flags & WALK_TREE_PHYSICAL), do not traverse symlinks.
+	 * If (walk_flags & WALK_TREE_LOGICAL), traverse all symlinks.
+	 * Otherwise, traverse only top-level symlinks.
 	 */
+	if (depth == 0)
+		flags |= WALK_TREE_TOPLEVEL;
 
-follow_symlink:
-	if (xstat(path, &st) != 0)
-		return func(path, NULL, local_walk_flags | WALK_TREE_FAILED, arg);
+	if (lstat(path, &st) != 0)
+		return func(path, NULL, flags | WALK_TREE_FAILED, arg);
 	if (S_ISLNK(st.st_mode)) {
-		if ((local_walk_flags & WALK_TREE_PHYSICAL) ||
-		    (!(local_walk_flags & WALK_TREE_LOGICAL) && depth > 1))
-			return 0;
-		local_walk_flags |= WALK_TREE_SYMLINK;
-		xstat = stat;
-		if (local_walk_flags & WALK_TREE_DEREFERENCE)
-			goto follow_symlink;
+		flags |= WALK_TREE_SYMLINK;
+		if (flags & WALK_TREE_DEREFERENCE) {
+			if (stat(path, &st) != 0)
+				return func(path, NULL,
+					    flags | WALK_TREE_FAILED, arg);
+			dir.dev = st.st_dev;
+			dir.ino = st.st_ino;
+			have_dir_stat = 1;
+		}
+	} else if (S_ISDIR(st.st_mode)) {
+		dir.dev = st.st_dev;
+		dir.ino = st.st_ino;
+		have_dir_stat = 1;
 	}
-	err = func(path, &st, local_walk_flags, arg);
-	if ((local_walk_flags & WALK_TREE_RECURSIVE) &&
-	    (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) {
-		char path2[FILENAME_MAX];
-		DIR *dir;
+	err = func(path, &st, flags, arg);
+	if ((flags & WALK_TREE_RECURSIVE) &&
+	    (S_ISDIR(st.st_mode) || (S_ISLNK(st.st_mode) && follow_symlinks))) {
 		struct dirent *entry;
-		int err2;
 
-		dir = opendir(path);
-		if (!dir) {
-			/* PATH may be a symlink to a regular file or a dead symlink
-			 * which we didn't follow above.
+		/*
+		 * Check if we have already visited this directory to break
+		 * endless loops.
+		 *
+		 * If we haven't stat()ed the file yet, do an opendir() for
+		 * figuring out whether we have a directory, and check whether
+		 * the directory has been visited afterwards. This saves a
+		 * system call for each non-directory found.
+		 */
+		if (have_dir_stat && walk_tree_visited(dir.dev, dir.ino))
+			return err;
+
+		if (num_dir_handles == 0 && closed->prev != &head) {
+close_another_dir:
+			/* Close the topmost directory handle still open. */
+			closed = closed->prev;
+			closed->pos = telldir(closed->stream);
+			closedir(closed->stream);
+			closed->stream = NULL;
+			num_dir_handles++;
+		}
+
+		dir.stream = opendir(path);
+		if (!dir.stream) {
+			if (errno == ENFILE && closed->prev != &head) {
+				/* Ran out of file descriptors. */
+				num_dir_handles = 0;
+				goto close_another_dir;
+			}
+
+			/*
+			 * PATH may be a symlink to a regular file, or a dead
+			 * symlink which we didn't follow above.
 			 */
 			if (errno != ENOTDIR && errno != ENOENT)
-				err += func(path, &st,
-					    local_walk_flags | WALK_TREE_FAILED, arg);
+				err += func(path, NULL, flags |
+							WALK_TREE_FAILED, arg);
 			return err;
 		}
-		while ((entry = readdir(dir)) != NULL) {
-			if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, ".."))
+
+		/* See walk_tree_visited() comment above... */
+		if (!have_dir_stat) {
+			if (stat(path, &st) != 0)
+				goto skip_dir;
+			dir.dev = st.st_dev;
+			dir.ino = st.st_ino;
+			if (walk_tree_visited(dir.dev, dir.ino))
+				goto skip_dir;
+		}
+
+		/* Insert into the list of handles. */
+		dir.next = head.next;
+		dir.prev = &head;
+		dir.prev->next = &dir;
+		dir.next->prev = &dir;
+		num_dir_handles--;
+
+		while ((entry = readdir(dir.stream)) != NULL) {
+			char *path_end;
+
+			if (!strcmp(entry->d_name, ".") ||
+			    !strcmp(entry->d_name, ".."))
 				continue;
-			err2 = snprintf(path2, sizeof(path2), "%s/%s", path,
-				       entry->d_name);
-			if (err2 < 0 || err2 > FILENAME_MAX) {
+			path_end = strchr(path, 0);
+			if ((path_end - path) + strlen(entry->d_name) + 1 >=
+			    FILENAME_MAX) {
 				errno = ENAMETOOLONG;
 				err += func(path, NULL,
-					    local_walk_flags | WALK_TREE_FAILED, arg);
+					    flags | WALK_TREE_FAILED, arg);
 				continue;
 			}
-			err += walk_tree_rec(path2, walk_flags, func, arg, depth + 1);
+			*path_end++ = '/';
+			strcpy(path_end, entry->d_name);
+			err += walk_tree_rec(path, walk_flags, func, arg,
+					     depth + 1);
+			*--path_end = 0;
+			if (!dir.stream) {
+				/* Reopen the directory handle. */
+				dir.stream = opendir(path);
+				if (!dir.stream)
+					return err + func(path, NULL, flags |
+						    WALK_TREE_FAILED, arg);
+				seekdir(dir.stream, dir.pos);
+
+				closed = closed->next;
+				num_dir_handles--;
+			}
 		}
-		if (closedir(dir) != 0)
-			err += func(path, &st, local_walk_flags | WALK_TREE_FAILED, arg);
+
+		/* Remove from the list of handles. */
+		dir.prev->next = dir.next;
+		dir.next->prev = dir.prev;
+		num_dir_handles++;
+
+	skip_dir:
+		if (closedir(dir.stream) != 0)
+			err += func(path, NULL, flags | WALK_TREE_FAILED, arg);
 	}
 	return err;
 }
 
-int walk_tree(const char *path, int walk_flags,
-	      int (*func)(const char *, const struct stat *, int, void *), void *arg)
+int walk_tree(const char *path, int walk_flags, unsigned int num,
+	      int (*func)(const char *, const struct stat *, int, void *),
+	      void *arg)
 {
+	char path_copy[FILENAME_MAX];
+
+	num_dir_handles = num;
+	if (num_dir_handles < 1) {
+		struct rlimit rlimit;
+
+		num_dir_handles = 1;
+		if (getrlimit(RLIMIT_NOFILE, &rlimit) == 0 &&
+		    rlimit.rlim_cur >= 2)
+			num_dir_handles = rlimit.rlim_cur / 2;
+	}
 	if (strlen(path) >= FILENAME_MAX) {
 		errno = ENAMETOOLONG;
 		return func(path, NULL, WALK_TREE_FAILED, arg);
 	}
-	return walk_tree_rec(path, walk_flags, func, arg, 1);
+	strcpy(path_copy, path);
+	return walk_tree_rec(path_copy, walk_flags, func, arg, 0);
 }
diff --git a/test/getfattr.test b/test/getfattr.test
index 1e9366d..c9458e6 100644
--- a/test/getfattr.test
+++ b/test/getfattr.test
@@ -4,49 +4,120 @@
 	$ touch f
 	$ setfattr -n user.test -v test f
 	$ ln -s f l
+	$ setfattr -h -n trusted.test -v test l
 
 This case should be obvious:
-	$ getfattr -d f
+	$ getfattr -m- -d f
 	> # file: f
 	> user.test="test"
 	>
 
-If a symlink is explicitly specified on the command line, follow it
-(-H behavior):
-	$ getfattr -d l
+Without -h, we dereference symlinks:
+	$ getfattr -m- -d l
 	> # file: l
 	> user.test="test"
 	>
 
-Unless we are explicitly told not to dereference symlinks:
-	$ getfattr -hd l
+With -h, we do not dereference symlinks:
+	$ getfattr -m- -hd l
+	> # file: l
+	> trusted.test="test"
+	>
 
-When walking a tree, it does not make sense to follow symlinks. We should
-only see f's attributes here -- that's a bug:
-	$ getfattr -Rd .
+Do the same for symlinks we find in a directory hierarchy:
+	$ getfattr -m- -Rd .
 	> # file: f
 	> user.test="test"
 	>
+	> # file: l
+	> user.test="test"
+	>
 
-This case works as expected:
-	$ getfattr -Rhd .
+	$ getfattr -m- -Rhd .
 	> # file: f
 	> user.test="test"
 	>
+	> # file: l
+	> trusted.test="test"
+	>
 
-In these two cases, getfattr should dereference the symlink passed on the
-command line, but not l. This doesn't work correctly, either; it's the same
-bug:
+Make sure we follow symlinks on the command line only when we should:
 	$ ln -s . here
-	$ getfattr -Rd here
+	$ getfattr -m- -Rd here
 	> # file: here/f
 	> user.test="test"
 	>
+	> # file: here/l
+	> user.test="test"
+	>
 
-	$ getfattr -Rhd here
+	$ getfattr -m- -Rhd here
 	> # file: here/f
 	> user.test="test"
 	>
+	> # file: here/l
+	> trusted.test="test"
+	>
+
+	$ getfattr -m- -RLhd here
+	> # file: here/f
+	> user.test="test"
+	>
+	> # file: here/l
+	> trusted.test="test"
+	>
+
+	$ getfattr -m- -RPhd here
+
+Make sure we recurse into sub-directories:
+	$ mkdir sub
+	$ mv f l sub
+	$ getfattr -m- -Rd .
+	> # file: sub/f
+	> user.test="test"
+	>
+	> # file: sub/l
+	> user.test="test"
+	>
+
+	$ getfattr -m- -Rhd .
+	> # file: sub/f
+	> user.test="test"
+	>
+	> # file: sub/l
+	> trusted.test="test"
+	>
+
+Make sure we follow symlinks to directories only when we should:
+	$ mkdir sub2
+	$ ln -s ../sub sub2/to-sub
+	$ getfattr -m- -Rhd sub2
+
+	$ getfattr -m- -RLhd sub2
+	> # file: sub2/to-sub/f
+	> user.test="test"
+	>
+	> # file: sub2/to-sub/l
+	> trusted.test="test"
+	>
+
+	$ getfattr -m- -RPhd sub2
+
+Symlink loop detection:
+	$ ln -s .. sub/up
+	$ getfattr -m- -RLhd .
+	> # file: sub2/to-sub/f
+	> user.test="test"
+	>
+	> # file: sub2/to-sub/l
+	> trusted.test="test"
+	>
+	> # file: sub/f
+	> user.test="test"
+	>
+	> # file: sub/l
+	> trusted.test="test"
+	>
 
 	$ cd ..
 	$ rm -rf d