Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

printf: improve support of printing multi-byte values of characters #7208

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jtracey
Copy link
Contributor

@jtracey jtracey commented Jan 24, 2025

Based on #7048.

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@jtracey jtracey marked this pull request as draft January 25, 2025 00:37
@jtracey jtracey marked this pull request as ready for review January 25, 2025 03:38
@jtracey
Copy link
Contributor Author

jtracey commented Jan 25, 2025

I was worried because, unlike #7048, this doesn't get the printf-mb GNU test to pass, but on closer investigation, the test was only passing due to a coincidental bug that made it pass, despite failing in the general case.

The problem is the same described here: #7048 (comment)
To see it in practice, run printf '%s' $(printf '\x61\xC3\xA1\xC0') and cargo run printf '%s' $(printf '\x61\xC3\xA1\xC0'), where non-cargo printf is GNU's, and the repo is checked out at 8f4c286. You'll get aá� and aáÀ, respectively. Proper multibyte support won't work until #6812 (or a successor) gets merged.

@jtracey
Copy link
Contributor Author

jtracey commented Jan 25, 2025

To be clear for posterity, the version in this PR will look correct and show aá�, but GNU's is actually passing the 0xC0 byte to the terminal, which is in turn rendering it with �. It's easier to see the remaining problem, the one we need #6812 to solve, if you redirect the output to a hex editor or the like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants