MPD
MPD copied to clipboard
system/Error: manually prepare localized error code messages through error category method on windows
FormatMessageA in VFmtLastError and std::system_category::message
produce non-utf8 multi-byte string, which causes
std::runtime_error("invalid utf8") thrown in
fmt::detail::utf8_to_utf16::utf8_to_utf16.
Use FormatMessageW with a custom category to override message method
for utf-8 error message.
Remove FormatMessageA in VFmtLastError to remove duplicated error message from error code.
fix #2302
We still don't know where the exception came from and your commit message doesn't explain anything. Please add more text to the commit message explaning the whole problem and how your change solves it.
Also note the GH Actions failure. The exception message looks suspicious (two trailing newlines?)
This PR does at least two distinct things in one commit:
- switch from FormatMessageA() to FormatMessageW()
- move the FormatMessage call to a new error category
Mixing both in one commit is confusing. The actual bug is only fixed by (1) and (2) has nothing to do with this. The reasons for (2) are not explained anywhere, and the implementation isn't good because it adds massive header dependencies, because you decided to have all of the code in the header for no apparent reason.
For (1) there should be a code comment explaining why you're going through all these hoops. This is all but obvious.
It would also be interesting to know what kind of illegal UTF-8 is generated by FormatMessageA(). I was thinking that FormatMessageA() generates just English ASCII, doesn't it?
This PR does at least two distinct things in one commit:
1. switch from FormatMessageA() to FormatMessageW() 2. move the FormatMessage call to a new error categoryMixing both in one commit is confusing. The actual bug is only fixed by (1) and (2) has nothing to do with this. The reasons for (2) are not explained anywhere, and the implementation isn't good because it adds massive header dependencies, because you decided to have all of the code in the header for no apparent reason.
The reason that we need two steps to avoid non-utf8 strings is that VFmtLastError appends non-utf8 message, and std::system_category appends same non-utf8 message again.
I'll split the commit into two.
For (1) there should be a code comment explaining why you're going through all these hoops. This is all but obvious.
It would also be interesting to know what kind of illegal UTF-8 is generated by FormatMessageA(). I was thinking that FormatMessageA() generates just English ASCII, doesn't it?
All A-ending function encode multi-byte string in local codepage, which is CP_ACP in WideCharToMultiByte
For example, typical windows machine in japan uses codepage-932 aka shift_jis, and the error message will be
"\x8e\x77\x92\xe8\x82\xb3\x82\xea\x82\xbd\x83\x74\x83\x40\x83\x43\x83\x8b\x82\xaa\x8c\xa9\x82\xc2\x82\xa9\x82\xe8\x\82\xdc\x82\xb9\x82\xf1\x81\x42\x0d\x0a" which includes two new line characters at end.
Use FORMAT_MESSAGE_MAX_WIDTH_MASK flags will replace that newline character into space character.
So it looks like the whole complicated LocaleSafeSystemCategory stuff is not necessary at all, and all we need to do is remove the FormatMessageA() call from VFmtLastError(). This PR does that, but in a very complicated way. I pushed a simpler version to v0.24.x: cd2a58d607e9837477910d8cec0d12ee2db1ad8d