From e00e56a7df616659c90d107c3d31d362b5dff103 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 16 Feb 2023 13:27:14 -0500 Subject: [PATCH 1/2] dir-iterator: drop unused `DIR_ITERATOR_FOLLOW_SYMLINKS` The `FOLLOW_SYMLINKS` flag was added to the dir-iterator API in fa1da7d2ee (dir-iterator: add flags parameter to dir_iterator_begin, 2019-07-10) in order to follow symbolic links while traversing through a directory. `FOLLOW_SYMLINKS` gained its first caller in ff7ccc8c9a (clone: use dir-iterator to avoid explicit dir traversal, 2019-07-10), but it was subsequently removed in 6f054f9fb3 (builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28). Since then, we've held on to the code for `DIR_ITERATOR_FOLLOW_SYMLINKS` in the name of making minimally invasive changes during a security embargo. In fact, we even changed the dir-iterator API in bffc762f87 (dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS, 2023-01-24) without having any non-test callers of that flag. Now that we're past those security embargo(s), let's finalize our cleanup of the `DIR_ITERATOR_FOLLOW_SYMLINKS` code and remove its implementation since there are no remaining callers. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- dir-iterator.c | 12 +++--------- dir-iterator.h | 20 +------------------- t/helper/test-dir-iterator.c | 6 ++---- t/t0066-dir-iterator.sh | 34 ---------------------------------- 4 files changed, 6 insertions(+), 66 deletions(-) diff --git a/dir-iterator.c b/dir-iterator.c index 3764dd81a1..cedd304759 100644 --- a/dir-iterator.c +++ b/dir-iterator.c @@ -112,10 +112,7 @@ static int prepare_next_entry_data(struct dir_iterator_int *iter, iter->base.basename = iter->base.path.buf + iter->levels[iter->levels_nr - 1].prefix_len; - if (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS) - err = stat(iter->base.path.buf, &iter->base.st); - else - err = lstat(iter->base.path.buf, &iter->base.st); + err = lstat(iter->base.path.buf, &iter->base.st); saved_errno = errno; if (err && errno != ENOENT) @@ -213,13 +210,10 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags) iter->flags = flags; /* - * Note: stat/lstat already checks for NULL or empty strings and + * Note: lstat already checks for NULL or empty strings and * nonexistent paths. */ - if (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS) - err = stat(iter->base.path.buf, &iter->base.st); - else - err = lstat(iter->base.path.buf, &iter->base.st); + err = lstat(iter->base.path.buf, &iter->base.st); if (err < 0) { saved_errno = errno; diff --git a/dir-iterator.h b/dir-iterator.h index e3b6ff2800..479e1ec784 100644 --- a/dir-iterator.h +++ b/dir-iterator.h @@ -54,24 +54,8 @@ * and ITER_ERROR is returned immediately. In both cases, a meaningful * warning is emitted. Note: ENOENT errors are always ignored so that * the API users may remove files during iteration. - * - * - DIR_ITERATOR_FOLLOW_SYMLINKS: make dir-iterator follow symlinks. - * i.e., linked directories' contents will be iterated over and - * iter->base.st will contain information on the referred files, - * not the symlinks themselves, which is the default behavior. Broken - * symlinks are ignored. - * - * Note: setting DIR_ITERATOR_FOLLOW_SYMLINKS affects resolving the - * starting path as well (e.g., attempting to iterate starting at a - * symbolic link pointing to a directory without FOLLOW_SYMLINKS will - * result in an error). - * - * Warning: circular symlinks are also followed when - * DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with - * an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set. */ #define DIR_ITERATOR_PEDANTIC (1 << 0) -#define DIR_ITERATOR_FOLLOW_SYMLINKS (1 << 1) struct dir_iterator { /* The current path: */ @@ -88,9 +72,7 @@ struct dir_iterator { const char *basename; /* - * The result of calling lstat() on path; or stat(), if the - * DIR_ITERATOR_FOLLOW_SYMLINKS flag was set at - * dir_iterator's initialization. + * The result of calling lstat() on path. */ struct stat st; }; diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c index 659b6bfa81..6b297bd753 100644 --- a/t/helper/test-dir-iterator.c +++ b/t/helper/test-dir-iterator.c @@ -15,7 +15,7 @@ static const char *error_name(int error_number) /* * usage: - * tool-test dir-iterator [--follow-symlinks] [--pedantic] directory_path + * tool-test dir-iterator [--pedantic] directory_path */ int cmd__dir_iterator(int argc, const char **argv) { @@ -24,9 +24,7 @@ int cmd__dir_iterator(int argc, const char **argv) int iter_status; for (++argv, --argc; *argv && starts_with(*argv, "--"); ++argv, --argc) { - if (strcmp(*argv, "--follow-symlinks") == 0) - flags |= DIR_ITERATOR_FOLLOW_SYMLINKS; - else if (strcmp(*argv, "--pedantic") == 0) + if (strcmp(*argv, "--pedantic") == 0) flags |= DIR_ITERATOR_PEDANTIC; else die("invalid option '%s'", *argv); diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh index 04b811622b..4ed3136b00 100755 --- a/t/t0066-dir-iterator.sh +++ b/t/t0066-dir-iterator.sh @@ -131,44 +131,10 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default test_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output ' -test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag' ' - cat >expected-follow-sorted-output <<-EOF && - [d] (a) [a] ./dir4/a - [d] (a/f) [f] ./dir4/a/f - [d] (a/f/c) [c] ./dir4/a/f/c - [d] (b) [b] ./dir4/b - [d] (b/c) [c] ./dir4/b/c - [f] (a/d) [d] ./dir4/a/d - [f] (a/e) [e] ./dir4/a/e - EOF - - test-tool dir-iterator --follow-symlinks ./dir4 >out && - sort out >actual-follow-sorted-output && - - test_cmp expected-follow-sorted-output actual-follow-sorted-output -' - test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' ' test_must_fail test-tool dir-iterator ./dir6 >out && grep "ENOTDIR" out ' -test_expect_success SYMLINKS 'dir-iterator resolves top-level symlinks w/ follow flag' ' - cat >expected-follow-sorted-output <<-EOF && - [d] (a) [a] ./dir6/a - [d] (a/f) [f] ./dir6/a/f - [d] (a/f/c) [c] ./dir6/a/f/c - [d] (b) [b] ./dir6/b - [d] (b/c) [c] ./dir6/b/c - [f] (a/d) [d] ./dir6/a/d - [f] (a/e) [e] ./dir6/a/e - EOF - - test-tool dir-iterator --follow-symlinks ./dir6 >out && - sort out >actual-follow-sorted-output && - - test_cmp expected-follow-sorted-output actual-follow-sorted-output -' - test_done From 3b0ebb7a8db6e5a1a11dcb2c1f549e4d6037185c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Feb 2023 19:40:29 -0500 Subject: [PATCH 2/2] t0066: drop setup of "dir5" The symlink setup in t0066 makes several directories with links, dir4 through dir6. But ever since dir5 was introduced in fa1da7d2ee (dir-iterator: add flags parameter to dir_iterator_begin, 2019-07-10), it has never actually been used. It was left over from an earlier iteration of the patch which tried to handle recursive symlinks specially, as seen in: https://lore.kernel.org/git/20190502144829.4394-7-matheus.bernardino@usp.br/ It's not hurting any of the existing tests to be there, but the extra setup is confusing to anybody trying to read and understand the tests. Let's drop the extra directory, and we'll rename "dir6" to "dir5" so nobody wonders whether the gap in naming is important. Helped-by: Matheus Tavares Bernardino Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0066-dir-iterator.sh | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh index 4ed3136b00..7d0a0da8c0 100755 --- a/t/t0066-dir-iterator.sh +++ b/t/t0066-dir-iterator.sh @@ -106,13 +106,7 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' ' ln -s d dir4/a/e && ln -s ../b dir4/a/f && - mkdir -p dir5/a/b && - mkdir -p dir5/a/c && - ln -s ../c dir5/a/b/d && - ln -s ../ dir5/a/b/e && - ln -s ../../ dir5/a/b/f && - - ln -s dir4 dir6 + ln -s dir4 dir5 ' test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' ' @@ -132,7 +126,7 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default ' test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' ' - test_must_fail test-tool dir-iterator ./dir6 >out && + test_must_fail test-tool dir-iterator ./dir5 >out && grep "ENOTDIR" out '