Unify broken symlink handling

Rather than open-code the fallback logic for broken symlinks everywhere
it's needed, introduce a new xfstatat() utility function that performs
the fallback automatically.

Using xfstatat() consistently fixes a few bugs, including cases where
broken symlinks are given as arguments to predicates like -samefile.

11 files changed, 74 insertions, 33 deletions

diff --git a/bftw.c b/bftw.c
index ce3a567..02e7adb 100644
--- a/bftw.c
+++ b/bftw.c
@@ -551,15 +551,12 @@ static void bftw_queue_destroy(struct bftw_queue *queue) {
 
 /** Call stat() and use the results. */
 static int ftwbuf_stat(struct BFTW *ftwbuf, struct stat *sb) {
-	int ret = fstatat(ftwbuf->at_fd, ftwbuf->at_path, sb, ftwbuf->at_flags);
-	if (ret != 0) {
-		return ret;
+	int ret = xfstatat(ftwbuf->at_fd, ftwbuf->at_path, sb, &ftwbuf->at_flags);
+	if (ret == 0) {
+		ftwbuf->statbuf = sb;
+		ftwbuf->typeflag = mode_to_typeflag(sb->st_mode);
 	}
-
-	ftwbuf->statbuf = sb;
-	ftwbuf->typeflag = mode_to_typeflag(sb->st_mode);
-
-	return 0;
+	return ret;
 }
 
 /**
@@ -837,14 +834,7 @@ static void bftw_init_buffers(struct bftw_state *state, const struct dirent *de)
 	    || ftwbuf->typeflag == BFTW_UNKNOWN
 	    || (ftwbuf->typeflag == BFTW_LNK && follow)
 	    || (ftwbuf->typeflag == BFTW_DIR && (detect_cycles || xdev))) {
-		int ret = ftwbuf_stat(ftwbuf, &state->statbuf);
-		if (ret != 0 && follow && (errno == ENOENT || errno == ENOTDIR)) {
-			// Could be a broken symlink, retry without following
-			ftwbuf->at_flags = AT_SYMLINK_NOFOLLOW;
-			ret = ftwbuf_stat(ftwbuf, &state->statbuf);
-		}
-
-		if (ret != 0) {
+		if (ftwbuf_stat(ftwbuf, &state->statbuf) != 0) {
 			bftw_set_error(state, errno);
 			return;
 		}
diff --git a/eval.c b/eval.c
index 6883ce4..a3c55d5 100644
--- a/eval.c
+++ b/eval.c
@@ -76,7 +76,7 @@ static void eval_error(struct eval_state *state) {
 static const struct stat *fill_statbuf(struct eval_state *state) {
 	struct BFTW *ftwbuf = state->ftwbuf;
 	if (!ftwbuf->statbuf) {
-		if (fstatat(ftwbuf->at_fd, ftwbuf->at_path, &state->statbuf, ftwbuf->at_flags) == 0) {
+		if (xfstatat(ftwbuf->at_fd, ftwbuf->at_path, &state->statbuf, &ftwbuf->at_flags) == 0) {
 			ftwbuf->statbuf = &state->statbuf;
 		} else {
 			eval_error(state);
@@ -821,29 +821,19 @@ bool eval_type(const struct expr *expr, struct eval_state *state) {
 bool eval_xtype(const struct expr *expr, struct eval_state *state) {
 	struct BFTW *ftwbuf = state->ftwbuf;
 
-	int follow_flags = BFTW_LOGICAL;
-	if (ftwbuf->depth == 0) {
-		follow_flags |= BFTW_COMFOLLOW;
-	}
-	bool follow = state->cmdline->flags & follow_flags;
-
+	bool follow = !(ftwbuf->at_flags & AT_SYMLINK_NOFOLLOW);
 	bool is_link = ftwbuf->typeflag == BFTW_LNK;
 	if (follow == is_link) {
 		return eval_type(expr, state);
 	}
 
 	// -xtype does the opposite of everything else
-	int at_flags = follow ? AT_SYMLINK_NOFOLLOW : 0;
+	int at_flags = ftwbuf->at_flags ^ AT_SYMLINK_NOFOLLOW;
 
 	struct stat sb;
-	if (fstatat(ftwbuf->at_fd, ftwbuf->at_path, &sb, at_flags) != 0) {
-		if (!follow && (errno == ENOENT || errno == ENOTDIR)) {
-			// Broken symlink
-			return eval_type(expr, state);
-		} else {
-			eval_error(state);
-			return false;
-		}
+	if (xfstatat(ftwbuf->at_fd, ftwbuf->at_path, &sb, &at_flags) != 0) {
+		eval_error(state);
+		return false;
 	}
 
 	return mode_to_typeflag(sb.st_mode) & expr->idata;
diff --git a/parse.c b/parse.c
index 2ba0602..ef252fb 100644
--- a/parse.c
+++ b/parse.c
@@ -345,7 +345,7 @@ static int stat_arg(const struct parser_state *state, struct expr *expr, struct
 	bool follow = cmdline->flags & (BFTW_COMFOLLOW | BFTW_LOGICAL);
 	int flags = follow ? 0 : AT_SYMLINK_NOFOLLOW;
 
-	int ret = fstatat(AT_FDCWD, expr->sdata, sb, flags);
+	int ret = xfstatat(AT_FDCWD, expr->sdata, sb, &flags);
 	if (ret != 0) {
 		cfprintf(cmdline->cerr, "%{er}error: '%s': %s%{rs}\n", expr->sdata, strerror(errno));
 	}
diff --git a/tests.sh b/tests.sh
index 88e93b1..1517bda 100755
--- a/tests.sh
+++ b/tests.sh
@@ -169,6 +169,7 @@ posix_tests=(
     test_name_trailing_slash
     test_path
     test_newer
+    test_newer_link
     test_links
     test_links_plus
     test_links_minus
@@ -215,6 +216,10 @@ bsd_tests=(
     test_X
     test_follow
     test_samefile
+    test_samefile_symlink
+    test_H_samefile_symlink
+    test_samefile_broken
+    test_H_samefile_broken
     test_name_slash
     test_name_slashes
     test_iname
@@ -285,6 +290,10 @@ gnu_tests=(
     test_P_slash
     test_follow
     test_samefile
+    test_samefile_symlink
+    test_H_samefile_symlink
+    test_samefile_broken
+    test_H_samefile_broken
     test_xtype_l
     test_xtype_f
     test_L_xtype_l
@@ -624,6 +633,10 @@ function test_newer() {
     bfs_diff times -newer times/a
 }
 
+function test_newer_link() {
+    bfs_diff times -newer times/l
+}
+
 function test_anewer() {
     bfs_diff times -anewer times/a
 }
@@ -684,6 +697,22 @@ function test_samefile() {
     bfs_diff links -samefile links/a
 }
 
+function test_samefile_symlink() {
+    bfs_diff links -samefile links/b
+}
+
+function test_H_samefile_symlink() {
+    bfs_diff -H links -samefile links/b
+}
+
+function test_samefile_broken() {
+    bfs_diff links -samefile links/d/e/i
+}
+
+function test_H_samefile_broken() {
+    bfs_diff -H links -samefile links/d/e/i
+}
+
 function test_xtype_l() {
     bfs_diff links -xtype l
 }
diff --git a/tests/test_H_samefile_broken.out b/tests/test_H_samefile_broken.out
new file mode 100644
index 0000000..e5e13fc
--- /dev/null
+++ b/tests/test_H_samefile_broken.out
@@ -0,0 +1 @@
+links/d/e/i
diff --git a/tests/test_H_samefile_symlink.out b/tests/test_H_samefile_symlink.out
new file mode 100644
index 0000000..892c879
--- /dev/null
+++ b/tests/test_H_samefile_symlink.out
@@ -0,0 +1,2 @@
+links/a
+links/c
diff --git a/tests/test_newer_link.out b/tests/test_newer_link.out
new file mode 100644
index 0000000..d2dcdd1
--- /dev/null
+++ b/tests/test_newer_link.out
@@ -0,0 +1 @@
+times
diff --git a/tests/test_samefile_broken.out b/tests/test_samefile_broken.out
new file mode 100644
index 0000000..e5e13fc
--- /dev/null
+++ b/tests/test_samefile_broken.out
@@ -0,0 +1 @@
+links/d/e/i
diff --git a/tests/test_samefile_symlink.out b/tests/test_samefile_symlink.out
new file mode 100644
index 0000000..76696d9
--- /dev/null
+++ b/tests/test_samefile_symlink.out
@@ -0,0 +1 @@
+links/b
diff --git a/util.c b/util.c
index 2e6b477..d825554 100644
--- a/util.c
+++ b/util.c
@@ -226,6 +226,17 @@ const char *xbasename(const char *path) {
 	return i;
 }
 
+int xfstatat(int fd, const char *path, struct stat *buf, int *flags) {
+	int ret = fstatat(fd, path, buf, *flags);
+
+	if (ret != 0 && !(*flags & AT_SYMLINK_NOFOLLOW) && (errno == ENOENT || errno == ENOTDIR)) {
+		*flags |= AT_SYMLINK_NOFOLLOW;
+		ret = fstatat(fd, path, buf, *flags);
+	}
+
+	return ret;
+}
+
 enum bftw_typeflag mode_to_typeflag(mode_t mode) {
 	switch (mode & S_IFMT) {
 #ifdef S_IFBLK
diff --git a/util.h b/util.h
index b2025e6..6798fb9 100644
--- a/util.h
+++ b/util.h
@@ -143,6 +143,21 @@ void format_mode(mode_t mode, char str[11]);
 const char *xbasename(const char *path);
 
 /**
+ * stat() a file, falling back on the link itself for broken symbolic links.
+ *
+ * @param fd
+ *         The base directory descriptor.
+ * @param path
+ *         The path to the file, relative to fd.
+ * @param buf
+ *         The stat buffer to fill.
+ * @param flags
+ *         AT_* flags for this call.  Will be updated if a fallback happens.
+ * @return 0 on success, -1 on failure.
+ */
+int xfstatat(int fd, const char *path, struct stat *buf, int *flags);
+
+/**
  * Convert a stat() st_mode to a bftw() typeflag.
  */
 enum bftw_typeflag mode_to_typeflag(mode_t mode);