Re: [review v3] Use ctime_r and localtime_r if available

[Pedro Alves <palves@redhat.com>]

On 11/6/19 8:30 PM, Christian Biesinger (Code Review) wrote:
> Patch Set 3:
> 
> I looked at gnulib's time_r module but it turns out that its localtime_r implementation is not in any way threadsafe.

It does seemingly look that way:

 struct tm *
 localtime_r (time_t const * restrict t, struct tm * restrict tp)
 {
   return copy_tm_result (tp, localtime (t));
 }

But that doesn't seem worse than the #ifdef fallback that you're leaving
in place.  I'd argue that it'd be better to use the gnulib version.

But read on, please.  Looking at:

 https://www.gnu.org/software/gnulib/manual/html_node/localtime_005fr.html

Of the problems that are relevant on our supported hosts, we see:

Portability problems fixed by Gnulib:

    This function is missing on some platforms: mingw, MSVC 14. 

And, note that localtime, like other C functions that use global
state, are thread safe on Windows, because the C run time stores
the global buffers in thread local storage.  So the gnulib
implementation ends up being thread safe there, even though
it doesn't look like it is.

Thanks,
Pedro Alves