jrfonseca / drmingw

Postmortem debugging tools for MinGW.
GNU Lesser General Public License v2.1
273 stars 53 forks source link

Support minidump locations/filenames containing unicode (wide char) characters #74

Closed Robyt3 closed 1 year ago

Robyt3 commented 1 year ago

The function ExcHndlSetLogFileNameA only supports ANSI paths. A separate companion function ExcHndlSetLogFileNameW would be necessary to correctly handle paths containing unicode characters (WCHAR in the Windows API). Otherwise minidumps cannot be written to folders/files containing unicode characters, which is a common occurrence for users in non-US/UK regions.

alvinhochun commented 1 year ago

If your application can afford it, you may also consider switching the process ANSI code page to UTF-8 using the application manifest: https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#set-a-process-code-page-to-utf-8

(Only applies since Windows version 1903.)

jrfonseca commented 1 year ago

Yeah, use of ANSI instead of Unicode has been a long standing tech debt...

It's not just ExcHndlSetLogFileNameW, but probably quite a few more internal functions that need changing.

I don't have much time to look into this, but I'd happily accept any PRs.

Robyt3 commented 1 year ago

I can make a PR to fix unicode handling in ExcHndl, including the case that the module file name contains unicode (e.g. if you rename the sample program to "sämple" it will no longer work).

What would be the appropriate prefix for wide char string variable names?

Should I also extend the tests for ExcHndlSetLogFileNameW? If so, should I further split them into test_exchndl_dynamic_ansi/test_exchndl_dynamic_unicode and test_exchndl_static_ansi/test_exchndl_static_unicode, or should test_exchndl_dynamic and test_exchndl_static each test both the ANSI and Unicode functions at the same time?

jrfonseca commented 1 year ago

Thanks.

I don't think it's common to prefix wide strings with w. I suspect the expectation is that everybody should be using wide char exclusively by now. See https://learn.microsoft.com/en-us/windows/win32/stg/coding-style-conventions

Yes, it would be good to exercise ExcHndlSetLogFileNameW. We don't necessarily need two test executables. For example, we could use one test executable, and then choose ExcHndlSetLogFileName(A|W) via a command line option. Whatever is easiest is fine by me.