[libc-commits] [PATCH] D98467: This introduces gmtime to LLVM libc, based on C99/C2X/Single Unix Spec.

Tue Mar 16 16:30:16 PDT 2021

rtenneti added a comment.

Thanks Siva for the comments. PTAL.

================
Comment at: libc/src/time/gmtime.cpp:18
+LLVM_LIBC_FUNCTION(struct tm *, gmtime, (const time_t *timer)) {
+  // TODO(rtenneti): This method is not thread safe.
+  static struct tm tm_out;
----------------
sivachandra wrote:
> jeffbailey wrote:
> > sivachandra wrote:
> > > Does any standard require thread safety? If yes, will making the below declaration thread-local solve the problem?
> > C20 introduces gmtime_r (and the family of _r time functions).
> > 
> > In 7.27.3:
> > 
> > " Execution of any of the functions that return a pointer to one of these static
> > objects may overwrite the information returned from any previous call to one of these functions that
> > uses the same object. "
> > 
> > But what's not clear to me is for the non-reentrant versions, is doing a thread-local OK? I'm not seeing anything in the standard discussing static buffers and the use of thread-locals to store them (it only's mentioned in the definition of errno)
> > 
> Whether thread-local is OK or not depends on the supported uses cases. For example, if a change in value in one thread is to be visible from another thread, then making it a thread-local is not a good idea. If it is unclear from the standards, I would say just leave it as you have it here.
I will implement gmtime_r in the next CL. Removed the TODO (it should have been a TODO in the description to implement gmtime_r).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98467/new/

https://reviews.llvm.org/D98467