Skip to content

Fix empty strerror text in error messages on musl#111620

Open
thevar1able wants to merge 3 commits into
masterfrom
fix-errno-to-string-musl
Open

Fix empty strerror text in error messages on musl#111620
thevar1able wants to merge 3 commits into
masterfrom
fix-errno-to-string-musl

Conversation

@thevar1able

@thevar1able thevar1able commented Jul 23, 2026

Copy link
Copy Markdown
Member

Fix empty strerror text in error messages on musl.

errnoToString selected the GNU variant of strerror_r whenever _GNU_SOURCE was defined and formatted its return value as the message. musl provides only the POSIX variant (returning int) even when _GNU_SOURCE is defined, so every error message rendered the return code instead of the text: Cannot open file ...: , errno: 2, strerror: 0.

Dispatch on the actual return type of strerror_r via overloads instead of feature-test macros (the same trick as Poco::Error). Verified on a static musl binary (No such file or directory) and on glibc (unchanged, including the Unknown error N fallback).

This failed test_dictionaries_update_and_reload in the amd_llvm_coverage integration jobs, which greps for No such file.

CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109239&sha=8ed85c44cd7e4afbef1f826ab59b26d3e540828e&name_0=PR

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed error messages on musl-based builds rendering an empty/numeric strerror value instead of the human-readable text (e.g. strerror: 0 instead of No such file or directory), caused by errnoToString mis-detecting the strerror_r variant.

thevar1able and others added 2 commits July 23, 2026 16:29
`errnoToString` selected the GNU variant of `strerror_r` whenever
_GNU_SOURCE was defined, formatting its return value as the message.
musl provides only the POSIX variant (returning `int`) even when
_GNU_SOURCE is defined, so every error message rendered the return
code instead of the text: `Cannot open file ...: , errno: 2,
strerror: 0`. This failed `test_dictionaries_update_and_reload` in the
`amd_llvm_coverage` integration jobs, which greps for `No such file`.

Dispatch on the actual return type of `strerror_r` via overloads
instead of feature-test macros, the same trick as `Poco::Error`.
Verified on a static musl binary ("No such file or directory") and on
glibc (unchanged, including the "Unknown error N" fallback).

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109239&sha=8ed85c44cd7e4afbef1f826ab59b26d3e540828e&name_0=PR

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [95dea82]


AI Review

Summary

This PR changes errnoToString to dispatch on the actual return type of strerror_r instead of _GNU_SOURCE, which fixes musl's POSIX-style implementation while preserving the existing glibc behavior. I did not find a correctness issue in the implementation or a missing regression path in the existing tests; the only issue I found is PR metadata, because this is a user-visible fix for musl/alpine users rather than an internal-only cleanup.

PR Metadata
  • Changelog category is mismatched. Not for changelog understates this change: it fixes user-visible error text on supported musl/alpine builds. Replace it with Bug Fix (user-visible misbehavior in an official stable release).
  • With Bug Fix, a changelog entry is required. The current entry is specific enough to keep.
Final Verdict
  • Status: ✅ Approve
  • Minimum follow-up: update the Changelog category to Bug Fix.

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jul 23, 2026
@thevar1able
thevar1able marked this pull request as ready for review July 23, 2026 14:33
@thevar1able thevar1able added pr-not-for-changelog This PR should not be mentioned in the changelog and removed pr-bugfix Pull request with bugfix, not backported by default labels Jul 23, 2026
@evillique evillique self-assigned this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants