Bug 162636 - [CMake] Add HAVE_LOCALTIME_R definition
Summary: [CMake] Add HAVE_LOCALTIME_R definition
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-27 15:06 PDT by Don Olmstead
Modified: 2016-09-27 21:55 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.22 KB, patch)
2016-09-27 15:14 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 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.
Comment 1 Don Olmstead 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.
Comment 2 Alex Christensen 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.
Comment 3 Don Olmstead 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.
Comment 4 WebKit Commit Bot 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>
Comment 5 WebKit Commit Bot 2016-09-27 21:55:17 PDT
All reviewed patches have been landed.  Closing bug.