Re: [coreutils] stat: reverting recent %X %Y %Z change

[Jim Meyering]

Jim Meyering wrote:

> Pádraig Brady wrote:
>> On 22/10/10 18:43, Jim Meyering wrote:
>>> Part of reverting this change:
>>>
>>>     stat now outputs the full sub-second resolution for the atime,
>>>     mtime, and ctime values since the Epoch, when using the %X, %Y, and
>>>     %Z directives of the --format option.  This matches the fact that
>>>     %x, %y, and %z were already doing so for the human-readable variant.
>>>
>>> means considering whether to do the same sort of thing with the
>>> newly-added %W (birth time/crtime).  Note that %W currently expands to "-"
>>> on systems with no support (e.g., linux).
>>>
>>> I don't particularly like the idea of making stat do this:
>>>
>>>     $ src/stat -c %W.%:W .
>>>     -.-
>>>
>>> Rather, I have a slight preference to make it do this:
>>>
>>>     $ src/stat -c %W.%:W .
>>>     0.000000000
>>
>> I prefer that as it would mean less special
>> cases in code that uses the output.
>
> Exactly.
>
>>> In any case, I think %w should still expand to "-".
>>>
>>> The alternative is to leave %W separate, with no %:W variant.
>>> I.e., %W would continue to print floating point seconds.
>>> In that case, it would be inconsistent with %X, %Y and %Z.
>>
>> I don't like that.
>
> Thanks for the feedback!

Here's a mostly-ready proposed change.

I've just realized it doesn't mention the nanosecond field width issue:
the default is 9, and it's always zero-padded, but only to a width of 9,
which is surprising if you ever specify a larger field width:

    $ src/stat -c %12:W .
       000000000

I dislike such surprises...

I should probably add a few tests, too.


>From d9d07d63f18e3482432fe458e3f7dd421bf51ef0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 21 Oct 2010 18:41:24 +0200
Subject: [PATCH] stat: revert change to %X, %Y, %Z; use %:X to print fractional 
seconds

This reverts part of the recent commit 9069af45,
"stat: print timestamps to full resolution".  We prefer to retain
portability of %X, %Y and %Z uses, while still providing access to
full-resolution time stamps.
* src/stat.c (print_it): Accept a new %...:[XYZ] format directive,
e.g., %:X, to print the nanoseconds portion of the corresponding time.
For example, %3.3:Y prints the zero-padded, truncated, milliseconds
part of the time of last modification.
(print_it): Update print_func signature to match.
(neg_to_zero): New helper function.
(epoch_time): Remove function; replace with...
(epoch_sec): New function; use timetostr.
(epoch_ns): New function.
(print_statfs): Change type of "m" to unsigned int,
now that it must accommodate values larger than 255.
(print_stat): Likewise.
Map :X to a code of 'X' + 256.  Likewise for Y, Z and W.
(usage): Update.
* tests/touch/60-seconds: Use %Y.%:Y in place of %Y.
* NEWS (Changes in behavior): Mention this.
Thanks to Andreas Schwab for raising the issue.
---
 NEWS                   |   15 ++++++
 doc/coreutils.texi     |    6 ++-
 src/stat.c             |  112 +++++++++++++++++++++++++++++++----------------
 tests/touch/60-seconds |    2 +-
 4 files changed, 95 insertions(+), 40 deletions(-)

diff --git a/NEWS b/NEWS
index f28c243..09b7225 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,21 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 * Noteworthy changes in release ?.? (????-??-??) [?]

+** Changes in behavior
+
+  stat's %X, %Y, and %Z directives once again print only the integer
+  part of seconds since the epoch.  This reverts a change, introduced
+  in coreutils-8.6, that was deemed unnecessarily disruptive.  To obtain
+  the nanoseconds portion corresponding to %X, you may now use %:X.
+  I.e., to print the floating point number of seconds using maximum
+  precision, use this format string: %X.%:X.  Likewise for %Y, %Z and %W.
+
+  stat's new %W format directive would print floating point seconds.
+  However, with the above change to %X, %Y and %Z, we've made %W work
+  the same way:  %W now expands to seconds since the epoch (or 0 when
+  not supported), and %:W expands to the nanoseconds portion, or to
+  0 if not supported.
+

 * Noteworthy changes in release 8.6 (2010-10-15) [stable]

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 4d17ed1..94e8ebf 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10698,13 +10698,17 @@ stat invocation
 @item %u - User ID of owner
 @item %U - User name of owner
 @item %w - Time of file birth, or @samp{-} if unknown
-@item %W - Time of file birth as seconds since Epoch, or @samp{-}
+@item %W - Time of file birth as seconds since Epoch, or @samp{0}
+@item %:W - Time of file birth: 0-padded nanoseconds remainder, or @samp{0}
 @item %x - Time of last access
 @item %X - Time of last access as seconds since Epoch
