localtime: detect thread-safe alternatives and use them #19957

vszakats · 2025-12-13T04:09:52Z

add local API toolx_localtime() to wrap the banned function
localtime(). Used from libcurl, libtests and test servers.
auto-detect and use localtime_r() where available (e.g. Linux).
Also to support multi-threading.
use localtime_s() on Windows. It requires MSVC or mingw-w64 v4+.
Also to support multi-threading.
Use local workaround to also support mingw-w64 v3.
add src/toolx to keep internal APIs used by the curl tool and tests,
but not by libcurl. toolx_localtime() is the first API in it.
replace localtime() calls with toolx_localtime().
Except in examples.
note Windows XP's default msvcrt.dll doesn't offer secure CRT APIs.
XP likely needs a newer version of this DLL, or may not run.
note that localtime() mirrors gmtime(), with the difference that
gmtime()'s internal wrapper lives in curlx.

Also:

drop redundant int casts.

Refs:
https://learn.microsoft.com/cpp/c-runtime-library/reference/localtime-localtime32-localtime64
https://learn.microsoft.com/cpp/c-runtime-library/reference/localtime-s-localtime32-s-localtime64-s
https://pubs.opengroup.org/onlinepubs/9799919799/functions/localtime.html
https://linux.die.net/man/3/localtime_r

Ref: #19955 (for gmtime_r())
Follow-up to 54d9f06

rebase on curlx: move Curl_gmtime(), use gmtime_s() on Windows #19955. [no longer critical, save a single collision.]
figure out if can be made work with mingw-w64 v3. see gmtime PR.

vszakats · 2025-12-13T04:26:58Z

Test 3207 flaky failure in mingw, AM x86_64 c-ares U:
https://github.com/curl/curl/actions/runs/20186607715/job/57957786919?pr=19957

bagder · 2025-12-13T13:30:21Z

Isn't the only use of localtime in src/tool_cb_dbg.c ? Why add it to libcurl at all?

vszakats · 2025-12-13T19:22:10Z

Isn't the only use of localtime in src/tool_cb_dbg.c ? Why add it to libcurl at all?

It's also used from libtests (and servers) to get the time for debug logs.

bagder · 2025-12-13T21:58:40Z

I still object to adding anything to libcurl that isn't used by libcurl.

vszakats · 2025-12-14T06:52:21Z

I don't completely get the point of this rule, but more importantly how to solve this then? Maintain 3 copies of this function doesn't look practical.

Or maybe make it a macro and put it in curl_setup.h?

Or add to a separate curlx c file that's excluded from libcurl?

Or add a guard to exclude it while building libcurl?

bagder · 2025-12-14T10:50:53Z

The rule is there to keep the crown jewel as slim as it can be and not add stuff that isn't used. For size, for complexity and more.

The tool and the tests don't need threadsafe localtime versions to begin with - at least not for as long as we don't use threads in the tool.

Or maybe make it a macro and put it in curl_setup.h?
Or add to a separate curlx c file that's excluded from libcurl?
Or add a guard to exclude it while building libcurl?

Neither I think. Because in my mind, anything that adds anything in lib/ is wrong if that code is not also used by code in lib/.

We could potentially create a curlt (name just a suggestion) set of functions that are written for the tool but can be re-used by the tests.

vszakats · 2025-12-14T11:22:08Z

The rule is there to keep the crown jewel as slim as it can be and not add stuff that isn't used. For size, for complexity and more.

ACK.

The tool and the tests don't need threadsafe localtime versions to begin with - at least not for as long as we don't use threads in the tool.

Or maybe make it a macro and put it in curl_setup.h?
Or add to a separate curlx c file that's excluded from libcurl?
Or add a guard to exclude it while building libcurl?

Neither I think. Because in my mind, anything that adds anything in lib/ is wrong if that code is not also used by code in lib/.

We could potentially create a curlt (name just a suggestion) set of functions that are written for the tool but can be re-used by the tests.

