Skip to content

build: assume snprintf() in mprintf, drop feature check#20763

Closed
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:reqsnprintf
Closed

build: assume snprintf() in mprintf, drop feature check#20763
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:reqsnprintf

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Feb 27, 2026

  • it was already required for curl_*printf() float/double support.
  • some curl tests always fail without it.
  • it was already assumed to be present to build test servers.
    Source code did not check for HAVE_SNPRINTF detection variable.
  • it was already required to build examples.

Windows builds stopped using this detection and the function via earlier
commits.

Follow-up to 64f28b8 #20765
Follow-up to 935b1bd #9570 #9569


@vszakats vszakats added build feature-window A merge of this requires an open feature window labels Feb 27, 2026
@testclutch

This comment was marked as outdated.

@vszakats vszakats changed the title build: assume snprintf(), drop feature checks build: assume snprintf() in mprintf, drop feature checks Mar 2, 2026
@vszakats vszakats force-pushed the reqsnprintf branch 2 times, most recently from d19237a to c926f0b Compare March 2, 2026 21:45
@vszakats vszakats requested a review from Copilot March 3, 2026 00:21
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 3, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 3, 2026

🤖 Augment PR Summary

Summary: Removes build-time detection/defines for snprintf() and treats it as a required libc feature across both autotools and CMake.

Change: Simplifies lib/mprintf.c float/double formatting to always call snprintf() (or curlx_win32_snprintf on Windows), avoiding broken builds/tests when it’s absent.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies libcurl’s build configuration by assuming snprintf() is available (needed for curl_*printf() float/double formatting) and removing the corresponding feature-detection plumbing.

Changes:

  • Make lib/mprintf.c always use snprintf() for float/double formatting on non-Windows platforms.
  • Remove HAVE_SNPRINTF generation from the CMake config header template and Unix cache.
  • Drop snprintf() feature checks from both Autotools (configure.ac) and CMake (CMakeLists.txt).

Reviewed changes

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

Show a summary per file
File Description
lib/mprintf.c Removes the non-snprintf() fallback path for float/double formatting (non-Windows now always uses snprintf()).
lib/curl_config-cmake.h.in Removes the HAVE_SNPRINTF config header define template entry.
configure.ac Stops checking for snprintf() in non-Windows function checks.
CMakeLists.txt Stops checking for snprintf() in non-Windows function checks.
CMake/unix-cache.cmake Removes cached HAVE_SNPRINTF value.

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

Comment thread configure.ac
Comment thread CMakeLists.txt
@vszakats vszakats changed the title build: assume snprintf() in mprintf, drop feature checks build: assume snprintf() in mprintf, drop feature check Mar 4, 2026
- it is required for `curl_*printf()` float/double support.
- some curl tests always fail without it.
- it was already assumed to be present to build test servers,
  as the code did not check for `HAVE_SNPRINTF` detection variable.
- it was already required to build examples.

VS2013 and older are known to miss this function, and a fallback is
already used.
@vszakats vszakats closed this in a8bc4cb Mar 21, 2026
@vszakats vszakats deleted the reqsnprintf branch March 21, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build feature-window A merge of this requires an open feature window

Development

Successfully merging this pull request may close these issues.

3 participants