Re: [PATCH 7/7] misc: syslog: Use RFC5424

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 05/10/2021 16:07, Paul Eggert wrote:
> On 10/5/21 6:56 AM, Adhemerval Zanella via Libc-alpha wrote:
> 
>> +struct timebuf_t
>> +{
>> +  char b[sizeof ("YYYY-MM-DDThh:mm:ss.nnnnnnZ")];
>> +};
> 
> This won't work if people fiddle with their system clocks and set the time far in the future or past. I suggest including <intprops.h> and using the following instead:
> 
>   struct timebuf
>   {
>     /* Leave room for outlandish but possible years.
>        The "+ 1" is for strftime's adding 1900 to tm_year.  */
>     char b[INT_STRLEN_BOUND (int) + 1
>        + sizeof "-MM-DDThh:mm:ss.nnnnnnZ"];
>   };

Indeed, I did not consider it and the suggestion makes sense.

> 
> 
>> +  struct tm now_tm;
>> +  __gmtime64_r (&ts.tv_sec, &now_tm);
> 
> This messes up if __gmtime64_r fails, which is possible when time_t is very large (or very negative).
> 
>> +  __snprintf (timebuf->b, sizeof (timebuf->b),
>> +              "%04d-%02d-%02dT%02d:%02d:%02d.%06dZ",
>> +              now_tm.tm_year + 1900, now_tm.tm_mon + 1, now_tm.tm_mday,
>> +              now_tm.tm_hour, now_tm.tm_min, now_tm.tm_sec,
>> +              (int32_t) ts.tv_nsec / 1000);
> 
> This messes up if now_tm.tm_year + 1900 overflows. (strftime has a similar bug; it should get fixed too but one thing at a time.) Also, that 'int32_t' should be plain 'int', to match the format. And it's a bit better to not use casts when it's easy, as is the case here.
> 
> I suggest something like this instead (I haven't tested it):
> 
>   struct tm *gmtm = __gmtime64_r (&ts.tv_sec, &now_tm);
>   if (gmtm == NULL)
>     strcpy (timebuf->b, "-");
>   else
>     {
>       size_t datebytes = __strftime_l (timebuf->b, sizeof timebuf->b,
>                                        "%FT%T", gmtm, _nl_C_locobj_ptr);
>       int usec = ts.tv_nsec / 1000;
>       __snprintf (timebuf->b + datebytes, sizeof timebuf->b - datebytes,
>                   ".%06dZ", usec);
>     }
> 
> The "-" is as per RFC 5434's NILVALUE.

It make sense, I changes to your suggestion thanks.