RESOLVED FIXED162636
[CMake] Add HAVE_LOCALTIME_R definition 
https://bugs.webkit.org/show_bug.cgi?id=162636
Summary [CMake] Add HAVE_LOCALTIME_R definition
Don Olmstead
Reported 2016-09-27 15:06:33 PDT
There should be a check for whether localtime_r is present. If not then localtime_s should be used.
Attachments
Patch (3.22 KB, patch)
2016-09-27 15:14 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Don Olmstead
Comment 1 2016-09-27 15:14:36 PDT
Created attachment 290012 [details] Patch This patch will search for localtime_r through CMake. If its there then it will attempt to use it. If its not there then it defaults to localtime_s which is a C++11 extension to time.h. The difference between the two seems to be that localtime_r is thread safe. If that's not a concern then maybe it should just be localtime_s across the board. MINGW appears to have the extension so I removed that. Windows still has its own localtime_s which reverses the parameters.
Alex Christensen
Comment 2 2016-09-27 17:19:03 PDT
Comment on attachment 290012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290012&action=review > Source/WTF/wtf/DateMath.cpp:126 > +#if COMPILER(MSVC) This is inconsistent. In GregorianDateTime.cpp we only check HAVE(LOCALTIME_R) and here we check both. I think this check is unneeded.
Don Olmstead
Comment 3 2016-09-27 17:28:08 PDT
In DateMath.cpp getLocalTime is called from calculateUTCOffset, calculateDSTOffset and calculateLocalTimeOffset. In the first two they are not called for Windows because there's an #if OS(WINDOWS) guard. In GregorianDateTime.cpp localtime is not called by Windows because of the #if OS(WINDOWS) guard. Any particular way you want this resolved? I could do another header that can be used from both files. Happy to approach it however you like.
WebKit Commit Bot
Comment 4 2016-09-27 21:55:13 PDT
Comment on attachment 290012 [details] Patch Clearing flags on attachment: 290012 Committed r206496: <http://trac.webkit.org/changeset/206496>
WebKit Commit Bot
Comment 5 2016-09-27 21:55:17 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.