Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution

[Eric Blake]

On 10/01/2010 02:21 AM, Jim Meyering wrote:
> > > +static char * ATTRIBUTE_WARN_UNUSED_RESULT
> > > +epoch_time (struct timespec t)
> > > +{
> > > +  static char str[INT_STRLEN_BOUND (time_t) + sizeof ".NNNNNNNNN"];
> > > +  if (TYPE_SIGNED (time_t))
> > > +    sprintf (str, "%" PRIdMAX ".%09lu", (intmax_t) t.tv_sec,
> > > +             (unsigned long) t.tv_nsec);
> > > +  else
> > > +    sprintf (str, "%" PRIuMAX ".%09lu", (uintmax_t) t.tv_sec,
> > > +             (unsigned long) t.tv_nsec);
> > > +  return str;
> >
> > time_t can be a float on weird platforms I think?
>
> If you use nstrftime, no one can complain ;-)

Correct - time_t need not be integral, but do we have any proof of a 
system using a floating-point time_t?  Using nstrftime("%s.%N") would 
indeed avoid that issue, but at what expense?  We'd have to convert 
t.tv_sec to a struct tm, just to have nstrftime convert that struct back 
to a time_t.

As it is, nstrftime's approach risks rounding errors if time_t is a 
floating point (should this be reported as a theoretical bug to the 
gnulib list?):

            /* Generate string value for T using time_t arithmetic;
               this works even if sizeof (long) < sizeof (time_t).  */

            bufp = buf + sizeof (buf) / sizeof (buf[0]);
            negative_number = t < 0;

            do
              {
                int d = t % 10;
                t /= 10;
                *--bufp = (negative_number ? -d : d) + L_('0');
              }
            while (t != 0);

I'm thinking that sticking with a cast to [u]intmax_t is the right thing 
to do for now, but am adding a comment to the source code.

> Other than that, my only suggestion would be to mention what
> these directives mean, so the NEWS entry stands more on its own.
> I had to look them up.
>
>    +  stat now outputs the full timestamp resolution for the %X, %Y, and
>    +  %Z format directives.

Reworded accordingly, and moved to changed behavior rather than new feature.

diff --git i/NEWS w/NEWS
index 690f693..2ce03e3 100644
--- i/NEWS
+++ w/NEWS
@@ -40,9 +40,6 @@ GNU coreutils NEWS 
-*- outline -*-
   for a file.  It also accepts the %w and %W format directives for
   outputting the birth time of a file, if one is available.

-  stat now outputs the full timestamp resolution for the %X, %Y, and
-  %Z format directives.
-
 ** Changes in behavior

   df now consistently prints the device name for a bind mounted file,
@@ -78,6 +75,11 @@ GNU coreutils NEWS 
 -*- outline -*-
   merely accepted and ignored, for compatibility.  Starting two years
   ago, with coreutils-7.0, its use evoked a warning.

+  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.
+
   touch's --file option is no longer recognized.  Use --reference=F (-r)
   instead.  --file has not been documented for 15 years, and its use has
   elicited a warning since coreutils-7.1.
diff --git i/src/stat.c w/src/stat.c
index b809330..5d5e620 100644
--- i/src/stat.c
+++ w/src/stat.c
@@ -465,6 +465,15 @@ static char * ATTRIBUTE_WARN_UNUSED_RESULT
 epoch_time (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 ".%09lu", (intmax_t) t.tv_sec,
              (unsigned long) t.tv_nsec);
diff --git i/tests/misc/stat-birthtime w/tests/misc/stat-birthtime
index d4f372e..2da326a 100755
--- i/tests/misc/stat-birthtime
+++ w/tests/misc/stat-birthtime
@@ -33,8 +33,7 @@ ctime=$(stat --format %Z a) || fail=1

 case $(stat --format %x a) in
   *.000000000*) sleep 2;; # worst case file system is FAT
-  *) # FIXME: sleep .1 would be sufficient if %X showed nanoseconds
- sleep 1;; # should be adequate for any system with subsecond resolution
+  *) sleep .1;; # should be adequate for any system with subsecond 
resolution
 esac

 touch a || fail=1


--
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org