+@item %:X - Time of last access: 0-padded nanoseconds remainder
 @item %y - Time of last modification
 @item %Y - Time of last modification as seconds since Epoch
+@item %:Y - Time of last modification: 0-padded nanoseconds remainder
 @item %z - Time of last change
 @item %Z - Time of last change as seconds since Epoch
+@item %:Z - Time of last change: 0-padded nanoseconds remainder
 @end itemize

 The mount point printed by @samp{%m} is similar to that output
diff --git a/src/stat.c b/src/stat.c
index fabbc17..9a6bb8f 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -462,23 +462,22 @@ human_time (struct timespec t)
   return str;
 }

+/* Return a string representation (in static storage)
+   of the number of seconds in T since the epoch.  */
 static char * ATTRIBUTE_WARN_UNUSED_RESULT
-epoch_time (struct timespec t)
+epoch_sec (struct timespec t)
 {
-  static char str[INT_STRLEN_BOUND (time_t) + sizeof ".NNNNNNNNN"];
-  /* Note that time_t can technically be a floating point value, such
-     that casting to [u]intmax_t could lose a fractional value or
-     suffer from overflow.  However, most porting targets have an
-     integral time_t; also, we know of no file systems that store
-     valid time values outside the bounds of intmax_t even if that
-     value were represented as a floating point.  Besides, the cost of
-     converting to struct tm just to use nstrftime (str, len, "%s.%N",
-     tm, 0, t.tv_nsec) is pointless, since nstrftime would have to
-     convert back to seconds as time_t.  */
-  if (TYPE_SIGNED (time_t))
-    sprintf (str, "%" PRIdMAX ".%09ld", (intmax_t) t.tv_sec, t.tv_nsec);
-  else
-    sprintf (str, "%" PRIuMAX ".%09ld", (uintmax_t) t.tv_sec, t.tv_nsec);
+  static char str[INT_BUFSIZE_BOUND (time_t)];
+  return timetostr (t.tv_sec, str);
+}
+
+/* Return a string representation (in static storage)
+   of nanoseconds part of T.  */
+static char * ATTRIBUTE_WARN_UNUSED_RESULT
+epoch_ns (struct timespec t)
+{
+  static char str[10];
+  sprintf (str, "%09ld", t.tv_nsec);
   return str;
 }

@@ -539,7 +538,8 @@ out_file_context (char *pformat, size_t prefix_len, char 
const *filename)

 /* Print statfs info.  Return zero upon success, nonzero upon failure.  */
 static bool ATTRIBUTE_WARN_UNUSED_RESULT
