acl and attr: Fix path walking code

acl and attr: Fix path walking code

[Andreas Gruenbacher]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

Hello,

the tree walking code in acl and attr broke when resolve_symlinks() was 
introduced (by me, unfortunately). Following symlinks passed in on the 
command line is the intended behavior for the tools (unless in -P mode). The 
first version was buggy, and so someone "fixed" it by replacing readlink() 
with realpath() in resolve_symlinks().

The result is that the output of getfattr and getfacl will show pathnames that 
may point anywhere. When processing a directory tree it sometimes is helpful 
to treat symlinks as regular files, but resolving the pathnames is totally 
wrong.

After runnig into problem after problem with nftw and never ending up with 
even half-way clean code, I think it's time to ditch it altogether and 
replace it with sane code. So here are two patches, one for attr and one for 
acl, that does that.

Files include/walk_tree.h and libmisc/walk_tree.c are identical in both 
patches; that code is shared between the two packages.

Okay to apply?

Thanks,
Andreas

[-- Attachment #2: walk-acl.diff --]
[-- Type: text/x-diff, Size: 18787 bytes --]

Index: acl-2.2.45/getfacl/getfacl.c
===================================================================
--- acl-2.2.45.orig/getfacl/getfacl.c
+++ acl-2.2.45/getfacl/getfacl.c
@@ -34,10 +34,10 @@
 #include <dirent.h>
 #include <libgen.h>
 #include <getopt.h>
-#include <ftw.h>
 #include <locale.h>
 #include "config.h"
 #include "user_group.h"
+#include "walk_tree.h"
 #include "misc.h"
 
 #define POSIXLY_CORRECT_STR "POSIXLY_CORRECT"
@@ -70,24 +70,22 @@ struct option long_options[] = {
 const char *progname;
 const char *cmd_line_options;
 
-int opt_recursive;  /* recurse into sub-directories? */
-int opt_walk_logical;  /* always follow symbolic links */
-int opt_walk_physical;  /* never follow symbolic links */
-int opt_print_acl = 0;
-int opt_print_default_acl = 0;
+int walk_flags = WALK_TREE_DEREFERENCE;
+int opt_print_acl;
+int opt_print_default_acl;
 int opt_strip_leading_slash = 1;
 int opt_comments = 1;  /* include comments */
-int opt_skip_base = 0;  /* skip files that only have the base entries */
-int opt_tabular = 0;  /* tabular output format (alias `showacl') */
+int opt_skip_base;  /* skip files that only have the base entries */
+int opt_tabular;  /* tabular output format (alias `showacl') */
 #if POSIXLY_CORRECT
 const int posixly_correct = 1;  /* Posix compatible behavior! */
 #else
-int posixly_correct = 0;  /* Posix compatible behavior? */
+int posixly_correct;  /* Posix compatible behavior? */
 #endif
-int had_errors = 0;
-int absolute_warning = 0;  /* Absolute path warning was issued */
+int had_errors;
+int absolute_warning;  /* Absolute path warning was issued */
 int print_options = TEXT_SOME_EFFECTIVE;
-int opt_numeric = 0;  /* don't convert id's to symbolic names */
+int opt_numeric;  /* don't convert id's to symbolic names */
 
 
 static const char *xquote(const char *str)
@@ -425,12 +423,23 @@ acl_get_file_mode(const char *path_p)
 	return acl_from_mode(st.st_mode);
 }
 
-int do_print(const char *path_p, const struct stat *st)
+int do_print(const char *path_p, const struct stat *st, int walk_flags, void *unused)
 {
 	const char *default_prefix = NULL;
 	acl_t acl = NULL, default_acl = NULL;
 	int error = 0;
 
+	if (walk_flags & WALK_TREE_FAILED) {
+		fprintf(stderr, "%s: %s: %s\n", progname, xquote(path_p),
+			strerror(errno));
+		return 1;
+	}
+
+	if ((walk_flags & WALK_TREE_SYMLINK) &&
+	    ((walk_flags & WALK_TREE_PHYSICAL) ||
+	     !(walk_flags & (WALK_TREE_TOPLEVEL | WALK_TREE_LOGICAL))))
+		return 0;
+
 	if (opt_print_acl) {
 		acl = acl_get_file(path_p, ACL_TYPE_ACCESS);
 		if (acl == NULL && (errno == ENOSYS || errno == ENOTSUP))
@@ -549,7 +558,7 @@ void help(void)
 "      --skip-base         skip files that only have the base entries\n"
 "  -R, --recursive         recurse into subdirectories\n"
 "  -L, --logical           logical walk, follow symbolic links\n"
-"  -P  --physical          physical walk, do not follow symbolic links\n"
+"  -P, --physical          physical walk, do not follow symbolic links\n"
 "      --tabular           use tabular output format\n"
 "      --numeric           print numeric user/group identifiers\n"
 "      --absolute-names    don't strip leading '/' in pathnames\n"));
@@ -560,75 +569,6 @@ void help(void)
 "      --help              this help text\n"));
 }
 
-
-static int __errors;
-int __do_print(const char *file, const struct stat *stat,
-               int flag, struct FTW *ftw)
-{
-	int saved_errno = errno;
-
-	/* Process the target of a symbolic link, and traverse the link,
-           only if doing a logical walk, or if the symbolic link was
-           specified on the command line. Always skip symbolic links if
-           doing a physical walk. */
-
-	if (S_ISLNK(stat->st_mode) &&
-	    (opt_walk_physical || (ftw->level > 0 && !opt_walk_logical)))
-		return 0;
-
-	if (do_print(file, stat))
-		__errors++;
-
-	if (flag == FTW_DNR && opt_recursive) {
-		/* Item is a directory which can't be read. */
-		fprintf(stderr, "%s: %s: %s\n",
-			progname, file, strerror(saved_errno));
-		return 0;
-	}
-
-	/* We also get here in non-recursive mode. In that case,
-	   return something != 0 to abort nftw. */
-
-	if (!opt_recursive)
-		return 1;
-
-	return 0;
-}
-
-char *resolve_symlinks(const char *file)
-{
-	static char buffer[4096];
-	struct stat stat;
-	char *path = NULL;
-
-	if (lstat(file, &stat) == -1)
-		return path;
-
-	if (S_ISLNK(stat.st_mode) && !opt_walk_physical)
-		path = realpath(file, buffer);
-	else
-		path = (char *)file; 	/* not a symlink, use given path */
-
-	return path;
-}
-
-int walk_tree(const char *file)
-{
-	const char *p;
-
-	__errors = 0;
-	if ((p = resolve_symlinks(file)) == NULL) {
-		fprintf(stderr, "%s: %s: %s\n", progname,
-			xquote(file), strerror(errno));
-		__errors++;
-	} else if (nftw(p, __do_print, 0, opt_walk_logical? 0 : FTW_PHYS) < 0) {
-		fprintf(stderr, "%s: %s: %s\n", progname, xquote(file),
-			strerror(errno));
-		__errors++;
-	}
-	return __errors;
-}
-
 int main(int argc, char *argv[])
 {
 	int opt;
@@ -691,21 +631,21 @@ int main(int argc, char *argv[])
 			case 'R':  /* recursive */
 				if (posixly_correct)
 					goto synopsis;
-				opt_recursive = 1;
+				walk_flags |= WALK_TREE_RECURSIVE;
 				break;
 
 			case 'L':  /* follow all symlinks */
 				if (posixly_correct)
 					goto synopsis;
-				opt_walk_logical = 1;
-				opt_walk_physical = 0;
+				walk_flags |= WALK_TREE_LOGICAL;
+				walk_flags &= ~WALK_TREE_PHYSICAL;
 				break;
 
 			case 'P':  /* skip all symlinks */
 				if (posixly_correct)
 					goto synopsis;
-				opt_walk_logical = 0;
-				opt_walk_physical = 1;
+				walk_flags |= WALK_TREE_PHYSICAL;
+				walk_flags &= ~WALK_TREE_LOGICAL;
 				break;
 
 			case 's':  /* skip files with only base entries */
@@ -762,7 +702,8 @@ int main(int argc, char *argv[])
 				if (*line == '\0')
 					continue;
 
-				had_errors += walk_tree(line);
+				had_errors += walk_tree(line, walk_flags, 0,
+							do_print, NULL);
 			}
 			if (!feof(stdin)) {
 				fprintf(stderr, _("%s: Standard input: %s\n"),
@@ -770,7 +711,8 @@ int main(int argc, char *argv[])
 				had_errors++;
 			}
 		} else
