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

Do support non-latin character sets with windows error text and do not limit in size. #229

Closed
wants to merge 1 commit into from

Conversation

aholzinger
Copy link

FormatMessageA does not work with a lot of languages.
As the function returns utf8 anyhow it's better to use FormatMessageW.
The new implementation also makes use of the abbility of FormatMessage to allocate the needed buffer. Like this the messages are not limited in size anymore.

@aholzinger
Copy link
Author

The failing GitHub actions belong to fetching the ASIO SDK:
-- verifying file...
file='D:/a/FlexASIO/FlexASIO/src/out/build/x86-Debug/dechamps_ASIOUtil-prefix/src/dechamps_ASIOUtil-build/_deps/asiosdk-subbuild/asiosdk-populate-prefix/src/asiosdk_2.3.3_2019-06-14.zip'
-- SHA1 hash of
D:/a/FlexASIO/FlexASIO/src/out/build/x86-Debug/dechamps_ASIOUtil-prefix/src/dechamps_ASIOUtil-build/_deps/asiosdk-subbuild/asiosdk-populate-prefix/src/asiosdk_2.3.3_2019-06-14.zip
does not match expected value
expected: '33c44f0c919a7218413ea81e06a66bb8b47be292'
actual: '4d0097725bcf1015c91fb84f89e1a888141bd131'
-- Hash mismatch, removing...
CMake Error at asiosdk-subbuild/asiosdk-populate-prefix/src/asiosdk-populate-stamp/download-asiosdk-populate.cmake:162 (message):
Each download failed!
In my local repo I used '33c44f0c919a7218413ea81e06a66bb8b47be292' instead of '4d0097725bcf1015c91fb84f89e1a888141bd131' then it works.
You might want to update this.

@dechamps
Copy link
Owner

The failing GitHub actions belong to fetching the ASIO SDK

Yeah, sorry about that, I know about this issue.

Thanks for the contribution by the way - in FlexASIO's 6-year history, you are literally the first person to open a PR against FlexASIO with actual code changes 😅

I don't have major objections to this change at first glance. I'll take a closer look and merge it as soon as I find the time to resume work on FlexASIO (which may not be any time soon, to be perfectly honest).

Out of curiosity, how did you come across this issue? Were you trying to read the log on a non-latin Windows install?

@aholzinger
Copy link
Author

Never mind.

There's always a first time :-)

I just studied the code, beginning with the headers and was curious about the error text function delivering UTF-8 and wanted to know if it would use FormatMessageW or FormatMessageA. IMHO it's a pitty that MS didn't decide to add U functions that use UTF-8. I think it would be high time for this, because nearly all portable libraries use UTF-8. It would make life much easier on Windows.

Take your time, there's no hurry.

@dechamps dechamps added this to the FlexASIO 1.10 milestone May 26, 2024
@dechamps
Copy link
Owner

I took a closer look at this and realized that actually, this function is not really needed, as the standard C++ library already knows how to produce these messages through std::system_error. Most FlexASIO code is already using that, and I cleaned up the remaining callsites in 71aab1a, culminating in the removal of GetWindowsErrorString(). This makes your PR moot I'm afraid, but thanks anyway for the contribution!

@dechamps dechamps closed this May 26, 2024
@aholzinger
Copy link
Author

Even better like this.

Did you check that it's behaving correctly on non-latin systems or at least non-US ones (German umlauts or French cedille for example)?

@dechamps
Copy link
Owner

No - I don't have such systems at hand. That being said, if it doesn't work, at this point it would be a bug in the MSVC standard library (i.e. their implementation of std::system_error), so it seems like something that should be reported and fixed there.

Quite frankly I am reluctant to add extra complexity to the code for something that users are never supposed to see, anyway: Windows system errors are rare in FlexASIO, and even when they do occur they are usually only visible in the log, which most users wouldn't look at anyway. I'd be open to revisit if I see reports from users where system error messages are not making it through properly.

@aholzinger
Copy link
Author

Fair enough, but I doubt that std::system_error::what() will produce UTF8 on Windows. I think it will produce ANSI and thus fail on non-Latin systems. And it seems (I only studied cppreference.com) that the standard is not saying that the string will be UTF8. So IMO the result will be the same as FormatMessageA. It's the same situation as with std::exception::what, isn't it.

Unfortunately Microsoft never adopted the UTF8 concept.

But I don't want to be nit picking. I'm fine with std::system_error::what. My approach with my PR was if using FormatMessage, then doing it right and use FormatMessageW.

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