-print_statfs (char *pformat, size_t prefix_len, char m, char const *filename,
+print_statfs (char *pformat, size_t prefix_len, unsigned int m,
+              char const *filename,
               void const *data)
 {
   STRUCT_STATVFS const *statfsbuf = data;
@@ -711,9 +711,19 @@ print_mount_point:
   return fail;
 }

+/* Map a TS with negative TS.tv_nsec to {0,0}.  */
+static inline struct
+timespec neg_to_zero (struct timespec ts)
+{
+  if (0 <= ts.tv_nsec)
+    return ts;
+  struct timespec z = {0, 0};
+  return z;
+}
+
 /* Print stat info.  Return zero upon success, nonzero upon failure.  */
 static bool
-print_stat (char *pformat, size_t prefix_len, char m,
+print_stat (char *pformat, size_t prefix_len, unsigned int m,
             char const *filename, void const *data)
 {
   struct stat *statbuf = (struct stat *) data;
@@ -815,31 +825,39 @@ print_stat (char *pformat, size_t prefix_len, char m,
       }
       break;
     case 'W':
-      {
-        struct timespec t = get_stat_birthtime (statbuf);
-        if (t.tv_nsec < 0)
-          out_string (pformat, prefix_len, "-");
-        else
-          out_string (pformat, prefix_len, epoch_time (t));
-      }
+      out_string (pformat, prefix_len,
+                  epoch_sec (neg_to_zero (get_stat_birthtime (statbuf))));
+      break;
+    case 'W' + 256:
+      out_string (pformat, prefix_len,
+                  epoch_ns (neg_to_zero (get_stat_birthtime (statbuf))));
       break;
     case 'x':
       out_string (pformat, prefix_len, human_time (get_stat_atime (statbuf)));
       break;
     case 'X':
-      out_string (pformat, prefix_len, epoch_time (get_stat_atime (statbuf)));
+      out_string (pformat, prefix_len, epoch_sec (get_stat_atime (statbuf)));
+      break;
+    case 'X' + 256:
+      out_string (pformat, prefix_len, epoch_ns (get_stat_atime (statbuf)));
       break;
     case 'y':
       out_string (pformat, prefix_len, human_time (get_stat_mtime (statbuf)));
       break;
     case 'Y':
-      out_string (pformat, prefix_len, epoch_time (get_stat_mtime (statbuf)));
+      out_string (pformat, prefix_len, epoch_sec (get_stat_mtime (statbuf)));
+      break;
+    case 'Y' + 256:
+      out_string (pformat, prefix_len, epoch_ns (get_stat_mtime (statbuf)));
       break;
     case 'z':
       out_string (pformat, prefix_len, human_time (get_stat_ctime (statbuf)));
       break;
     case 'Z':
-      out_string (pformat, prefix_len, epoch_time (get_stat_ctime (statbuf)));
+      out_string (pformat, prefix_len, epoch_sec (get_stat_ctime (statbuf)));
+      break;
+    case 'Z' + 256:
+      out_string (pformat, prefix_len, epoch_ns (get_stat_ctime (statbuf)));
       break;
     case 'C':
       fail |= out_file_context (pformat, prefix_len, filename);
@@ -897,7 +915,8 @@ print_esc_char (char c)
    Return zero upon success, nonzero upon failure.  */
 static bool ATTRIBUTE_WARN_UNUSED_RESULT
 print_it (char const *format, char const *filename,
-          bool (*print_func) (char *, size_t, char, char const *, void const 
*),
+          bool (*print_func) (char *, size_t, unsigned int,
+                              char const *, void const *),
           void const *data)
 {
   bool fail = false;
@@ -922,10 +941,23 @@ print_it (char const *format, char const *filename,
           {
             size_t len = strspn (b + 1, "#-+.I 0123456789");
             char const *fmt_char = b + len + 1;
+            unsigned int fmt_code;
             memcpy (dest, b, len + 1);

+            /* The ":" modifier just before the letter in %W, %X, %Y, %Z
+               tells stat to print the nanoseconds portion of the date.  */
+            if (*fmt_char == ':' && strchr ("WXYZ", fmt_char[1]))
+              {
+                fmt_code = fmt_char[1] + 256;
+                ++fmt_char;
+              }
+            else
+              {
+                fmt_code = fmt_char[0];
+              }
+
             b = fmt_char;
-            switch (*fmt_char)
+            switch (fmt_code)
               {
               case '\0':
                 --b;
@@ -941,7 +973,7 @@ print_it (char const *format, char const *filename,
                 putchar ('%');
                 break;
               default:
-                fail |= print_func (dest, len + 1, *fmt_char, filename, data);
+                fail |= print_func (dest, len + 1, fmt_code, filename, data);
                 break;
               }
             break;
@@ -1215,14 +1247,18 @@ The valid format sequences for files (without 
--file-system):\n\
       fputs (_("\
   %u   User ID of owner\n\
   %U   User name of owner\n\
-  %w   Time of file birth, or - if unknown\n\
-  %W   Time of file birth as seconds since Epoch, or - if unknown\n\
-  %x   Time of last access\n\
-  %X   Time of last access as seconds since Epoch\n\
-  %y   Time of last modification\n\
-  %Y   Time of last modification as seconds since Epoch\n\
-  %z   Time of last change\n\
-  %Z   Time of last change as seconds since Epoch\n\
+  %w   Time of file birth, human-readable; - if unknown\n\
+  %W   Time of file birth, seconds since Epoch; 0 if unknown\n\
+  %:W  Time of file birth, nanoseconds remainder; 0 if unknown\n\
+  %x   Time of last access, human-readable\n\
+  %X   Time of last access, seconds since Epoch\n\
+  %:X  Time of last access, nanoseconds remainder\n\
+  %y   Time of last modification, human-readable\n\
+  %Y   Time of last modification, seconds since Epoch\n\
+  %:Y  Time of last modification, nanoseconds remainder\n\
+  %z   Time of last change, human-readable\n\
+  %Z   Time of last change, seconds since Epoch\n\
+  %:Z  Time of last change, nanoseconds remainder\n\
 \n\
 "), stdout);

diff --git a/tests/touch/60-seconds b/tests/touch/60-seconds
index f98f0c5..d008296 100755
--- a/tests/touch/60-seconds
+++ b/tests/touch/60-seconds
@@ -30,7 +30,7 @@ echo 60.000000000 > exp || framework_failure
 # an `invalid date format'.  Specifying 60 seconds *is* valid.
 TZ=UTC0 touch -t 197001010000.60 f || fail=1

-stat --p='%Y\n' f > out || fail=1
+stat --p='%Y.%:Y\n' f > out || fail=1

 compare out exp || fail=1

--
1.7.3.1.216.g329be