-			had_errors += walk_tree(argv[optind]);
+			had_errors += walk_tree(argv[optind], walk_flags, 0,
+						do_print, NULL);
 		optind++;
 	} while (optind < argc);
 
Index: acl-2.2.45/libmisc/Makefile
===================================================================
--- acl-2.2.45.orig/libmisc/Makefile
+++ acl-2.2.45/libmisc/Makefile
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTLIBRARY = libmisc.la
 LTLDFLAGS =
 
-CFILES = quote.c unquote.c high_water_alloc.c next_line.c
+CFILES = quote.c unquote.c high_water_alloc.c next_line.c walk_tree.c
 
 default: $(LTLIBRARY)
 install install-dev install-lib:
Index: acl-2.2.45/setfacl/do_set.c
===================================================================
--- acl-2.2.45.orig/setfacl/do_set.c
+++ acl-2.2.45/setfacl/do_set.c
@@ -36,10 +36,10 @@
 #include "sequence.h"
 #include "parse.h"
 #include "config.h"
+#include "walk_tree.h"
 
 
 extern const char *progname;
-extern int opt_recursive;
 extern int opt_recalculate;
 extern int opt_test;
 extern int print_options;
@@ -259,8 +259,10 @@ int
 do_set(
 	const char *path_p,
 	const struct stat *st,
-	const seq_t seq)
+	int walk_flags,
+	void *arg)
 {
+	const seq_t seq = (const seq_t)arg;
 	acl_t old_acl = NULL, old_default_acl = NULL;
 	acl_t acl = NULL, default_acl = NULL;
 	acl_t *xacl, *old_xacl;
@@ -272,6 +274,16 @@ do_set(
 	int acl_modified = 0, default_acl_modified = 0;
 	int acl_mask_provided = 0, default_acl_mask_provided = 0;
 
+	if (walk_flags & WALK_TREE_FAILED) {
+		fprintf(stderr, "%s: %s: %s\n", progname, path_p, strerror(errno));
+		return 1;
+	}
+
+        if ((walk_flags & WALK_TREE_SYMLINK) &&
+	    ((walk_flags & WALK_TREE_PHYSICAL) ||
+	     !(walk_flags & (WALK_TREE_TOPLEVEL | WALK_TREE_LOGICAL))))
+		return 0;
+
 	/* Execute the commands in seq (read ACLs on demand) */
 	error = seq_get_cmd(seq, SEQ_FIRST_CMD, &cmd);
 	if (error == 0)
@@ -426,7 +438,7 @@ do_set(
 	}
 
 	/* Only directores can have default ACLs */
-	if (default_acl && !S_ISDIR(st->st_mode) && opt_recursive) {
+	if (default_acl && !S_ISDIR(st->st_mode) && (walk_flags & WALK_TREE_RECURSIVE)) {
 		/* In recursive mode, ignore default ACLs for files */
 		acl_free(default_acl);
 		default_acl = NULL;
Index: acl-2.2.45/setfacl/setfacl.c
===================================================================
--- acl-2.2.45.orig/setfacl/setfacl.c
+++ acl-2.2.45/setfacl/setfacl.c
@@ -28,20 +28,15 @@
 #include <sys/stat.h>
 #include <dirent.h>
 #include <libgen.h>
-#include <ftw.h>
 #include <getopt.h>
 #include <locale.h>
 #include "config.h"
 #include "sequence.h"
 #include "parse.h"
+#include "walk_tree.h"
 #include "misc.h"
 
-extern int
-do_set(
-        const char *path_p,
-        const struct stat *stat_p,
-	const seq_t seq);
-
+extern int do_set(const char *path_p, const struct stat *stat_p, int flags, void *arg);
 
 #define POSIXLY_CORRECT_STR "POSIXLY_CORRECT"
 
@@ -82,9 +77,7 @@ struct option long_options[] = {
 const char *progname;
 const char *cmd_line_options, *cmd_line_spec;
 
-int opt_recursive;  /* recurse into sub-directories? */
-int opt_walk_logical;  /* always follow symbolic links */
-int opt_walk_physical;  /* never follow symbolic links */
+int walk_flags = WALK_TREE_DEREFERENCE;
 int opt_recalculate;  /* recalculate mask entry (0=default, 1=yes, -1=no) */
 int opt_promote;  /* promote access ACL to default ACL */
 int opt_test;  /* do not write to the file system.
@@ -188,7 +181,7 @@ restore(
 		stat.st_uid = uid;
 		stat.st_gid = gid;
 
-		error = do_set(path_p, &stat, seq);
+		error = do_set(path_p, &stat, 0, seq);
 		if (error != 0) {
 			status = 1;
 			goto resume;
@@ -275,77 +268,6 @@ void help(void)
 }
 
 
-static int __errors;
-static seq_t __seq;
-
-int __do_set(const char *file, const struct stat *stat,
-             int flag, struct FTW *ftw)
-{
-	int saved_errno = errno;
-	
-	/* Process the target of a symbolic link, and traverse the link,
-	   only if doing a logical walk, or if the symbolic link was
-	   specified on the command line. Always skip symbolic links if
-	   doing a physical walk. */
-
-	if (S_ISLNK(stat->st_mode) &&
-	    (opt_walk_physical || (ftw->level > 0 && !opt_walk_logical)))
-		return 0;
-
-	if (do_set(file, stat, __seq))
-		__errors++;
-
-	if (flag == FTW_DNR && opt_recursive) {
-		/* Item is a directory which can't be read. */
-		fprintf(stderr, "%s: %s: %s\n",
-			progname, file, strerror(saved_errno));
-		return 0;
-	}
-
-	/* We also get here in non-recursive mode. In that case,
-	   return something != 0 to abort nftw. */
-
-	if (!opt_recursive)
-		return 1;
-
-	return 0;
-}
-
-char *resolve_symlinks(const char *file)
-{
-	static char buffer[4096];
-	struct stat stat;
-	char *path = NULL;
-
-	if (lstat(file, &stat) == -1)
-		return path;
-
-	if (S_ISLNK(stat.st_mode) && !opt_walk_physical)
-		path = realpath(file, buffer);
-	else
-		path = (char *)file;    /* not a symlink, use given path */
-
-	return path;
-}
-
-int walk_tree(const char *file, seq_t seq)
-{
-	const char *p;
-
-	__errors = 0;
-	__seq = seq;
-	if ((p = resolve_symlinks(file)) == NULL) {
-		fprintf(stderr, "%s: %s: %s\n", progname,
-			xquote(file), strerror(errno));
-		__errors++;
-	} else if (nftw(p, __do_set, 0, opt_walk_logical ? 0 : FTW_PHYS) < 0) {
-		fprintf(stderr, "%s: %s: %s\n", progname,
-			xquote(file), strerror(errno));
-		__errors++;
-	}
-	return __errors;
-}
-
 int next_file(const char *arg, seq_t seq)
 {
 	char *line;
@@ -353,14 +275,14 @@ int next_file(const char *arg, seq_t seq
 
 	if (strcmp(arg, "-") == 0) {
 		while ((line = next_line(stdin)))
-			errors = walk_tree(line, seq);
+			errors = walk_tree(line, walk_flags, 0, do_set, seq);
 		if (!feof(stdin)) {
 			fprintf(stderr, _("%s: Standard input: %s\n"),
 				progname, strerror(errno));
 			errors = 1;
 		}
 	} else {
-		errors = walk_tree(arg, seq);
+		errors = walk_tree(arg, walk_flags, 0, do_set, seq);
 	}
 	return errors ? 1 : 0;
 }
@@ -627,17 +549,17 @@ int main(int argc, char *argv[])
 				break;
 
 			case 'R':  /* recursive */
-				opt_recursive = 1;
+				walk_flags |= WALK_TREE_RECURSIVE;
 				break;
 
 			case 'L':  /* follow symlinks */
-				opt_walk_logical = 1;
-				opt_walk_physical = 0;
+				walk_flags |= WALK_TREE_LOGICAL;
+				walk_flags &= ~WALK_TREE_PHYSICAL;
 				break;
 
 			case 'P':  /* do not follow symlinks */
-				opt_walk_logical = 0;
-				opt_walk_physical = 1;
+				walk_flags |= WALK_TREE_PHYSICAL;
+				walk_flags &= ~WALK_TREE_LOGICAL;
 				break;
 
 			case 't':  /* test mode */
Index: acl-2.2.45/include/walk_tree.h
===================================================================
--- /dev/null
+++ acl-2.2.45/include/walk_tree.h
@@ -0,0 +1,19 @@
+#ifndef __WALK_TREE_H
+#define __WALK_TREE_H
+
+#define WALK_TREE_RECURSIVE	0x1
+#define WALK_TREE_PHYSICAL	0x2
+#define WALK_TREE_LOGICAL	0x4
+#define WALK_TREE_DEREFERENCE	0x8
+
+#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, unsigned int num,
+		     int (*func)(const char *, const struct stat *, int,
+				 void *), void *arg);
+
+#endif
Index: acl-2.2.45/libmisc/walk_tree.c
===================================================================
--- /dev/null
+++ acl-2.2.45/libmisc/walk_tree.c
@@ -0,0 +1,188 @@
+/*
+  File: walk_tree.c
+
+  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.
+*/
+
+#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>
+#include <errno.h>
+
+#include "walk_tree.h"
+
+struct entry_handle {
+	struct entry_handle *prev, *next;
+	struct stat st;
+	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_rec(const char *path, int walk_flags,
+			 int (*func)(const char *, const struct stat *, int,
+			 	     void *), void *arg, int depth)
+{
+	int (*xstat)(const char *, struct stat *) = lstat;
+	int flags = walk_flags, err;
+	struct entry_handle dir;
+
+	/*
+	 * 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, &dir.st) != 0)
+		return func(path, NULL, flags | WALK_TREE_FAILED, arg);
+	if (S_ISLNK(dir.st.st_mode)) {
+		flags |= WALK_TREE_SYMLINK;
+		if (flags & WALK_TREE_DEREFERENCE) {
+			xstat = stat;
+			goto follow_symlink;
+		}
+	}
+	err = func(path, &dir.st, flags, arg);
+	if ((flags & WALK_TREE_RECURSIVE) &&
+	    (S_ISDIR(dir.st.st_mode) || (S_ISLNK(dir.st.st_mode))) &&
+	    (!(flags & WALK_TREE_PHYSICAL) || !(flags & WALK_TREE_SYMLINK)) &&
+	    (flags & (WALK_TREE_LOGICAL | WALK_TREE_TOPLEVEL))) {
+		struct entry_handle *i;
+		struct dirent *entry;
+
+		/* Check if we have already visited this directory. */
+		for (i = head.next; i != &head; i = i->next)
+			if (i->st.st_dev == dir.st.st_dev &&
+			    i->st.st_ino == dir.st.st_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, &dir.st,
+					    flags | WALK_TREE_FAILED, arg);
+			return err;
+		}
+
+		/* 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;
+			path_end = strchr(path, 0);
+			if ((path_end - path) + strlen(entry->d_name) + 1 >=
+			    FILENAME_MAX) {
+				errno = ENAMETOOLONG;
+				err += func(path, NULL,
+					    flags | WALK_TREE_FAILED, arg);
+				continue;
+			}
+			*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, &dir.st, flags |
+						    WALK_TREE_FAILED, arg);
+				seekdir(dir.stream, dir.pos);
+
+				closed = closed->next;
+				num_dir_handles--;
+			}
+		}
+
+		if (closedir(dir.stream) != 0)
+			err += func(path, &dir.st, flags | WALK_TREE_FAILED,
+				    arg);
+
+		/* Remove from the list of handles. */
+		dir.prev->next = dir.next;
+		dir.next->prev = dir.prev;
+		num_dir_handles++;
+	}
+	return err;
+}
+
+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);
+	}
+	strcpy(path_copy, path);
+	return walk_tree_rec(path_copy, walk_flags, func, arg, 0);
+}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: walk-attr.diff --]
[-- Type: text/x-diff; charset="us-ascii"; name="walk-attr.diff", Size: 31168 bytes --]

