Skip to content

build: Convert HAS_TIME_T_UNSIGNED to a compile-time check - #22849

Closed
tyler-yankee wants to merge 1 commit into
curl:masterfrom
tyler-yankee:time-t-compile
Closed

tyler-yankee wants to merge 1 commit into
curl:masterfrom
tyler-yankee:time-t-compile

Conversation

@tyler-yankee

Copy link
Copy Markdown
Contributor

(time_t)-1 > 0 is a constant expression. Checking this at compile time instead of running a program (a) is faster, and (b) can be checked when cross-compiling.

`(time_t)-1 > 0` is a constant expression. Checking this at compile
time instead of running a program (a) is faster, and (b) can be checked
when cross-compiling.

Suggested-by: Florent Castelli <florent@engflow.com>
@github-actions github-actions Bot added the build label Sep 6, 2026
@bagder
bagder requested a lite review from Copilot September 6, 2026 21:42

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change replaces runtime probes with equivalent compile-time assertions in both Autoconf and CMake without introducing behavioral risk to the produced binaries.

Pull request overview

This PR updates curl’s build-time feature detection for HAVE_TIME_T_UNSIGNED to use compile-time (non-executed) checks, improving speed and enabling correct detection while cross-compiling.

Changes:

  • Autoconf: replace the runtime CURL_RUN_IFELSE probe with an AC_COMPILE_IFELSE compile-time assertion.
  • CMake: replace check_c_source_runs() with curl_internal_test(HAVE_TIME_T_UNSIGNED) using try_compile.
  • Add a corresponding compile-time assertion test block to CMake/CurlTests.c.
File summaries
File Description
configure.ac Switches time_t unsignedness detection to a compile-time check (works for cross-compiling).
CMakeLists.txt Routes HAVE_TIME_T_UNSIGNED detection through curl’s internal try_compile-based test framework.
CMake/CurlTests.c Adds the compile-time assertion used by curl_internal_test(HAVE_TIME_T_UNSIGNED).
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@bagder bagder closed this in 2f1a21b Sep 6, 2026
@bagder

bagder commented Sep 6, 2026

Copy link
Copy Markdown
Member

Thanks!

vszakats added a commit that referenced this pull request Sep 30, 2026
- autotools: sync LibreSSL detection with rest of forks.
- CMakeLists.txt: say `time_t` sign is pre-fill (for amiga/dos),
  auto-detection also works now.
  Follow-up to 2f1a21b #22849
- urldata.h: drop redundant parentheses from macro values.
- curl_setup.h, libssh.c: fix indent.
- GHA/windows: update comment.
- GHA/windows: update Cygwin package search URL.
- GHA/linux: drop curl build option from LibreSSL configure line.
- GHA: sync option order between workflows.
- docs/INSTALL.md: mention missing AWS-LC in comment.
- test804: add newline for readability.
- OS400: clang-format.
- docs, lib: drop spaces after cast.
  Credits-to: Patrick Monnerat
  Ref: #22729
- tests/memanalyzer.pm: move literal to right-side of comparison.
- drop `v` prefix from version numbers in docs, where missing.
- minor whitespace fixes.

Closes #23146
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants