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

[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 ".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

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.