On the other hand localtime is a banned function, yet it's used from places,
and it's unlikely to go away. It's also deprecated by MSVC. (It's also an exact
mirror of gmtime, with identical issues/properties.) So I figure it'd be nice
to fix, and fix in one place, then forget.

How does src/toolx sound?

bagder · 2025-12-14T11:32:53Z

How does src/toolx sound?

Perfect!

add curlx_localtime curl_setup.h drop localtime use curlx_localtime https://learn.microsoft.com/cpp/c-runtime-library/reference/localtime-localtime32-localtime64 https://learn.microsoft.com/cpp/c-runtime-library/reference/localtime-s-localtime32-s-localtime64-s https://en.cppreference.com/w/c/chrono/localtime move to toolx more adjustments

Can be added once it makes sense (to mirror curlx.h), for now it's overkill

Copilot

Pull request overview

This PR introduces a thread-safe wrapper for the localtime() function by creating a new internal API toolx_localtime() in a new src/toolx directory. The wrapper automatically detects and uses thread-safe alternatives: localtime_r() on Unix-like systems and localtime_s() on Windows (with special handling for mingw-w64 v3). All occurrences of localtime() in the curl tool, test servers, and test libraries have been replaced with this new wrapper to improve thread-safety across the codebase.

Introduces toolx_localtime() as a portable, thread-safe replacement for localtime()
Creates new src/toolx directory for tool-internal APIs shared between curl tool and tests
Updates build systems (Autotools, CMake, Windows projects) to include the new toolx module and detect localtime_r availability

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/toolx/tool_time.h	New header defining the toolx_localtime() API
src/toolx/tool_time.c	Implementation using platform-specific thread-safe localtime variants
src/tool_cb_dbg.c	Replaces localtime() with toolx_localtime() in debug callback
tests/server/util.c	Converts logging to use thread-safe toolx_localtime()
tests/libtest/testtrace.c	Updates test tracing to use toolx_localtime()
tests/server/Makefile.inc	Adds TOOLX_C variable for new toolx sources
tests/server/Makefile.am	Includes toolx headers and sources in build
tests/server/CMakeLists.txt	Updates CMake to build with toolx
tests/libtest/Makefile.inc	Adds TOOLX_C for libtest builds
tests/libtest/Makefile.am	Integrates toolx into libtest build
tests/libtest/CMakeLists.txt	Updates CMake for toolx integration
src/Makefile.inc	Adds toolx sources and headers to curl build
projects/generate.bat	Adds toolx file generation for Windows projects
projects/Windows/tmpl/curl.vcxproj	Includes toolx in Visual Studio project template
m4/curl-functions.m4	Adds autoconf macro to detect localtime_r availability
configure.ac	Invokes CURL_CHECK_FUNC_LOCALTIME_R during configuration
CMakeLists.txt	Adds check for localtime_r symbol
lib/curl_config.h.cmake	Defines HAVE_LOCALTIME_R configuration
lib/config-plan9.h	Enables HAVE_LOCALTIME_R for Plan 9
lib/config-os400.h	Enables HAVE_LOCALTIME_R for OS/400
CMake/win32-cache.cmake	Sets HAVE_LOCALTIME_R=0 for Windows
CMake/unix-cache.cmake	Sets HAVE_LOCALTIME_R=1 for Unix
lib/curl_setup.h	Updates comment to reflect localtime() is no longer used in tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/toolx/tool_time.h

src/toolx/tool_time.c

tests/server/util.c

tests/libtest/testtrace.c

src/tool_cb_dbg.c

m4/curl-functions.m4

To avoid callers using trash on failure.

to silence AIs, and avoid unnecessary/misleading variations of this code.

This reverts commit 885c684.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/server/util.c

