[PATCH] du now diagnoses cycles, rather than ignoring them

[Jim Meyering]

[fall-out from the FTS vs. automount-changing-st_dev problem, ...]

While du ignoring cycles may not be an official "bug"
(in the sense that the POSIX du spec says nothing about cycles)
I think it deserves to be diagnosed and to provoke a nonzero exit status.

You can reproduce the problem by following these instructions on a
Fedora 12 system:

    http://bugzilla.redhat.com/501848#c45

Then, do this:

    $ ./du -s --exclude='/tmp/mnt/t*' --exclude '[os.fbvuxdiepljr]*' /tmp/mnt
    ./du: WARNING: Circular directory structure.
    This almost certainly means that you have a corrupted file system.
    NOTIFY YOUR SYSTEM MANAGER.
    The following directory is part of the cycle:
      `/tmp/mnt/home'

    12      /tmp/mnt
    [Exit 1]

[the exclusions are simply to make it run quickly,
 by not traversing into directories that aren't relevant]

With the following patch, it prints the above diagnostic and fails.
Before, it would silently ignore everything under /tmp/mnt/home and exit 0.
BTW, the diagnostic (including trailing NL) is copied verbatim from remove.c.
As I write this, I now realize I should factor that out.  Besides, the
other fts-using programs probably need a similar change.

Before this patch, it skipped /tmp/mnt/home because fts_read reported
FTS_DC (indicating a directory cycle) and process_file ignored that
problem.  Here you can see the two directories with identical dev/inode
pairs:

    $ stat --printf "%d %i %n\n" /tmp/mnt/home /tmp/mnt
    24 2 /tmp/mnt/home
    24 2 /tmp/mnt

That is due to a bug in the kernel.

The reproducer mentioned above is more invasive than usual,
so I'm going to refrain from adding a test.



From 772a8448c936d85f531011b68eaf91248679e651 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@...>
Date: Wed, 4 Nov 2009 22:01:52 +0100
Subject: [PATCH] du now diagnoses cycles, rather than ignoring them

* src/du.c (symlink_deref_bits): New global, decl moved from ...
(main): ...here.
(process_file): When fts detects a directory cycle that can't
be due to symlinks, report it and arrange to exit nonzero.
* NEWS (Bug fixes): Mention it.
---
 NEWS     |    3 +++
 src/du.c |   28 +++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 03ed83f..ffecd86 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +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."
+
   env -u A=B now fails, rather than silently adding A to the
   environment.  Likewise, printenv A=B silently ignores the invalid
   name.  [the bugs date back to the initial implementation]
diff --git a/src/du.c b/src/du.c
index 9831a17..c684d1a 100644
--- a/src/du.c
+++ b/src/du.c
@@ -65,9 +65,12 @@ 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
 {
   ino_t st_ino;
@@ -495,6 +498,25 @@ process_file (FTS *fts, FTSENT *ent)
       ok = false;
       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))
+        {
+          error (0, 0, _("\
+WARNING: Circular directory structure.\n\
+This almost certainly means that you have a corrupted file system.\n\
+NOTIFY YOUR SYSTEM MANAGER.\n\
+The following directory is part of the cycle:\n  %s\n"),
+                 quote (ent->fts_path));
+          return false;
+        }
+      ok = true;
+      break;
+
     default:
       ok = true;
       break;
@@ -661,10 +683,6 @@ 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.292.g1cda2

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> +      if (symlink_deref_bits == FTS_PHYSICAL
> +          || (symlink_deref_bits == (FTS_COMFOLLOW | FTS_PHYSICAL)
> +              && ent->fts_level != FTS_ROOTLEVEL))

Can't this be simplified?

     if (symlink_deref_bits == FTS_PHYSICAL
         || (symlink_deref_bits == FTS_COMFOLLOW
             && ent->fts_level != FTS_ROOTLEVEL))

--
Eric Blake

[Jim Meyering]

Eric Blake wrote:

> Jim Meyering <jim <at> meyering.net> writes:
>
>> +      if (symlink_deref_bits == FTS_PHYSICAL
>> +          || (symlink_deref_bits == (FTS_COMFOLLOW | FTS_PHYSICAL)
>> +              && ent->fts_level != FTS_ROOTLEVEL))
>
> Can't this be simplified?
>
>      if (symlink_deref_bits == FTS_PHYSICAL
>          || (symlink_deref_bits == FTS_COMFOLLOW
>              && ent->fts_level != FTS_ROOTLEVEL))

I don't see how, since with -D, it does this:

        case 'H':  /* NOTE: before 2008-12, -H was equivalent to --si.  */
        case 'D':
          symlink_deref_bits = FTS_COMFOLLOW | FTS_PHYSICAL;
          break;