-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fallback to language id 0 with FormatMessageA
if the error messages are not available in system locale
#3260
Conversation
… are not available for system locale
… are not available for system locale
… are not available for system locale
With your fix, the issue #2451 is back again: #include <clocale>
#include <windows.h>
#include <system_error>
#include <stdio.h>
int main()
{
setlocale(LC_ALL, "");
std::error_code ec(ERROR_DIR_NOT_EMPTY, std::system_category());
auto s = ec.message();
printf("message = '%s' (%lld)\n", s.c_str(), static_cast<long long>(_MSC_FULL_VER));
for (auto c: s) {
printf("%c = %d\n", c, (int)c);
}
}
|
Here's an idea: why not use only US English? There is certainly a precedent for this, for example all STL-thrown exceptions have a English message in My comment earlier mentions it failing without the US English but this isn't quite true. After more investigation, MUI (multilanguage) installs should always have US English available, but non-MUI installs (e.g. Windows 10 Single Language) do not have US English, in which case asking for the US English error message actually returns the localized message without error (I misremembered). But this won't be an issue, as the only valid codepage would be the locale matching said message anyways. Alternatively, if this is not a valid suggestion, we should go with @fsb4000's suggestion of always falling back to US English instead of lang ID 0, to avoid regressing #2451. |
Whichever solution we pick, the fix needs to be checked on a variety of configs (single language installs, MUI installs with only non-English language available, mixed user/system locale (both ways), etc.) |
@fsb4000 I am aware that this will break for users who does not have the messages in the system locale language and have display language different from system language, they are already getting "unknown error" because the current behaviour only uses system locale. By the way the suggestion to try en-US if system locale fails seems good to go with |
Other options I thought of:
|
This comment was marked as outdated.
This comment was marked as outdated.
I appreciate it's probably not part of the standard, but would providing an alternate wide string version alleviate the problems of non-English (US) texts? |
If you're a Windows application, you can easily do this out of the standard (pseudo code): std::wstring error_message(const std::error_code &err)
{
if (err.category() == std::system_category())
{
return FormatMessage(err.value()));
}
else if (err.category() == std::generic_category())
{
return _wcserror_s(err.value());
}
else
{
return L"Unknown error";
}
} |
I already have my own implementation, but everything I don't have to write myself is usually a better solution (though not in this case until this gets resolved) :) |
I tried to implement the suggestion from @davemcincork in #3254 (comment). diff --git a/stl/src/syserror_import_lib.cpp b/stl/src/syserror_import_lib.cpp
index e2f2d2b7..a70424ba 100644
--- a/stl/src/syserror_import_lib.cpp
+++ b/stl/src/syserror_import_lib.cpp
@@ -46,11 +46,48 @@ extern "C" {
if (_Ret == 0) {
_Lang_id = 0;
}
- const unsigned long _Chars =
+ const unsigned long _Chars_primary_path =
FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr, _Message_id, _Lang_id, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
- return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars);
+ const WORD _Lang_id_primary = PRIMARYLANGID(_Lang_id);
+ if (_Chars_primary_path != 0 || _Lang_id_primary == LANG_NEUTRAL) {
+ return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars_primary_path);
+ }
+
+ const WORD _Lang_id_sub = SUBLANGID(_Lang_id);
+ if (_Lang_id_sub != SUBLANG_NEUTRAL && _Lang_id_sub != SUBLANG_DEFAULT) {
+ const DWORD _Adjusted_lang_id = MAKELANGID(_Lang_id_primary, SUBLANG_DEFAULT);
+ const unsigned long _Chars_sub_default_path =
+ FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, _Message_id, _Adjusted_lang_id, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
+ if (_Chars_sub_default_path != 0) {
+ return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars_sub_default_path);
+ }
+ }
+
+ if (_Lang_id_sub != SUBLANG_NEUTRAL) {
+ const DWORD _Adjusted_lang_id = MAKELANGID(_Lang_id_primary, SUBLANG_NEUTRAL);
+ const unsigned long _Chars_sub_neutral_path =
+ FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, _Message_id, _Adjusted_lang_id, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
+ if (_Chars_sub_neutral_path != 0) {
+ return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars_sub_neutral_path);
+ }
+ }
+
+ constexpr DWORD _En_us_lang_id = MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US);
+ const unsigned long _Chars_en_us_path =
+ FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, _Message_id, _En_us_lang_id, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
+ if (_Chars_en_us_path != 0) {
+ return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars_en_us_path);
+ }
+
+ const unsigned long _Chars_neutral_path =
+ FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, _Message_id, 0, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
+ return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars_neutral_path);
}
void __CLRCALL_PURE_OR_STDCALL __std_system_error_deallocate_message(char* const _Str) noexcept { Does this work? @cppdev123 @fsb4000 @sylveon |
I can't be certain of the following because you've just provided a diff which is a little difficult to visualise in context... I think your proposal will leak. As in my suggestion, you're calling Just curious: why did you move away from the loop-based approach that I used? |
I think when
I think it would be clearer to expand the loop to exactly see which lang-ids are tried. |
I was referring to what happens in the case where the API call succeeds. Keep in mind that because you're just showing a diff here, it is difficult for me to see the full context of this change, so perhaps the allocation will be freed in the calling function.
That's probably subjective so fair enough. |
I believe that freeing should be done by the call site with the |
Yes, after I posted my last comment above, I looked at the entire source file so I could see your proposed changed in context, and I saw the |
It does seem like the "last chance" call to FormatMessageA() should use a LANGID of 0, and thus benefit from the built-in hunting algorithm of that function (which tries "en-US" as a last resort anyway, according to its documentation). The "Files Changed" changes that in this review appear to restore the behaviour from VS 17.2.x (i.e. before #2669) when the system language doesn't have strings installed, and that would be a great help, and couldn't come soon enough! |
Thanks for reporting this bug and creating this PR. I'm sorry it took me almost 2 years to investigate this! After reading everyone's comments, I believe that we should follow a US English-only strategy here. I've created #5104 for that, and my initial testing indicates that it fixes #3254 and keeps the earlier #2451 fixed. Accordingly I'm going to close your PR without merging, which always makes me sad 😿, but I truly appreciate the work you did here 😻. |
After #2669,
std::system_category
used the locale system language withFormatMessageA
to get the error message which fixed a problem and introduced another one. As reported in #3254 the error messages may not be available in the system locale, but they are available in en-US locale shipped with windows soFormatMessageA
failed withERROR_MUI_FILE_NOT_FOUND
andstd::system_category
returned hard coded "unknown error". It was working before because if the language id is 0 it will eventually fall back to en-US locale and return an error message in that locale.I reproduced the problem on following windows installations:
FormatMessageA
failed withERROR_MUI_FILE_NOT_FOUND
. Seems that there are no error messages in the Arabic mui because windows apps, explorer and login screen can show properly in arabic and right to left layoutThis should fix #3254