`_Locinfo`: Use `_wsetlocale` to query and restore locales

[lb90]

_Locinfo changes the locale temporarily and then reverts to the previous locale on destruction. The sequence of setlocale calls are as follows:

1. oldlocname = setlocale(LC_ALL, nullptr) to query the locale string
2. setlocale(LC_ALL, newlocname) to set the temporary locale
3. setlocale(LC_ALL, oldlocname) to restore the previous locale

However there's a catch here: the fully-qualified locale names returned by setlocale are not always ASCII strings (more on that below). This creates challenges because the oldlocname is encoded depending on the "outer" locale, while the setlocale call at point 3) expects an encoding which follows the "inner" locale, and the two may not match.

To solve this issue, use the wide variant of setlocale: _wsetlocale. This way all strings are UTF-16 and there's no issue with varying narrow string encodings.

Addendum:

Actually, the C RunTime library does its best to use ASCII strings! It queries the english name of the locale using GetLocaleInfoEx. MSDN says that the returned string is always ASCII [1], but that's not always the case [2].

Fixes #5780

References:

1. https://learn.microsoft.com/en-us/windows/win32/intl/locale-senglish-constants
2. https://developercommunity.visualstudio.com/t/GetLocaleInfoEx-w-LOCALE_SENGLISHLANGUA/10981789

I made a similar change in libc++: https://github.com/llvm/llvm-project/pull/160479

[StephanTLavavej]

Also, what about _Yarn<char> _Newlocname? It appears to get its content from setlocale. That one is trickier because it is exposed to the headers via _Getname().

[muellerj2]

Also, what about _Yarn _Newlocname? It appears to get its content from setlocale. That one is trickier because it is exposed to the headers via _Getname().

Its value is effectively returned by locale:name(), so there might be some users out there relying on its encoding.

[frederick-vs-ja]

Is it possible to add test cases?

[lb90]

Is it possible to add test cases?

Sure :)

P.S: will get back to this within 1 or 2 days...

[StephanTLavavej]

By the way, after code review has started, we recommend avoiding force-pushes, as that makes it harder to see what's changed. Not a big deal, just wanted to mention it for the future.

[lb90]

Hi! Should I rebase to solve the conflict?

[AlexGuteniev]

Hi! Should I rebase to solve the conflict?

You should merge instead of rebasing. This is not mandatory; maintainers would be able to do this for you too (be sure to allow maintainers making changes).

[StephanTLavavej]

If it's just a test.lst conflict I agree that there's no need to push a merge and rerun the checks, I can do that when I review the PR again. (I will try to get to this soon since I've been working through the PR backlog and this is now the second-oldest.) When there are more significant conflicts, I do appreciate when contributors resolve the conflicts, locally rerun affected tests, and push normally.

[StephanTLavavej]

My bosses want me to focus on <flat_map> and <flat_set> so my review may be delayed.

[lb90]

No problem at all! 😊

[mikekaganski]

Does this fix https://developercommunity.visualstudio.com/t/setlocale-may-crash-CRT-in-some-cases/10603395 and https://github.com/Exiv2/exiv2/issues/3225 ?

[lb90]

Hello @mikekaganski! https://github.com/Exiv2/exiv2/issues/3225 is fixed by the LLVM/LibC++ PR linked above. On the other hand, issue https://developercommunity.visualstudio.com/t/setlocale-may-crash-CRT-in-some-cases/10603395 must be fixed in the UCRT. The UCRT is source-available but not open source, so can only be fixed by MS developers :)