[coreutils] Re: [PATCH] du: don't print junk when diagnosing out-of-range time stamps

[Jim Meyering]

Paul Eggert wrote:
> While looking into the inttostr issues circulated recently on
> bug-gnulib, I was experimenting with an inttostr variant that I hope
> will be nicer.  In the process, I discovered a bug in du, where du
> uses uninitialized storage.  Here's a patch.

Good catch!

> The tabs in the new file tests/du/bigtime should not be replaced by
> spaces: hope that's OK.

It's ok, but you can avoid them and save two lines:

printf "0\t$bignum\tfuture\n" > exp || framework_failure_

The framework_failure_ isn't absolutely essential, but good
practice, just in case the printf prints nothing and fails
and the following du prints nothing and succeeds.
Without the framework_failure_ use, the test would
mistakenly pass.

> There are a lot of file systems that do reeeeally flaky things
> with time stamps.  For example, NetApp silently replaces out-of-range
> time stamps with the nearest representable time stamp: it uses
> 32-bit unsigned time stamps so the year range is from 1970 to 2106.
> tests/du/bigtime attempts to detect any such shortcomings, and to
> apply the test to du only on hosts where file systems work correctly
> on big time stamps, but localtime does not.
>
> Conversely, it's conceivable that the file system and localtime
> both support big timestamps, in which case du should not report
> an error when converting.  The test also tries to detect hosts where
> that is true, and to skip the test if so.  I don't know of any such
> hosts, so I haven't tested that part of the script.
>
> I haven't pushed this patch, because I don't know whether it's
> safe to get back into the water after the recent coreutils release.

Thanks for asking.
You've guessed right that I want to make a new release ASAP
with an adjusted stat command.  However, this is obviously
a bug fix, so it belongs, too.

> Subject: [PATCH] du: don't print junk when diagnosing out-of-range time stamps
>
> * src/du.c (show_date): Fix call to fputs with a buffer that
> contains some uninitialized data.  While we're at it, print
> the nanosecond part of the time stamp, too, if the file system
> supports that.
> * tests/Makefile.am (TESTS): Add du/big-timestamp.
> * tests/du/bigtime: New file, which checks for the bug.
> ---
>  src/du.c          |   27 ++++++++++++++++++++++++---
>  tests/Makefile.am |    1 +
>  tests/du/bigtime  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 3 deletions(-)
>  create mode 100755 tests/du/bigtime
>
> diff --git a/src/du.c b/src/du.c
> index 3d92579..33f2656 100644
> --- a/src/du.c
> +++ b/src/du.c
> @@ -350,9 +350,30 @@ show_date (const char *format, struct timespec when)
>    struct tm *tm = localtime (&when.tv_sec);
>    if (! tm)
>      {
> -      char buf[INT_BUFSIZE_BOUND (intmax_t)];
> -      error (0, 0, _("time %s is out of range"), timetostr (when.tv_sec, 
> buf));
> -      fputs (buf, stdout);
> +      char buf[INT_BUFSIZE_BOUND (intmax_t) + 10];
> +      char *when_str = timetostr (when.tv_sec, buf);
> +      int nsec = when.tv_nsec;
> +
> +      /* Display the fraction, omitting trailing zeros.  */

Why is it useful to display the fractional part here?

> +      if (nsec)
> +        {
> +          char *p = when_str + strlen (when_str);
> +          int power_of_ten = 100000000;
> +          *p++ = '.';
> +
> +          do
> +            {
> +              *p++ = '0' + nsec / power_of_ten;
> +              nsec %= power_of_ten;
> +              power_of_ten /= 10;
> +            }
> +          while (nsec);
> +
> +          *p = '\0';
> +        }
> +
> +      error (0, 0, _("time %s is out of range"), when_str);
> +      fputs (when_str, stdout);
>        return;
>      }
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 41e0cbc..84db367 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -346,6 +346,7 @@ TESTS =                                           \
>    du/2g                                              \
>    du/8gb                                     \
>    du/basic                                   \
> +  du/bigtime                                 \
>    du/deref                                   \
>    du/deref-args                                      \
>    du/exclude                                 \
> diff --git a/tests/du/bigtime b/tests/du/bigtime
> new file mode 100755
> index 0000000..1a78d52
> --- /dev/null
> +++ b/tests/du/bigtime
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +# Exercise du on a file with a big time stamp.
> +
> +# Copyright (C) 2010 Free Software Foundation, Inc.
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +. "${srcdir=.}/init.sh"; path_prepend_ ../src
> +
> +export LC_ALL=C
> +export TZ=UTC0
> +
> +# 2**63 - 1
> +bignum=9223372036854775807
> +
> +touch -d @$bignum future 2>/dev/null &&
> +future_time=$(ls -l future) &&
> +case "$future_time" in
> +*" $bignum "*)
> +  : ;;
> +*' Dec  4  300627798676 '*)
> +  skip_ "file system and localtime both handle big timestamps" ;;
> +*)
> +  skip_ "file system or localtime mishandles big time stamps: $future_time" 
> ;;
> +esac || skip_ "file system cannot represent big time stamps"
> +
> +cat >exp <<EOF
> +0    $bignum future
> +EOF
> +cat >err_ok <<EOF
> +du: time $bignum is out of range
> +EOF
> +du --time future >out 2>err || fail=1
> +compare out exp || fail=1
> +compare err err_ok || fail=1
> +
> +Exit $fail