Re: bug in du --time=atime

[William Brendling]

On 10/4/05, Paul Eggert <address@hidden> wrote:
> >> Or (this may be better), how about an option that restores atime
> >> after the directory is read?
> >
> > The new option --preserve-atime does this.
>
> On second thought this might not be such a good idea, since it
> clobbers any atime values set by other processes.  Perhaps we
> should just leave this part out.

My revised patch below omits this option. Instead it provides the
option --time-exclude-dirs which ignores directories when determining
the last time.

This is most useful for --time=atime when performing a periodic scan
for unused files, as the du pass will change the atime for the
directories. However the option works for any of the times (mtime,
ctime or atime).

> Also -- and this may be another subject -- it seems to me that the
> time code has a problem around here:
>
>   /* If the file is being excluded or if it has already been counted
>      via a hard link, then don't let it contribute to the sums.  */
>
> With sizes it makes sense to skip hard links because you don't want to
> count the same size twice.  But with time stamps you're not adding
> things so does it really make sense to do that?  Wouldn't it be better
> to count time stamps to hard links?

This is corrected. On second encounter with a multiply-linked file the
measured size is set to zero but the time is accumulated as for any
other file.


Index: coreutils/ChangeLog
===================================================================
RCS file: /cvsroot/coreutils/coreutils/ChangeLog,v
retrieving revision 1.1489
diff -u -r1.1489 ChangeLog
--- coreutils/ChangeLog 5 Oct 2005 18:53:59 -0000       1.1489
+++ coreutils/ChangeLog 10 Oct 2005 19:12:10 -0000
@@ -1,3 +1,8 @@
+2005-10-10  William Brendling  <address@hidden>
+
+       * src/du.c (process_file): Collect time of file even if multiply linked.
+       * src/du.c: Added --time-exclude-dirs option.
+
 2005-10-05  Jim Meyering  <address@hidden>

        * Version 5.91-cvs.
Index: coreutils/src/du.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/du.c,v
retrieving revision 1.222
diff -u -r1.222 du.c
--- coreutils/src/du.c  16 Sep 2005 08:08:32 -0000      1.222
+++ coreutils/src/du.c  10 Oct 2005 19:12:13 -0000
@@ -98,14 +98,6 @@
   a->tmax.tv_nsec = -1;
 }

-/* Set directory data.  */
-static inline void
-duinfo_set (struct duinfo *a, uintmax_t size, struct timespec tmax)
-{
-  a->size = size;
-  a->tmax = tmax;
-}
-
 /* Accumulate directory data.  */
 static inline void
 duinfo_add (struct duinfo *a, struct duinfo const *b)
@@ -123,6 +115,9 @@

   /* Total for subdirectories.  */
   struct duinfo subdir;
+
+  /* Pre-traverse atime.       */
+  struct timespec at_prev;
 };

 /* Name under which this program was invoked.  */
@@ -175,6 +170,9 @@
 /* Format used to display date / time. Controlled by --time-style */
 static char const *time_format = NULL;

+/* Exclude directories from latest time.       */
+static bool time_exclude_dirs;
+
 /* The units to use when printing sizes.  */
 static uintmax_t output_block_size;

@@ -198,7 +196,8 @@
   HUMAN_SI_OPTION,
   MAX_DEPTH_OPTION,
   TIME_OPTION,
-  TIME_STYLE_OPTION
+  TIME_STYLE_OPTION,
+  TIME_EXCLUDE_DIRS_OPTION
 };

 static struct option const long_options[] =
@@ -226,6 +225,7 @@
   {"total", no_argument, NULL, 'c'},
   {"time", optional_argument, NULL, TIME_OPTION},
   {"time-style", required_argument, NULL, TIME_STYLE_OPTION},
