NotificationsYou must be signed in to change notification settings
Fork33.3k
Star69.7k

gh-130567: Remove optimistic allocation in locale.strxfrm()#137143

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

encukou merged 3 commits intopython:mainfromserhiy-storchaka:locale-wcsxfrm

Oct 16, 2025

Merged

gh-130567: Remove optimistic allocation in locale.strxfrm()#137143

encukou merged 3 commits intopython:mainfromserhiy-storchaka:locale-wcsxfrm

Oct 16, 2025

Conversation

Copy link

Member

serhiy-storchaka commentedJul 27, 2025•
edited by bedevere-appbot
Loading

On modern systems, the result of wcsxfrm() is much larger the size of the input string (from 4+2n on Windows to 4+5n on Linux for simple ASCII strings), so optimistic allocation of the buffer of the same size never works.

Issue:strxfrm fails on macOS 15 with OSError: [Errno 22] Invalid argument #130567

Remove optimistic allocation in locale.strxfrm()

77f5515

On modern systems, the result of wcsxfrm() is much larger the size ofthe input string (from 4+2*n on Windows to 4+5*n on Linux for simpleASCII strings), so optimistic allocation of the buffer of the same sizenever works.

serhiy-storchaka requested a review fromronaldoussoren

July 27, 2025 10:22

bedevere-appbot added the awaiting core review label

Jul 27, 2025

serhiy-storchaka mentioned this pull request

Jul 27, 2025

strxfrm fails on macOS 15 with OSError: [Errno 22] Invalid argument#130567

Closed

nijel reviewed

Jul 27, 2025

View reviewed changes

Modules/_localemodule.c Outdated

		gotoexit;
		}

		/* assume no change in size, first */

Copy link

Contributor

nijelJul 27, 2025

Choose a reason for hiding this comment

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

The comment should be updated to match changed code.

picnixz changed the title~~Remove optimistic allocation in locale.strxfrm()~~gh-130567: Remove optimistic allocation in locale.strxfrm()

Jul 27, 2025

Remove outdated comment.

fbf6e8e

Copy link

MemberAuthor

serhiy-storchaka commentedJul 28, 2025

If this is a bug fix, it needs a NEWS entry. If the bug will be fixed in other way -- it is just cleanup and minor optimization not worth a NEWS entry.

encukou approved these changes

Sep 2, 2025

View reviewed changes

Copy link

Member

encukou left a comment

Choose a reason for hiding this comment

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

This does not fix the bug; macOS raisesEINVAL inwcsxfrm(NULL, s, 0) on the Czech and Chinese strings.

So, it's just cleanup and minor optimization.

bedevere-appbot added awaiting merge and removed awaiting core review labels

Sep 2, 2025

Copy link

MemberAuthor

serhiy-storchaka commentedSep 2, 2025

Actually, optimistic allocation works if the locale was not set or set to "C".

>>> import locale>>> locale.strxfrm('abc')'abc'>>> locale.setlocale(locale.LC_ALL, 'C')'C'>>> locale.strxfrm('abc')'abc'>>> locale.setlocale(locale.LC_ALL, 'en_US.UTF-8')'en_US.UTF-8'>>> locale.strxfrm('abc')'šŮŹ\x01\x1d\x1d\x1d\x01\x02\x02\x02\x01\x01悝\x01惹\x01愝'

But why would you uselocale.strxfrm() in the C locale? And since the call ofwcsxfrm() is cheap in the C locale, I believe that the loss in the worst case is less that the gain in average.