Index: attr-2.4.39/getfattr/getfattr.c
===================================================================
--- attr-2.4.39.orig/getfattr/getfattr.c
+++ attr-2.4.39/getfattr/getfattr.c
@@ -28,11 +28,11 @@
 #include <ctype.h>
 #include <getopt.h>
 #include <regex.h>
-#include <ftw.h>
 #include <locale.h>
 
 #include <attr/xattr.h>
 #include "config.h"
+#include "walk_tree.h"
 #include "misc.h"
 
 #define CMD_LINE_OPTIONS "n:de:m:hRLP"
@@ -54,11 +54,8 @@ struct option long_options[] = {
 	{ NULL,			0, 0, 0 }
 };
 
-int opt_recursive;  /* recurse into sub-directories? */
-int opt_walk_logical;  /* always follow symbolic links */
-int opt_walk_physical;  /* never follow symbolic links */
+int walk_flags = WALK_TREE_DEREFERENCE;
 int opt_dump;  /* dump attribute values (or only list the names) */
-int opt_deref = 1;  /* dereference symbolic links */
 char *opt_name;  /* dump named attributes */
 char *opt_name_pattern = "^user\\.";  /* include only matching names */
 char *opt_encoding;  /* encode values automatically (NULL), or as "text",
@@ -84,12 +81,14 @@ static const char *xquote(const char *st
 
 int do_getxattr(const char *path, const char *name, void *value, size_t size)
 {
-	return (opt_deref ? getxattr : lgetxattr)(path, name, value, size);
+	return ((walk_flags & WALK_TREE_DEREFERENCE) ?
+		getxattr : lgetxattr)(path, name, value, size);
 }
 
 int do_listxattr(const char *path, char *list, size_t size)
 {
-	return (opt_deref ? listxattr : llistxattr)(path, list, size);
+	return ((walk_flags & WALK_TREE_DEREFERENCE) ?
+		listxattr : llistxattr)(path, list, size);
 }
 
 const char *strerror_ea(int err)
@@ -347,21 +346,19 @@ int list_attributes(const char *path, in
 	return 0;
 }
 
-int do_print(const char *path, const struct stat *stat,
-             int flag, struct FTW *ftw)
+int do_print(const char *path, const struct stat *stat, int walk_flags, void *unused)
 {
-	int saved_errno = errno;
 	int header_printed = 0;
 
-	/*
-	 * Process the target of a symbolic link, and traverse the
-	 * link, only if doing a logical walk, or if the symbolic link
-	 * was specified on the command line. Always skip symbolic
-	 * links if doing a physical walk.
-	 */
+	if (walk_flags & WALK_TREE_FAILED) {
+		fprintf(stderr, "%s: %s: %s\n", progname, xquote(path), strerror(errno));
+		return 1;
+	}
 