+  {"time-exclude-dirs", no_argument, NULL, TIME_EXCLUDE_DIRS_OPTION},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -325,6 +325,7 @@
       --time-style=STYLE show times using style STYLE:\n\
                           full-iso, long-iso, iso, +FORMAT\n\
                           FORMAT is interpreted like `date'\n\
+      --time-exclude-dirs exclude directory times when evaluating last time\n\
 "), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
@@ -503,87 +504,111 @@
       break;
     }

-  /* If this is the first (pre-order) encounter with a directory,
-     or if it's the second encounter for a skipped directory, then
+  /* If this is the second encounter for a skipped directory, then
      return right away.  */
-  if (ent->fts_info == FTS_D || skip)
+  if (skip)
+    return ok;
+
+  /* If we don't need to obtain the atime of a directory in pre-order,
+     return before allocating storage. */
+  if ( (ent->fts_info == FTS_D) && (time_type != time_atime) )
     return ok;

+  /* Allocate storage for additional levels if required.       */
+
+  level = ent->fts_level;
+  if (n_alloc == 0)
+    {
+      n_alloc = level + 10;
+      dulvl = xcalloc (n_alloc, sizeof *dulvl);
+    }
+  else if (n_alloc <= level)
+    {
+      dulvl = xnrealloc (dulvl, level, 2 * sizeof *dulvl);
+      n_alloc = level * 2;
+    }
+
+  /* If this is the first (pre-order) encounter with a directory,
+     then collect atime and return.  */
+  if (ent->fts_info == FTS_D)
+    {
+      struct stat stdir;
+      stat (ent->fts_accpath, &stdir);
+      dulvl[level].at_prev = get_stat_atime (&stdir);
+      return ok;
+    }
+
+  /* Collect the required information on the file. */
+  dui.size = (apparent_size ? sb->st_size : ST_NBLOCKS (*sb) * ST_NBLOCKSIZE);
+
+  switch (time_type)
+    {
+    case time_mtime:
+      dui.tmax = get_stat_mtime (sb);
+      break;
+
+    case time_atime:
+      dui.tmax = ( ent->fts_info == FTS_DP ?
+                   dulvl[level].at_prev : get_stat_atime (sb) );
+      break;
+
+    default: /* time_ctime */
+      dui.tmax = get_stat_ctime (sb);
+    }
+
   /* If the file is being excluded or if it has already been counted
-     via a hard link, then don't let it contribute to the sums.  */
-  if (skip
-      || (!opt_count_all
+     via a hard link, then don't let it contribute to the size.  */
+  if (  ! opt_count_all
          && ! S_ISDIR (sb->st_mode)
          && 1 < sb->st_nlink
-         && ! hash_ins (sb->st_ino, sb->st_dev)))
+         && ! hash_ins (sb->st_ino, sb->st_dev))
     {
-      /* Note that we must not simply return here.
-        We still have to update prev_level and maybe propagate
-        some sums up the hierarchy.  */
-      duinfo_init (&dui);
+      dui.size = 0;
       print = false;
     }
-  else
+
+  /* Optionally ignore directory times.   */
+  if ( time_exclude_dirs && S_ISDIR (sb->st_mode) )
     {
-      duinfo_set (&dui,
-                 (apparent_size
-                  ? sb->st_size
-                  : ST_NBLOCKS (*sb) * ST_NBLOCKSIZE),
-                 (time_type == time_mtime ? get_stat_mtime (sb)
-                  : time_type == time_atime ? get_stat_atime (sb)
-                  : get_stat_ctime (sb)));
+      dui.tmax.tv_sec = TYPE_MINIMUM (time_t);
+      dui.tmax.tv_nsec = -1;
     }

-  level = ent->fts_level;
   dui_to_print = dui;

-  if (n_alloc == 0)
+  if (level == prev_level)
     {
-      n_alloc = level + 10;
-      dulvl = xcalloc (n_alloc, sizeof *dulvl);
+      /* This is usually the most common case.  Do nothing.  */
     }
-  else
+  else if (level > prev_level)
     {
-      if (level == prev_level)
-       {
-         /* This is usually the most common case.  Do nothing.  */
-       }
-      else if (level > prev_level)
-       {
-         /* Descending the hierarchy.
-            Clear the accumulators for *all* levels between prev_level
-            and the current one.  The depth may change dramatically,
-            e.g., from 1 to 10.  */
-         size_t i;
-
-         if (n_alloc <= level)
-           {
-             dulvl = xnrealloc (dulvl, level, 2 * sizeof *dulvl);
-             n_alloc = level * 2;
-           }
+      /* Descending the hierarchy.
+          Clear the accumulators for *all* levels between prev_level
+          and the current one.  The depth may change dramatically,
+          e.g., from 1 to 10.  */
+      size_t i;

-         for (i = prev_level + 1; i <= level; i++)
-           {
-             duinfo_init (&dulvl[i].ent);
-             duinfo_init (&dulvl[i].subdir);
-           }
-       }
-      else /* level < prev_level */
+      for (i = prev_level + 1; i <= level; i++)
        {
-         /* Ascending the hierarchy.
-            Process a directory only after all entries in that
-            directory have been processed.  When the depth decreases,
-            propagate sums from the children (prev_level) to the parent.
-            Here, the current level is always one smaller than the
-            previous one.  */
-         assert (level == prev_level - 1);
-         duinfo_add (&dui_to_print, &dulvl[prev_level].ent);
-         if (!opt_separate_dirs)
-           duinfo_add (&dui_to_print, &dulvl[prev_level].subdir);
-         duinfo_add (&dulvl[level].subdir, &dulvl[prev_level].ent);
-         duinfo_add (&dulvl[level].subdir, &dulvl[prev_level].subdir);
+         duinfo_init (&dulvl[i].ent);
+         duinfo_init (&dulvl[i].subdir);
        }
     }
+  else /* level < prev_level */
+    {
+      /* Ascending the hierarchy.
+          Process a directory only after all entries in that
+          directory have been processed.  When the depth decreases,
+          propagate sums from the children (prev_level) to the parent.
+          Here, the current level is always one smaller than the
+          previous one.  */
+       assert (level == prev_level - 1);
+       duinfo_add (&dui_to_print, &dulvl[prev_level].ent);
+       if (!opt_separate_dirs)
+         duinfo_add (&dui_to_print, &dulvl[prev_level].subdir);
+       duinfo_add (&dulvl[level].subdir, &dulvl[prev_level].ent);
+       duinfo_add (&dulvl[level].subdir, &dulvl[prev_level].subdir);
+    }

   prev_level = level;

@@ -827,6 +852,10 @@

        case TIME_STYLE_OPTION:
          time_style = optarg;
+         break;
+
+       case TIME_EXCLUDE_DIRS_OPTION:
+         time_exclude_dirs = true;
          break;

        case_GETOPT_HELP_CHAR;
Index: coreutils/doc/ChangeLog
===================================================================
RCS file: /cvsroot/coreutils/coreutils/doc/ChangeLog,v
retrieving revision 1.258
diff -u -r1.258 ChangeLog
--- coreutils/doc/ChangeLog     25 Sep 2005 06:11:45 -0000      1.258
+++ coreutils/doc/ChangeLog     10 Oct 2005 19:12:13 -0000
@@ -1,3 +1,7 @@
+2005-10-10  William Brendling  <address@hidden>
+
+       * coreutils.texi (du invocation): Added --time-exclude-dirs option.
+
 2005-09-24  Paul Eggert  <address@hidden>

        * coreutils.texi (touch invocation):
Index: coreutils/doc/coreutils.texi
===================================================================
RCS file: /cvsroot/coreutils/coreutils/doc/coreutils.texi,v
retrieving revision 1.285
diff -u -r1.285 coreutils.texi
--- coreutils/doc/coreutils.texi        25 Sep 2005 06:08:17 -0000      1.285
+++ coreutils/doc/coreutils.texi        10 Oct 2005 19:12:28 -0000
@@ -9073,6 +9073,14 @@
 Show the most recent access time (the @samp{atime} in the inode) of
 any file in the directory, instead of the modification time.

address@hidden --time-exclude-dirs
address@hidden --time-exclude-dirs
address@hidden directory modified times, excluding in @command{du}
+Exclude the times of directories from the most recent file time shown
+by @option{--time} option. This is most useful for the @option{--time=atime}
+option, since running  @command{du} changes the @samp{atime} of all directories
+traversed.
+
 @item address@hidden
 @opindex --time-style
 @cindex time style