From acd7f7ed437793e7c67ecd869cfac32a87c1ec52 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Sat, 12 Aug 2017 18:12:13 -0400 Subject: 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. --- bftw.c | 22 ++++++---------------- eval.c | 22 ++++++---------------- parse.c | 2 +- tests.sh | 29 +++++++++++++++++++++++++++++ tests/test_H_samefile_broken.out | 1 + tests/test_H_samefile_symlink.out | 2 ++ tests/test_newer_link.out | 1 + tests/test_samefile_broken.out | 1 + tests/test_samefile_symlink.out | 1 + util.c | 11 +++++++++++ util.h | 15 +++++++++++++++ 11 files changed, 74 insertions(+), 33 deletions(-) create mode 100644 tests/test_H_samefile_broken.out create mode 100644 tests/test_H_samefile_symlink.out create mode 100644 tests/test_newer_link.out create mode 100644 tests/test_samefile_broken.out create mode 100644 tests/test_samefile_symlink.out 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 @@ -142,6 +142,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. */ -- cgit v1.2.3