-	if (S_ISLNK(stat->st_mode) &&
-	    (opt_walk_physical || (ftw->level > 0 && !opt_walk_logical)))
+	if ((walk_flags & WALK_TREE_SYMLINK) &&
+	    (walk_flags & WALK_TREE_DEREFERENCE) &&
+	    ((walk_flags & WALK_TREE_PHYSICAL) ||
+	    !(walk_flags & (WALK_TREE_TOPLEVEL | WALK_TREE_LOGICAL))))
 		return 0;
 
 	if (opt_name)
@@ -371,21 +368,6 @@ int do_print(const char *path, const str
 
 	if (header_printed)
 		puts("");
-
-	if (flag == FTW_DNR && opt_recursive) {
-		/* Item is a directory which can't be read. */
-		fprintf(stderr, "%s: %s: %s\n", progname, xquote(path),
-			strerror(saved_errno));
-		return 0;
-	}
-
-	/*
-	 * We also get here in non-recursive mode. In that case,
-	 *  return something != 0 to abort nftw.
-	 */
-
-	if (!opt_recursive)
-		return 1;
 	return 0;
 }
 
@@ -410,39 +392,6 @@ void help(void)
 "      --help              this help text\n"));
 }
 
-char *resolve_symlinks(const char *file)
-{
-	static char buffer[4096];
-	struct stat stat;
-	char *path = NULL;
-
-	if (lstat(file, &stat) == -1)
-		return path;
-
-	if (S_ISLNK(stat.st_mode) && !opt_walk_physical)
-		path = realpath(file, buffer);
-	else
-		path = (char *)file;    /* not a symlink, use given path */
-
-	return path;
-}
-
-int walk_tree(const char *file)
-{
-	const char *p;
-
-	if ((p = resolve_symlinks(file)) == NULL) {
-		fprintf(stderr, "%s: %s: %s\n", progname,
-			xquote(file), strerror(errno));
-		return 1;
-	} else if (nftw(p, do_print, 0, opt_walk_logical? 0 : FTW_PHYS) < 0) {
-		fprintf(stderr, "%s: %s: %s\n", progname, xquote(file),
-			strerror(errno));
-		return 1;
-	}
-	return 0;
-}
-
 int main(int argc, char *argv[])
 {
 	int opt;
@@ -478,7 +427,7 @@ int main(int argc, char *argv[])
 				return 0;
 
 			case 'h': /* do not dereference symlinks */
-				opt_deref = 0;
+				walk_flags &= ~WALK_TREE_DEREFERENCE;
 				break;
 
 			case 'n':  /* get named attribute */
@@ -497,17 +446,17 @@ int main(int argc, char *argv[])
 				break;
 
 			case 'L':
-				opt_walk_logical = 1;
-				opt_walk_physical = 0;
+				walk_flags |= WALK_TREE_LOGICAL;
+				walk_flags &= ~WALK_TREE_PHYSICAL;
 				break;
 
 			case 'P':
-				opt_walk_logical = 0;
-				opt_walk_physical = 1;
+				walk_flags |= WALK_TREE_PHYSICAL;
+				walk_flags &= ~WALK_TREE_LOGICAL;
 				break;
 
 			case 'R':
-				opt_recursive = 1;
+				walk_flags |= WALK_TREE_RECURSIVE;
 				break;
 
 			case 'V':
@@ -531,7 +480,8 @@ int main(int argc, char *argv[])
 	}
 
 	while (optind < argc) {
-		had_errors += walk_tree(argv[optind]);
+		had_errors += walk_tree(argv[optind], walk_flags, 0,
+					do_print, NULL);
 		optind++;
 	}
 
Index: attr-2.4.39/libmisc/Makefile
===================================================================
--- attr-2.4.39.orig/libmisc/Makefile
+++ attr-2.4.39/libmisc/Makefile
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTLIBRARY = libmisc.la
 LTLDFLAGS =
 
-CFILES = quote.c unquote.c high_water_alloc.c next_line.c
+CFILES = quote.c unquote.c high_water_alloc.c next_line.c walk_tree.c
 
 default: $(LTLIBRARY)
 install install-dev install-lib:
