[coreutils] [PATCH 2/2] stat: print timestamps to full resolution
* src/stat.c (epoch_time): New function. (print_stat): Use it for %[WXYZ]. * NEWS: Document this. * tests/touch/60-seconds: Adjust test to match. --- It bugs me that %x has more information than %X in 'stat --format', especially, since we don't support any format modifiers for getting at the additional information. We're already incompatible with BSD stat(1) format modifiers, and there is no standard for stat(1), so I wasn't too worried about changing the meaning of existing modifiers rather than burning new letters just for the nanosecond portions. And now that POSIX 2008 requires nanonsecond resolution in stat(2), you could argue that we should always be displaying it. NEWS |3 +++ src/stat.c | 32 +--- tests/touch/60-seconds |2 +- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index 0c7cc38..690f693 100644 --- a/NEWS +++ b/NEWS @@ -40,6 +40,9 @@ 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, diff --git a/src/stat.c b/src/stat.c index c465e77..fb9b2c2 100644 --- a/src/stat.c +++ b/src/stat.c @@ -461,6 +461,19 @@ human_time (struct timespec t) return str; } +static char * ATTRIBUTE_WARN_UNUSED_RESULT +epoch_time (struct timespec t) +{ + static char str[INT_STRLEN_BOUND (time_t) + sizeof ".N"]; + 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; +} + static void out_string (char *pformat, size_t prefix_len, char const *arg) { @@ -802,38 +815,27 @@ print_stat (char *pformat, size_t prefix_len, char m, struct timespec t = get_stat_birthtime (statbuf); if (t.tv_nsec < 0) out_string (pformat, prefix_len, "-"); -else if (TYPE_SIGNED (time_t)) - out_int (pformat, prefix_len, t.tv_sec); else - out_uint (pformat, prefix_len, t.tv_sec); + out_string (pformat, prefix_len, epoch_time (t)); } break; case 'x': out_string (pformat, prefix_len, human_time (get_stat_atime (statbuf))); break; case 'X': - if (TYPE_SIGNED (time_t)) -out_int (pformat, prefix_len, statbuf->st_atime); - else -out_uint (pformat, prefix_len, statbuf->st_atime); + out_string (pformat, prefix_len, epoch_time (get_stat_atime (statbuf))); break; case 'y': out_string (pformat, prefix_len, human_time (get_stat_mtime (statbuf))); break; case 'Y': - if (TYPE_SIGNED (time_t)) -out_int (pformat, prefix_len, statbuf->st_mtime); - else -out_uint (pformat, prefix_len, statbuf->st_mtime); + out_string (pformat, prefix_len, epoch_time (get_stat_mtime (statbuf))); break; case 'z': out_string (pformat, prefix_len, human_time (get_stat_ctime (statbuf))); break; case 'Z': - if (TYPE_SIGNED (time_t)) -out_int (pformat, prefix_len, statbuf->st_ctime); - else -out_uint (pformat, prefix_len, statbuf->st_ctime); + out_string (pformat, prefix_len, epoch_time (get_stat_ctime (statbuf))); break; case 'C': fail |= out_file_context (filename, pformat, prefix_len); diff --git a/tests/touch/60-seconds b/tests/touch/60-seconds index f858a30..f98f0c5 100755 --- a/tests/touch/60-seconds +++ b/tests/touch/60-seconds @@ -23,7 +23,7 @@ fi . $srcdir/test-lib.sh -echo 60 > exp || framework_failure +echo 60.0 > exp || framework_failure # Before coreutils-7.7, this would fail, complaining of -- 1.7.2.3
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
On 01/10/10 00:32, Eric Blake wrote: > * src/stat.c (epoch_time): New function. > (print_stat): Use it for %[WXYZ]. > * NEWS: Document this. > * tests/touch/60-seconds: Adjust test to match. > --- > > It bugs me that %x has more information than %X in 'stat --format', > especially, since we don't support any format modifiers for getting > at the additional information. We're already incompatible with > BSD stat(1) format modifiers, and there is no standard for stat(1), > so I wasn't too worried about changing the meaning of existing > modifiers rather than burning new letters just for the nanosecond > portions. And now that POSIX 2008 requires nanonsecond resolution > in stat(2), you could argue that we should always be displaying it. It looks like ext4 at least supports nanosecond resolution timestamps $ date --reference=/ext4/ +%s.%N 1285519989.081870491 $ date --reference=/ext3/ +%s.%N 1266874130.0 There is a fair chance that scripts may break that assume %X is an integer. I'd be all on for it otherwise. As it is I'm 60:40 for the change. cheers, Pádraig. > > +static char * ATTRIBUTE_WARN_UNUSED_RESULT > +epoch_time (struct timespec t) > +{ > + static char str[INT_STRLEN_BOUND (time_t) + sizeof ".N"]; > + 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? cheers, Pádraig.
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
Pádraig Brady wrote: > On 01/10/10 00:32, Eric Blake wrote: >> * src/stat.c (epoch_time): New function. >> (print_stat): Use it for %[WXYZ]. >> * NEWS: Document this. >> * tests/touch/60-seconds: Adjust test to match. >> --- >> >> It bugs me that %x has more information than %X in 'stat --format', >> especially, since we don't support any format modifiers for getting >> at the additional information. We're already incompatible with >> BSD stat(1) format modifiers, and there is no standard for stat(1), >> so I wasn't too worried about changing the meaning of existing >> modifiers rather than burning new letters just for the nanosecond >> portions. And now that POSIX 2008 requires nanonsecond resolution >> in stat(2), you could argue that we should always be displaying it. > > It looks like ext4 at least supports > nanosecond resolution timestamps > > $ date --reference=/ext4/ +%s.%N > 1285519989.081870491 > $ date --reference=/ext3/ +%s.%N > 1266874130.0 > > There is a fair chance that scripts may break > that assume %X is an integer. > I'd be all on for it otherwise. > As it is I'm 60:40 for the change. Good point. There's at least one script in the code indexed here http://codesearch.google.com/codesearch?as_q=\bstat\+%2B-.*%25[0-9.]*[WXYZ]&sbtn=Search that might fail: eclipseclient's src/main/java/com/sshtools/j2ssh/SftpClient.java does this: 513: String mtimeS = run("stat -c %Y "+localPath); 514: String atimeS = run("stat -c %X "+localPath); 515: long atime = Long.parseLong(atimeS); Depending on how strict that parsing code is... Thus, I'm a little leery of the change, but not enough to require something else, since the alternative seems wasteful and relatively short-sighted. So go for it ;-) >> +static char * ATTRIBUTE_WARN_UNUSED_RESULT >> +epoch_time (struct timespec t) >> +{ >> + static char str[INT_STRLEN_BOUND (time_t) + sizeof ".N"]; >> + 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 ;-) 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.
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
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 ".N"]; + 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 ".N"]; + /* 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 *.0*) 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 ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
Eric Blake wrote: > 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 ".N"]; + 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 Someone can probably dig one up, but I won't insist. > 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. That's fine. >> 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. Thanks.
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
On 10/01/2010 09:03 AM, Eric Blake wrote: 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? My understanding of POSIX is that time_t was permitted to be floating point to allow for subsecond resolution in early versions of the standard, in earlier versions of the standard where there was no other required support for subsecond resolution (that is, by being weasel-wordy about whether time_t was an integer, it was possible to do a weirdnix implementation that improved QoI by using floating-point time_t). But now that POSIX 2008 uniformly requires subsecond resolution via struct timespec, it makes less sense to have a floating-point time_t with a fractional value alongside the tv_nsec fractional value. I think I'm going to write a POSIX bug requesting that time_t be tightened to integral in light of the fact that subsecond support is now uniformly guaranteed via tv_nsec, and given the argument that existing platforms never took advantage of float time_t. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
On 10/01/2010 09:19 AM, Eric Blake wrote: Correct - time_t need not be integral, but do we have any proof of a system using a floating-point time_t? I think I'm going to write a POSIX bug requesting that time_t be tightened to integral in light of the fact that subsecond support is now uniformly guaranteed via tv_nsec, and given the argument that existing platforms never took advantage of float time_t. http://austingroupbugs.net/view.php?id=327 -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
On 10/01/2010 04:34 PM, Eric Blake wrote: On 10/01/2010 09:19 AM, Eric Blake wrote: Correct - time_t need not be integral, but do we have any proof of a system using a floating-point time_t? I think I'm going to write a POSIX bug requesting that time_t be tightened to integral in light of the fact that subsecond support is now uniformly guaranteed via tv_nsec, and given the argument that existing platforms never took advantage of float time_t. http://austingroupbugs.net/view.php?id=327 This was accepted in yesterday's Austin Group meeting. Would anyone oppose a patch that adds a compile-time verification in gnulib's that time_t is integral on all platforms that we support, given that POSIX now requires this even though C99 doesn't? That way, projects using gnulib can rely on the POSIX requirement that time_t is an integer (still unspecified whether it is signed or unsigned, and whether it is 32- or 64-bit, but much easier to code with than having to worry about floating point in the mix). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org
Re: [coreutils] [PATCH 2/2] stat: print timestamps to full resolution
Eric Blake wrote: > On 10/01/2010 04:34 PM, Eric Blake wrote: >> On 10/01/2010 09:19 AM, Eric Blake wrote: Correct - time_t need not be integral, but do we have any proof of a system using a floating-point time_t? >>> >>> I think I'm going to write a POSIX bug requesting that time_t be >>> tightened to integral in light of the fact that subsecond support is now >>> uniformly guaranteed via tv_nsec, and given the argument that existing >>> platforms never took advantage of float time_t. >> >> http://austingroupbugs.net/view.php?id=327 > > This was accepted in yesterday's Austin Group meeting. Would anyone Nice. > oppose a patch that adds a compile-time verification in gnulib's > that time_t is integral on all platforms that we support, > given that POSIX now requires this even though C99 doesn't? That way, > projects using gnulib can rely on the POSIX requirement that time_t is > an integer (still unspecified whether it is signed or unsigned, and > whether it is 32- or 64-bit, but much easier to code with than having > to worry about floating point in the mix). I can't imagine anyone objecting. Thanks!