FYI, now chcon, chgrp, chmod and chown diagnose directory cycles, too: The other fts clients, du and rm, did already.
>From d9dbbb9a455f6bfc4e09d9f5f6c6c633f1b03c52 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sat, 7 Nov 2009 08:09:12 +0100 Subject: [PATCH 1/2] chcon, chgrp, chmod and chown now diagnose a directory cycle * lib/xfts.c (cycle_warning_required): New function. * lib/xfts.h: Declare it. * src/chown-core.c (change_file_owner): Diagnose a cycle. * src/chmod.c (process_file): Likewise. * src/chcon.c (process_file): Likewise. * NEWS (Bug fixes): Mention this. --- NEWS | 5 +++-- lib/xfts.c | 19 +++++++++++++++++++ lib/xfts.h | 4 ++++ src/chcon.c | 8 ++++++++ src/chmod.c | 9 +++++++++ src/chown-core.c | 8 ++++++++ 6 files changed, 51 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 24b2afa..b13a5f2 100644 --- a/NEWS +++ b/NEWS @@ -8,8 +8,9 @@ GNU coreutils NEWS -*- outline -*- Even then, chcon may still be useful. [bug introduced in coreutils-8.0] - du now diagnoses an ostensible directory cycle and arranges to exit nonzero. - Before, it would silently ignore the offending directory and all "contents." + chcon, chgrp, chmod, chown and du now diagnose an ostensible directory cycle + and arrange to exit nonzero. Before, they would silently ignore the + offending directory and all "contents." env -u A=B now fails, rather than silently adding A to the environment. Likewise, printenv A=B silently ignores the invalid diff --git a/lib/xfts.c b/lib/xfts.c index 5994a5f..9c46f6a 100644 --- a/lib/xfts.c +++ b/lib/xfts.c @@ -61,3 +61,22 @@ xfts_open (char * const *argv, int options, return fts; } + +/* When fts_read returns FTS_DC to indicate a directory cycle, + it may or may not indicate a real problem. When a program like + chgrp performs a recursive traversal that requires traversing + symbolic links, it is *not* a problem. However, when invoked + with "-P -R", it deserves a warning. The fts_options member + records the options that control this aspect of fts's behavior, + so test that. */ +bool +cycle_warning_required (FTS const *fts, FTSENT const *ent) +{ +#define ISSET(Fts,Opt) ((Fts)->fts_options & (Opt)) + /* When dereferencing no symlinks, or when dereferencing only + those listed on the command line and we're not processing + a command-line argument, then a cycle is a serious problem. */ + return ((ISSET (fts, FTS_PHYSICAL) && !ISSET (fts, FTS_COMFOLLOW)) + || (ISSET (fts, FTS_PHYSICAL) && ISSET (fts, FTS_COMFOLLOW) + && ent->fts_level != FTS_ROOTLEVEL)); +} diff --git a/lib/xfts.h b/lib/xfts.h index 27ddb5d..fc3ba90 100644 --- a/lib/xfts.h +++ b/lib/xfts.h @@ -1,5 +1,9 @@ +#include <stdbool.h> #include "fts_.h" FTS * xfts_open (char * const *, int options, int (*) (const FTSENT **, const FTSENT **)); + +bool +cycle_warning_required (FTS const *fts, FTSENT const *ent); diff --git a/src/chcon.c b/src/chcon.c index 2badefb..5e58cac 100644 --- a/src/chcon.c +++ b/src/chcon.c @@ -267,6 +267,14 @@ process_file (FTS *fts, FTSENT *ent) ok = false; break; + case FTS_DC: /* directory that causes cycles */ + if (cycle_warning_required (fts, ent)) + { + emit_cycle_warning (file_full_name); + return false; + } + break; + default: break; } diff --git a/src/chmod.c b/src/chmod.c index da35003..1a0dafa 100644 --- a/src/chmod.c +++ b/src/chmod.c @@ -228,6 +228,15 @@ process_file (FTS *fts, FTSENT *ent) error (0, 0, _("cannot operate on dangling symlink %s"), quote (file_full_name)); ok = false; + break; + + case FTS_DC: /* directory that causes cycles */ + if (cycle_warning_required (fts, ent)) + { + emit_cycle_warning (file_full_name); + return false; + } + break; default: break; diff --git a/src/chown-core.c b/src/chown-core.c index e7dacf6..eaebe60 100644 --- a/src/chown-core.c +++ b/src/chown-core.c @@ -316,6 +316,14 @@ change_file_owner (FTS *fts, FTSENT *ent, ok = false; break; + case FTS_DC: /* directory that causes cycles */ + if (cycle_warning_required (fts, ent)) + { + emit_cycle_warning (file_full_name); + return false; + } + break; + default: break; } -- 1.6.5.2.303.g13162 >From 9a8d8f46a541d333f98dca26d053d1f5bd4424bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sat, 7 Nov 2009 08:17:28 +0100 Subject: [PATCH 2/2] maint: make du's cycle-detection code consistent * src/du.c (process_file): Revert the du.c-changing part of commit 8ba5d1a7. Use cycle_warning_required instead. --- src/du.c | 17 ++++++----------- 1 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/du.c b/src/du.c index c33bbb7..bee006d 100644 --- a/src/du.c +++ b/src/du.c @@ -65,10 +65,6 @@ extern bool fts_debug; /* Initial size of the hash table. */ #define INITIAL_TABLE_SIZE 103 -/* Select one of the three FTS_ options that control if/when - to follow a symlink. */ -static int symlink_deref_bits = FTS_PHYSICAL; - /* Hash structure for inode and device numbers. The separate entry structure makes it easier to rehash "in place". */ struct entry @@ -498,14 +494,9 @@ process_file (FTS *fts, FTSENT *ent) break; case FTS_DC: /* directory that causes cycles */ - /* When dereferencing no symlinks, or when dereferencing only - those listed on the command line and we're not processing - a command-line argument, then a cycle is a serious problem. */ - if (symlink_deref_bits == FTS_PHYSICAL - || (symlink_deref_bits == (FTS_COMFOLLOW | FTS_PHYSICAL) - && ent->fts_level != FTS_ROOTLEVEL)) + if (cycle_warning_required (fts, ent)) { - emit_cycle_warning (ent->fts_path); + emit_cycle_warning (file); return false; } ok = true; @@ -677,6 +668,10 @@ main (int argc, char **argv) /* Bit flags that control how fts works. */ int bit_flags = FTS_TIGHT_CYCLE_CHECK | FTS_DEFER_STAT; + /* Select one of the three FTS_ options that control if/when + to follow a symlink. */ + int symlink_deref_bits = FTS_PHYSICAL; + /* If true, display only a total for each argument. */ bool opt_summarize_only = false; -- 1.6.5.2.303.g13162
