-
Notifications
You must be signed in to change notification settings - Fork 77
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
Conversation
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? |
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. |
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 |
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)? |
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 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. |
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. |
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.