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

2010-09-30 Thread Eric Blake
* 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

2010-09-30 Thread Pádraig Brady
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

2010-10-01 Thread Jim Meyering
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

2010-10-01 Thread 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 ".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

2010-10-01 Thread Jim Meyering
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

2010-10-01 Thread Eric Blake

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

2010-10-01 Thread Eric Blake

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

2010-10-08 Thread Eric Blake

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

2010-10-08 Thread Jim Meyering
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!