Index: attr-2.4.39/test/attr.test
===================================================================
--- attr-2.4.39.orig/test/attr.test
+++ attr-2.4.39/test/attr.test
@@ -10,6 +10,9 @@ Execute this test using the `run' script
 
 Try various valid and invalid names
 	
+	$ mkdir d
+	$ cd d
+
 	$ touch f
 	$ setfattr -n user -v value f
 	> setfattr: f: Operation not supported
@@ -29,8 +32,8 @@ Try various valid and invalid names
 Size checks, for an ext2/ext3 file system with a block size of 4K
 
 	$ touch f
-	$ setfattr -n user.name -v 4040+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ f
-	$ setfattr -n user.name -v 4041++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ f
+	$ setfattr -n user.name -v 4040++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ f
+	$ setfattr -n user.name -v 4041+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ f
 	> setfattr: f: No space left on device
 	
 	$ rm f
@@ -86,13 +89,6 @@ Value encodings
 	> user.name3=0s3vrO
 	> 
 	
-	$ getfattr -d -e text f
-	> # file: f
-	> user.name="º¾"
-	> user.name2="Þ­¾ï"
-	> user.name3="ÞúÎ"
-	> 
-	
 	$ rm f
 
 Everything with one file
@@ -105,7 +101,7 @@ Everything with one file
 	$ setfattr -n user.short -v value f
 	$ setfattr -n user.novalue-yet f
 	$ ls -s f
-	>    4 f
+	> 4 f
 	
 	$ getfattr -d f
 	> # file: f
@@ -143,7 +139,7 @@ Everything with one file
 	$ setfattr -x user.novalue-yet f
 	$ getfattr -d f
 	$ ls -s f
-	>    0 f
+	> 0 f
 	
 	$ rm f
 
@@ -152,15 +148,15 @@ Test extended attribute block sharing
 	$ touch f g h
 	$ setfattr -n user.novalue f g h
 	$ ls -s f g h
-	>    4 f
-	>    4 g
-	>    4 h
+	> 4 f
+	> 4 g
+	> 4 h
 	
 	$ setfattr -n user.name -v value f
 	$ ls -s f g h
-	>    4 f
-	>    4 g
-	>    4 h
+	> 4 f
+	> 4 g
+	> 4 h
 	
 	$ getfattr -d f g h
 	> # file: f
@@ -176,15 +172,15 @@ Test extended attribute block sharing
 	
 	$ setfattr -n user.name -v value g
 	$ ls -s f g h
-	>    4 f
-	>    4 g
-	>    4 h
+	> 4 f
+	> 4 g
+	> 4 h
 	
 	$ setfattr -x user.novalue h
 	$ ls -s f g h
-	>    4 f
-	>    4 g
-	>    0 h
+	> 4 f
+	> 4 g
+	> 0 h
 	
 	$ getfattr -d f g h
 	> # file: f
@@ -201,9 +197,9 @@ Test extended attribute block sharing
 	$ setfattr -x user.name f g
 	$ setfattr -x user.novalue f g
 	$ ls -s f g h
-	>    0 f
-	>    0 g
-	>    0 h
+	> 0 f
+	> 0 g
+	> 0 h
 	
 	$ rm f g h
 
@@ -260,6 +256,5 @@ Tests for attribute names that contains 
 	$ setfattr -x "user.special\\007" f
 	$ rm f
 
-Some POSIX ACL tests...
-
-	$ touch f
+	$ cd ..
+	$ rm -rf d
Index: attr-2.4.39/include/walk_tree.h
===================================================================
--- /dev/null
+++ attr-2.4.39/include/walk_tree.h
@@ -0,0 +1,19 @@
+#ifndef __WALK_TREE_H
+#define __WALK_TREE_H
+
+#define WALK_TREE_RECURSIVE	0x1
+#define WALK_TREE_PHYSICAL	0x2
+#define WALK_TREE_LOGICAL	0x4
+#define WALK_TREE_DEREFERENCE	0x8
+
+#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, unsigned int num,
+		     int (*func)(const char *, const struct stat *, int,
+				 void *), void *arg);
+
+#endif
Index: attr-2.4.39/libmisc/walk_tree.c
===================================================================
--- /dev/null
+++ attr-2.4.39/libmisc/walk_tree.c
@@ -0,0 +1,188 @@
+/*
+  File: walk_tree.c
+
+  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.
+*/
+
+#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>
+#include <errno.h>
+
+#include "walk_tree.h"
+
+struct entry_handle {
+	struct entry_handle *prev, *next;
+	struct stat st;
+	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_rec(const char *path, int walk_flags,
+			 int (*func)(const char *, const struct stat *, int,
+			 	     void *), void *arg, int depth)
+{
+	int (*xstat)(const char *, struct stat *) = lstat;
+	int flags = walk_flags, err;
+	struct entry_handle dir;
+
+	/*
+	 * 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, &dir.st) != 0)
+		return func(path, NULL, flags | WALK_TREE_FAILED, arg);
+	if (S_ISLNK(dir.st.st_mode)) {
+		flags |= WALK_TREE_SYMLINK;
+		if (flags & WALK_TREE_DEREFERENCE) {
+			xstat = stat;
+			goto follow_symlink;
+		}
+	}
+	err = func(path, &dir.st, flags, arg);
+	if ((flags & WALK_TREE_RECURSIVE) &&
+	    (S_ISDIR(dir.st.st_mode) || (S_ISLNK(dir.st.st_mode))) &&
+	    (!(flags & WALK_TREE_PHYSICAL) || !(flags & WALK_TREE_SYMLINK)) &&
+	    (flags & (WALK_TREE_LOGICAL | WALK_TREE_TOPLEVEL))) {
+		struct entry_handle *i;
+		struct dirent *entry;
+
+		/* Check if we have already visited this directory. */
+		for (i = head.next; i != &head; i = i->next)
+			if (i->st.st_dev == dir.st.st_dev &&
+			    i->st.st_ino == dir.st.st_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, &dir.st,
+					    flags | WALK_TREE_FAILED, arg);
+			return err;
+		}
+
+		/* 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;
+			path_end = strchr(path, 0);
+			if ((path_end - path) + strlen(entry->d_name) + 1 >=
+			    FILENAME_MAX) {
+				errno = ENAMETOOLONG;
+				err += func(path, NULL,
+					    flags | WALK_TREE_FAILED, arg);
+				continue;
+			}
+			*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, &dir.st, flags |
+						    WALK_TREE_FAILED, arg);
+				seekdir(dir.stream, dir.pos);
+
+				closed = closed->next;
+				num_dir_handles--;
+			}
+		}
+
+		if (closedir(dir.stream) != 0)
+			err += func(path, &dir.st, flags | WALK_TREE_FAILED,
+				    arg);
+
+		/* Remove from the list of handles. */
+		dir.prev->next = dir.next;
+		dir.next->prev = dir.prev;
+		num_dir_handles++;
+	}
+	return err;
+}
+
+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);
+	}
+	strcpy(path_copy, path);
+	return walk_tree_rec(path_copy, walk_flags, func, arg, 0);
+}
Index: attr-2.4.39/test/getfattr.test
===================================================================
--- /dev/null
+++ attr-2.4.39/test/getfattr.test
@@ -0,0 +1,52 @@
+	$ mkdir d
+	$ cd d
+
+	$ touch f
+	$ setfattr -n user.test -v test f
+	$ ln -s f l
+
+This case should be obvious:
+	$ getfattr -d f
+	> # file: f
+	> user.test="test"
+	>
+
+If a symlink is explicitly specified on the command line, follow it
+(-H behavior):
+	$ getfattr -d l
+	> # file: l
+	> user.test="test"
+	>
+
+Unless we are explicitly told not to dereference symlinks:
+	$ getfattr -hd l
+
+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 .
+	> # file: f
+	> user.test="test"
+	>
+
+This case works as expected:
+	$ getfattr -Rhd .
+	> # file: f
+	> user.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:
+	$ ln -s . here
+	$ getfattr -Rd here
+	> # file: here/f
+	> user.test="test"
+	>
+
+	$ getfattr -Rhd here
+	> # file: here/f
+	> user.test="test"
+	>
+
+	$ cd ..
+	$ rm -rf d

Re: acl and attr: Fix path walking code

[Timothy Shimmin]

Hi Andreas,

Andreas Gruenbacher wrote:
> Hello,
> 
> the tree walking code in acl and attr broke when resolve_symlinks() was 
> introduced (by me, unfortunately). Following symlinks passed in on the 
> command line is the intended behavior for the tools (unless in -P mode). The 
> first version was buggy, and so someone "fixed" it by replacing readlink() 
> with realpath() in resolve_symlinks().
> 
> The result is that the output of getfattr and getfacl will show pathnames that 
> may point anywhere. When processing a directory tree it sometimes is helpful 
> to treat symlinks as regular files, but resolving the pathnames is totally 
> wrong.
> 
> After runnig into problem after problem with nftw and never ending up with 
> even half-way clean code, I think it's time to ditch it altogether and 
> replace it with sane code. So here are two patches, one for attr and one for 
> acl, that does that.
> 
> Files include/walk_tree.h and libmisc/walk_tree.c are identical in both 
> patches; that code is shared between the two packages.
> 
> Okay to apply?
> 
> Thanks,
> Andreas
> 

I applied attr patch and tried it out on xfstests/062
(which I believe was based on one of your tests).

==========================================================
--- 062.out     2006-03-28 12:52:32.000000000 +1000
+++ 062.out.bad 2007-11-09 15:38:09.000000000 +1100
@@ -526,6 +526,10 @@
  user.name=0xbabe
  user.name3=0xdeface

+# file: SCRATCH_MNT/lnk
+trusted.name=0xbabe
+trusted.name3=0xdeface
+
  # file: SCRATCH_MNT/dev/b
  trusted.name=0xbabe
  trusted.name3=0xdeface
@@ -562,6 +566,10 @@
  user.1=0x3233
  user.x=0x797a

+# file: SCRATCH_MNT/descend/and/ascend
+trusted.9=0x3837
+trusted.a=0x6263
+

  *** directory descent without following symlinks
  # file: SCRATCH_MNT/reg
==========================================================

So for the following of symlinks with getfattr -L
i.e.
   echo "*** directory descent with us following symlinks"
   getfattr -h -L -R -m '.' -e hex $SCRATCH_MNT

Looking at the 2nd difference...
It now picks up descend/and/ascend which contains the symlink
of descend/and --> here/up.
So that makes sense, it is following a symlink which it
didn't before and finding a dir, "up" in the linked dir.
Good.

Looking at 1st difference...
It is now showing up "lnk" which is a symlink: lnk --> dir
So why is it showing this up
and yet it is not showing descend/and (which is a link to here/up)?
So yes we are following symlinks but are we supposed
to just do the symlinks themselves as well?

BTW, do we not allow user EAs on symlinks? (I've forgotten)

--Tim

Re: acl and attr: Fix path walking code

[Andreas Gruenbacher]

On Friday 09 November 2007 06:41:21 Timothy Shimmin wrote:
> I applied attr patch and tried it out on xfstests/062
> (which I believe was based on one of your tests).
>
> ==========================================================
> --- 062.out     2006-03-28 12:52:32.000000000 +1000
> +++ 062.out.bad 2007-11-09 15:38:09.000000000 +1100
> @@ -526,6 +526,10 @@
>   user.name=0xbabe
>   user.name3=0xdeface
>
> +# file: SCRATCH_MNT/lnk
> +trusted.name=0xbabe
> +trusted.name3=0xdeface
> +
>   # file: SCRATCH_MNT/dev/b
>   trusted.name=0xbabe
>   trusted.name3=0xdeface
> @@ -562,6 +566,10 @@
>   user.1=0x3233
>   user.x=0x797a
>
> +# file: SCRATCH_MNT/descend/and/ascend
> +trusted.9=0x3837
> +trusted.a=0x6263
> +
>
>   *** directory descent without following symlinks
>   # file: SCRATCH_MNT/reg
> ==========================================================
>
> So for the following of symlinks with getfattr -L
> i.e.
>    echo "*** directory descent with us following symlinks"
>    getfattr -h -L -R -m '.' -e hex $SCRATCH_MNT
>
> Looking at the 2nd difference...
> It now picks up descend/and/ascend which contains the symlink
> of descend/and --> here/up.
> So that makes sense, it is following a symlink which it
> didn't before and finding a dir, "up" in the linked dir.
> Good.
>
> Looking at 1st difference...
> It is now showing up "lnk" which is a symlink: lnk --> dir
> So why is it showing this up
> and yet it is not showing descend/and (which is a link to here/up)?
> So yes we are following symlinks but are we supposed
> to just do the symlinks themselves as well?

With -h, the utilities operate on the symlinks rather than the files that the 
symlinks point to. The test case sets attributes on SCRATCH_MNT/lnk, but not 
on descend/and.

The -h and -L options together don't make much sense actually.

> BTW, do we not allow user EAs on symlinks? (I've forgotten)

No we don't --- that's explained on attr(5).

Thanks for looking at this!

Andreas

Re: acl and attr: Fix path walking code

[Timothy Shimmin]

Andreas Gruenbacher wrote:
 > On Friday 09 November 2007 06:41:21 Timothy Shimmin wrote:
 >> I applied attr patch and tried it out on xfstests/062
 >> (which I believe was based on one of your tests).
 >>
 >> ==========================================================
 >> --- 062.out     2006-03-28 12:52:32.000000000 +1000
 >> +++ 062.out.bad 2007-11-09 15:38:09.000000000 +1100
 >> @@ -526,6 +526,10 @@
 >>   user.name=0xbabe
 >>   user.name3=0xdeface
 >>
 >> +# file: SCRATCH_MNT/lnk
 >> +trusted.name=0xbabe
 >> +trusted.name3=0xdeface
 >> +
 >>   # file: SCRATCH_MNT/dev/b
 >>   trusted.name=0xbabe
 >>   trusted.name3=0xdeface
 >> @@ -562,6 +566,10 @@
 >>   user.1=0x3233
 >>   user.x=0x797a
 >>
 >> +# file: SCRATCH_MNT/descend/and/ascend
 >> +trusted.9=0x3837
 >> +trusted.a=0x6263
 >> +
 >>
 >>   *** directory descent without following symlinks
 >>   # file: SCRATCH_MNT/reg
 >> ==========================================================
 >>
 >> So for the following of symlinks with getfattr -L
 >> i.e.
 >>    echo "*** directory descent with us following symlinks"
 >>    getfattr -h -L -R -m '.' -e hex $SCRATCH_MNT
 >>
 >> Looking at the 2nd difference...
 >> It now picks up descend/and/ascend which contains the symlink
 >> of descend/and --> here/up.
 >> So that makes sense, it is following a symlink which it
 >> didn't before and finding a dir, "up" in the linked dir.
 >> Good.
 >>
 >> Looking at 1st difference...
 >> It is now showing up "lnk" which is a symlink: lnk --> dir
 >> So why is it showing this up
 >> and yet it is not showing descend/and (which is a link to here/up)?
 >> So yes we are following symlinks but are we supposed
 >> to just do the symlinks themselves as well?
 >
 > With -h, the utilities operate on the symlinks rather than the files that the
 > symlinks point to. The test case sets attributes on SCRATCH_MNT/lnk, but not
 > on descend/and.

Oops, yep, there is no EA on descend/and.

 >
 > The -h and -L options together don't make much sense actually.
 >
No they don't :)
So will it not follow the argument but follow any descendents that
it finds on the walk.
It kind of looked from the manpage that the -h is about just
the argument and not about the walk.

<okay you clarified this below...>

Anyway, I took out the -h with the -L,
i.e. $ getfattr  -L -R -m '.' -e hex $SCRATCH_MNT

And it is still reporting
 >> +# file: SCRATCH_MNT/lnk
 >> +trusted.name=0xbabe
 >> +trusted.name3=0xdeface
 >> +

So I presume following symlinks also mean operating on symlinks too
(i.e. getting the EA)?

---


 > On Friday 09 November 2007 08:39:56 Timothy Shimmin wrote:
 >> > You mention -L/-P is like chown.
 >> > However, -P for getattr isn't about not walking symlinks
 >> > to directories,
 >> > it's about skipping symlinks altogether, right?
 >
 > Hmm, -L and -P define which files and directories are visited, and -h defines
 > whether we are looking at symlinks or the files they point to. The two
 > concepts are orthogonal. -P is not about skipping symlinks, only about not
 > recursing into them.
 >
Oh okay.
There is the concept of following the symlink for traversal versus
following the symlink to get the EA on.

So with -L should it just follow the symlink or look at the symlink first
and then follow it?
And will -h modify this behavior?
I'm still confused about the 1st difference in 062 output.

I wonder if the man pages can be clarified in this area :)

--Tim

Re: acl and attr: Fix path walking code

[Timothy Shimmin]

> 
> 
>  > On Friday 09 November 2007 08:39:56 Timothy Shimmin wrote:
>  >> > You mention -L/-P is like chown.
>  >> > However, -P for getattr isn't about not walking symlinks
>  >> > to directories,
>  >> > it's about skipping symlinks altogether, right?
>  >
>  > Hmm, -L and -P define which files and directories are visited, and -h 
> defines
>  > whether we are looking at symlinks or the files they point to. The two
>  > concepts are orthogonal. -P is not about skipping symlinks, only 
> about not
>  > recursing into them.
>  >
> Oh okay.
> There is the concept of following the symlink for traversal versus
> following the symlink to get the EA on.
> 
> So with -L should it just follow the symlink or look at the symlink first
> and then follow it?
> And will -h modify this behavior?
> I'm still confused about the 1st difference in 062 output.
> 
> I wonder if the man pages can be clarified in this area :)
> 
> --Tim
> 

Okay, looked at the code.

---

no -h => stat, getxattr, listxattr
-h    => lstat, lgetxattr, llistxattr

-P => skip symlinks (as soon as see them, then return from place in walk)

-L => process symlink and then opendir on symlink (hence follow/traverse it)

default => -L for argument (depth==1) and -P for subdirs (depth>1)

----

This was different than my intuition.
I am happy with -h.
For -P, I was expecting it to process symlink but not follow/traverse it,
I wouldn't think it should just skip them altogether (I realise the
man page says that).

For -L, I wasn't sure if it should process it first and then traverse
or simply just traverse.

So were these your intentions?
If so, the code seems to follow them but it would be nicer to
have more explanation in the man page.

--Tim

Re: acl and attr: Fix path walking code

[Andreas Gruenbacher]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On Tuesday 20 November 2007 06:11:12 Timothy Shimmin wrote:
> Okay, looked at the code.
>
> ---
>
> no -h => stat, getxattr, listxattr
> -h    => lstat, lgetxattr, llistxattr

Yes.

> -P => skip symlinks (as soon as see them, then return from place in walk)

No, -h never skips symlinks. (But depending on -L and -P, it may not follow 
symlinks to directories.) Here is an additional comment for do_print, and an 
equivalent version of the if in there. I hope that this will finally clarify 
the code.

> it would be nicer to have more explanation in the man page.

Agreed. How about the attached manpage patches?

Thanks,
Andreas

[-- Attachment #2: equivalent-version.diff --]
[-- Type: text/x-diff, Size: 890 bytes --]

Index: attr-2.4.39/getfattr/getfattr.c
===================================================================
--- attr-2.4.39.orig/getfattr/getfattr.c
+++ attr-2.4.39/getfattr/getfattr.c
@@ -355,14 +355,15 @@ int do_print(const char *path, const str
 		return 1;
 	}
 
-	 /*
-	 * When doing a physical walk or neither doing a logical walk nor processing a
-	 * direct command like argument, do not dereference symlinks.
+	/*
+	 * Only dereference symlinks when doing a logical walk, or when procesing
+	 * a direct command-line argument while not doing a physical walk.
 	 */
 	if ((walk_flags & WALK_TREE_SYMLINK) &&
 	    (walk_flags & WALK_TREE_DEREFERENCE) &&
+	    !(walk_flags & WALK_TREE_LOGICAL) &&
 	    ((walk_flags & WALK_TREE_PHYSICAL) ||
-	    !(walk_flags & (WALK_TREE_TOPLEVEL | WALK_TREE_LOGICAL))))
+	     !(walk_flags & (WALK_TREE_TOPLEVEL))))
 		return 0;
 
 	if (opt_name)

[-- Attachment #3: comment.diff --]
[-- Type: text/x-diff, Size: 554 bytes --]

Index: attr-2.4.39/getfattr/getfattr.c
===================================================================
--- attr-2.4.39.orig/getfattr/getfattr.c
+++ attr-2.4.39/getfattr/getfattr.c
@@ -355,6 +355,10 @@ int do_print(const char *path, const str
 		return 1;
 	}
 
+	 /*
+	 * When doing a physical walk or neither doing a logical walk nor processing a
+	 * direct command like argument, do not dereference symlinks.
+	 */
 	if ((walk_flags & WALK_TREE_SYMLINK) &&
 	    (walk_flags & WALK_TREE_DEREFERENCE) &&
 	    ((walk_flags & WALK_TREE_PHYSICAL) ||

[-- Attachment #4: man-acl.diff --]
[-- Type: text/x-diff, Size: 2461 bytes --]

Index: acl-2.2.45/man/man1/getfacl.1
===================================================================
--- acl-2.2.45.orig/man/man1/getfacl.1
+++ acl-2.2.45/man/man1/getfacl.1
@@ -100,13 +100,14 @@ Skip files that only have the base ACL e
 List the ACLs of all files and directories recursively.
 .TP
 .I \-L, \-\-logical
-Logical walk, follow symbolic links. The default behavior is to follow
-symbolic link arguments, and to skip symbolic links encountered in
-subdirectories.
+Logical walk, follow symbolic links to directories. The default behavior is to follow
+symbolic link arguments, and skip symbolic links encountered in subdirectories.
+Only effective in combination with \-R.
 .TP
 .I \-P, \-\-physical
-Physical walk, skip all symbolic links. This also skips symbolic link
-arguments.
+Physical walk, do not follow symbolic links to directories. This also skips symbolic
+link arguments.
+Only effective in combination with \-R.
 .TP
 .I \-\-tabular
 Use an alternative tabular output format. The ACL and the default ACL are displayed side by side. Permissions that are ineffective due to the ACL mask entry are displayed capitalized. The entry tag names for the ACL_USER_OBJ and ACL_GROUP_OBJ entries are also displayed in capital letters, which helps in spotting those entries.
Index: acl-2.2.45/man/man1/setfacl.1
===================================================================
--- acl-2.2.45.orig/man/man1/setfacl.1
+++ acl-2.2.45/man/man1/setfacl.1
@@ -104,11 +104,15 @@ Test mode. Instead of changing the ACLs 
 Apply operations to all files and directories recursively. This option cannot be mixed with `\-\-restore'.
 .TP 4
 .I \-L, \-\-logical
