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

[Pádraig Brady]

On 26/10/10 18:59, Jim Meyering wrote:
> Jim Meyering wrote:
>> 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've adjusted things so that it's not so bad.
> 
> Setup:
> 
>     $ touch -d '2010-10-21 18:43:33.000000789' k
> 
> With the incremental patch below, you get "09" modifiers only
> when you specify no other modifier:
> 
>     $ stat -c ns_%12:X_ k
>     ns_         789_
>     $ stat -c ns_%:X_ k
>     ns_000000789_
>     $ stat -c ns_%9:X_ k
>     ns_      789_
> 
> diff --git a/src/stat.c b/src/stat.c
> index 9a6bb8f..9d3db1b 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -471,14 +471,19 @@ epoch_sec (struct timespec 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)
> +/* Output the number of nanoseconds, ARG.tv_nsec.  */
> +static void
> +out_ns (char *pformat, size_t prefix_len, struct timespec arg)
>  {
> -  static char str[10];
> -  sprintf (str, "%09ld", t.tv_nsec);
> -  return str;
> +  /* If no format modifier is specified, i.e., nothing between the
> +     "%" and ":" of "%:X", then use the default of zero-padding and
> +     a width of 9.  Otherwise, use the specified modifier(s).
> +     This is to avoid the mistake of omitting the zero padding on
> +     a number with fewer digits than the field width: when printing
> +     nanoseconds after a decimal place, the resulting floating point
> +     fraction would off by a factor of 10 or more.  */
> +  strcpy (pformat + prefix_len, prefix_len == 1 ? "09ld" : "ld");
> +  printf (pformat, arg.tv_nsec);
>  }
> 
>  static void
> @@ -829,8 +834,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>                    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))));
> +      out_ns (pformat, prefix_len, neg_to_zero (get_stat_birthtime 
> (statbuf)));
>        break;
>      case 'x':
>        out_string (pformat, prefix_len, human_time (get_stat_atime 
> (statbuf)));
> @@ -839,7 +843,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>        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)));
> +      out_ns (pformat, prefix_len, get_stat_atime (statbuf));
>        break;
>      case 'y':
>        out_string (pformat, prefix_len, human_time (get_stat_mtime 
> (statbuf)));
> @@ -848,7 +852,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>        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)));
> +      out_ns (pformat, prefix_len, get_stat_mtime (statbuf));
>        break;
>      case 'z':
>        out_string (pformat, prefix_len, human_time (get_stat_ctime 
> (statbuf)));
> @@ -857,7 +861,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>        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)));
> +      out_ns (pformat, prefix_len, get_stat_ctime (statbuf));
>        break;
>      case 'C':
>        fail |= out_file_context (pformat, prefix_len, filename);
> 
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> index 94e8ebf..58a700b 100644
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -10699,18 +10699,23 @@ stat invocation
>  @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{0}
> -@item %:W - Time of file birth: 0-padded nanoseconds remainder, or @samp{0}
> +@item %:W - Time of file birth: 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 %:X - Time of last access: 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 %:Y - Time of last modification: 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
> +@item %:Z - Time of last change: nanoseconds remainder
>  @end itemize
> 
> +Note that each of @samp{%:W}, @samp{%:X}, @samp{%:Y} and @samp{%:Z}
> +prints its zero-padded number of nanoseconds on a field of width 9.
> +However, if you specify anything between the @samp{%} and @samp{:},
> +e.g., @samp{%10:X}, your modifier replaces the default of @samp{09}.
> +
>  The mount point printed by @samp{%m} is similar to that output
>  by @command{df}, except that:
>  @itemize @bullet
> 
> Here's the combined patch:
> 
>>From eba6292f45c9b46fb8a64f8a017bf2e9437cc60e 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 %X-%Y-%Z change; use e.g., %:X to print 
> fractional seconds
> 
> This reverts part of the recent commit 9069af45,
> "stat: print timestamps to full resolution", which made %X, %Y, %Z
> print floating point numbers.  We prefer to retain portability of
> %X, %Y and %Z uses, while still providing access to full-resolution
> time stamps via modified format strings.  Also make the new
> %W consistent.
> * 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.

That is no longer the case since you're using %ld rather than %s
$ ./src/stat -c [%12.10:Y] /
[  0521544487]
$ ./src/stat -c [%3.3:Y] /
[521544487]

Do we still want to support truncation?
It seems useful. date supports it but unconventionally

$ src/date '+[%N]' --ref=/usr
[031367045]
$ src/date '+[%-N]' --ref=/usr
[31367045]
$ src/date '+[%12N]' --ref=/usr
[000031367045]
$ src/date '+[%12.3N]' --ref=/usr
[        %12.3N]
$ src/date '+[%3N]' --ref=/usr
[031]

cheers,
Pádraig.