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

[Jim Meyering]

Pádraig Brady wrote:

> 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]

Thanks for another good catch.

I think I have fixed things, finally.
Here's how my latest works:

  $ touch -d '2010-10-21 18:43:33.012345678' k
  $ src/stat -c %3:X k
  12345678
  $ src/stat -c %3.3:X k
   12
  $ src/stat -c %03.3:X k
  012
  $ src/stat -c %:X k
  012345678
  $ src/stat -c %03.3:X k
  012
  $ src/stat -c %010.3:X k
  0000000012
  $ src/stat -c %-3.3:X k
  12

I'll post code when I have tests.