tests/libtest/testtrace.c

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Move `Curl_gmtime()` to curlx and rename to `curlx_gmtime()`. Then call the internal wrapper also from the curl tool, to avoid using the banned `gmtime()` directly, and using better, thread-safe alternatives when available. Windows `gmtime_s()` requires mingw-w64 v4+ or MSVC. Use local workaround to also support mingw-w64 v3. `gmtime_s()` also makes defining `_CRT_SECURE_NO_WARNINGS` unnecessary. Also: - lib: drop unused `parsedate.h` includes. - drop redundant cast from `gmtime_r()` result. - autotools: reverse condition in the proto detection to avoid misleading readers. (the condition plays no role in detection.) - note Windows XP's default `msvcrt.dll` doesn't offer secure CRT APIs. XP likely needs a newer version of this DLL, or may not run. Refs: https://learn.microsoft.com/cpp/c-runtime-library/reference/gmtime-gmtime32-gmtime64 https://learn.microsoft.com/cpp/c-runtime-library/reference/gmtime-s-gmtime32-s-gmtime64-s https://pubs.opengroup.org/onlinepubs/9799919799/functions/gmtime.html https://linux.die.net/man/3/gmtime_r Ref: #19957 (for `localtime_r()`) Follow-up to 54d9f06 Closes #19955

vszakats marked this pull request as draft December 13, 2025 04:09

github-actions bot added cmdline tool tests labels Dec 13, 2025

vszakats mentioned this pull request Dec 13, 2025

curlx: move Curl_gmtime(), use gmtime_s() on Windows #19955

Closed

2 tasks

vszakats force-pushed the localtime branch from ef43c09 to 8f7f817 Compare December 14, 2025 12:32

vszakats added 6 commits December 14, 2025 13:40

dist toolx.h

a6b3d9e

misplaced include

64a6051

drop umbrella header

326f62a

Can be added once it makes sense (to mirror curlx.h), for now it's overkill

legacy vc files

f7e936d

legacy vc files fixup

c90995e

add mingw-w64 v3 workaround

a6ac5c0

vszakats marked this pull request as ready for review December 14, 2025 15:31

vszakats added 2 commits December 14, 2025 16:34

cleanup to prev

a0e4ec7

cleanup

d31f920

vszakats requested a review from Copilot December 14, 2025 15:38

Copilot started reviewing on behalf of vszakats December 14, 2025 15:38 View session

oops

a8204ad

Copilot AI reviewed Dec 14, 2025

View reviewed changes

vszakats added 3 commits December 14, 2025 17:48

clean result on failure

23167a9

To avoid callers using trash on failure.

cleanup

92b9fa0

drop redundant cast

4c90a98

vszakats added 3 commits December 14, 2025 18:04

m4: sync prototype detection

885c684

to silence AIs, and avoid unnecessary/misleading variations of this code.

Revert "m4: sync prototype detection"

2e42a1e

This reverts commit 885c684.

tidy up detection (go simpler)

f5f95f0

vszakats requested a review from Copilot December 14, 2025 18:02

Copilot started reviewing on behalf of vszakats December 14, 2025 18:03 View session

Copilot AI reviewed Dec 14, 2025

View reviewed changes

tests/server/util.c Outdated Show resolved Hide resolved

tests/libtest/testtrace.c Outdated Show resolved Hide resolved

vszakats added 2 commits December 14, 2025 19:35

cleanup

6d6b597

check return value and clear time struct on error

d0b90be

vszakats requested a review from Copilot December 14, 2025 18:46

Copilot started reviewing on behalf of vszakats December 14, 2025 18:46 View session

Copilot AI reviewed Dec 14, 2025

View reviewed changes

vszakats closed this in 32454b9 Dec 16, 2025

vszakats deleted the localtime branch December 16, 2025 13:32

Uh oh!

localtime: detect thread-safe alternatives and use them #19957

localtime: detect thread-safe alternatives and use them #19957

Conversation

vszakats commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vszakats commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bagder commented Dec 13, 2025

Uh oh!

vszakats commented Dec 13, 2025

Uh oh!

bagder commented Dec 13, 2025

Uh oh!

vszakats commented Dec 14, 2025

Uh oh!

bagder commented Dec 14, 2025

Uh oh!

vszakats commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bagder commented Dec 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

vszakats commented Dec 13, 2025 •

edited

Loading

vszakats commented Dec 13, 2025 •

edited

Loading

vszakats commented Dec 14, 2025 •

edited

Loading