-Logical walk, follow symbolic links. The default behavior is to follow symbolic link arguments, and to skip symbolic links encountered
-in subdirectories. This option cannot be mixed with `\-\-restore'.
+Logical walk, follow symbolic links to directories. The default behavior is to follow
+symbolic link arguments, and skip symbolic links encountered in subdirectories.
+Only effective in combination with \-R.
+This option cannot be mixed with `\-\-restore'.
 .TP 4
 .I \-P, \-\-physical
-Physical walk, skip all symbolic links. This also skips symbolic link arguments.
+Physical walk, do not follow symbolic links to directories.
+This also skips symbolic link arguments.
+Only effective in combination with \-R.
 This option cannot be mixed with `\-\-restore'.
 .TP 4
 .I \-\-version

[-- Attachment #5: man-attr.diff --]
[-- Type: text/x-diff, Size: 1351 bytes --]

Index: attr-2.4.39/man/man1/getfattr.1
===================================================================
--- attr-2.4.39.orig/man/man1/getfattr.1
+++ attr-2.4.39/man/man1/getfattr.1
@@ -56,11 +56,8 @@ while strings encoded as hexidecimal and
 0x and 0s, respectively.
 .TP
 .BR \-h ", " \-\-no-dereference
-Do not follow symlinks.
-If
-.I pathname
-is a symbolic link, the symbolic link itself is examined,
-rather than the file the link refers to.
+Do not dereference symlinks. Instead of the file a symlink refers to, the
+symlink itself is examined.
 .TP
 .BR \-m " \f2pattern\f1, " \-\-match "=\f2pattern\f1"
 Only include attributes with names matching the regular expression
@@ -85,13 +82,15 @@ Dump out the extended attribute value(s)
 List the attributes of all files and directories recursively.
 .TP
 .BR \-L ", " \-\-logical
-Logical walk, follow symbolic links.
+Logical walk, follow symbolic links to directories.
 The default behaviour is to follow symbolic link arguments, and to 
 skip symbolic links encountered in subdirectories.
+Only effective in combination with \-R.
 .TP
 .BR \-P ", " \-\-physical
-Physical walk, skip all symbolic links.
+Physical walk, do not follow symbolic links to directories.
 This also skips symbolic link arguments.
+Only effective in combination with \-R.
 .TP
 .B \-\-version
 Print the version of

Re: acl and attr: Fix path walking code

[Timothy Shimmin]

Andreas Gruenbacher wrote:
> Hello,
> 
> the tree walking code in acl and attr broke when resolve_symlinks() was 
> introduced (by me, unfortunately). Following symlinks passed in on the 
> command line is the intended behavior for the tools (unless in -P mode). The 
> first version was buggy, and so someone "fixed" it by replacing readlink() 
> with realpath() in resolve_symlinks().
> 
> The result is that the output of getfattr and getfacl will show pathnames that 
> may point anywhere. When processing a directory tree it sometimes is helpful 
> to treat symlinks as regular files, but resolving the pathnames is totally 
> wrong.
> 
> After runnig into problem after problem with nftw and never ending up with 
> even half-way clean code, I think it's time to ditch it altogether and 
> replace it with sane code. So here are two patches, one for attr and one for 
> acl, that does that.
> 
> Files include/walk_tree.h and libmisc/walk_tree.c are identical in both 
> patches; that code is shared between the two packages.
> 
> Okay to apply?
> 
> Thanks,
> Andreas
> 

You mention -L/-P is like chown.
However, -P for getattr isn't about not walking symlinks
to directories,
it's about skipping symlinks altogether, right?

--Tim

Re: acl and attr: Fix path walking code

[Andreas Gruenbacher]

On Friday 09 November 2007 08:39:56 Timothy Shimmin wrote:
> You mention -L/-P is like chown.
> However, -P for getattr isn't about not walking symlinks
> to directories,
> it's about skipping symlinks altogether, right?

Hmm, -L and -P define which files and directories are visited, and -h defines 
whether we are looking at symlinks or the files they point to. The two 
concepts are orthogonal. -P is not about skipping symlinks, only about not 
recursing into them.

You can do this (as root), for example:

	$ ln -s dead link
	$ setfattr -h -n trusted.name -v value link
	$ getfattr -h -m- -d -P link
	# file: link
	trusted.name="value"

With "getfattr -R -P -h" you get a physical dump of all attributes ("a real, 
complete dump"), while with "getfattr -R -L" you get a logical dump that 
treats all symlinks as the files they point to. I somewhat doubt that -L 
with -h has real value.

Thanks,
Andreas