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

Correctly handle Unicode file names in ExcHndl, add tests #75

Merged
merged 2 commits into from
Oct 3, 2022

Conversation

Robyt3
Copy link
Contributor

@Robyt3 Robyt3 commented Oct 1, 2022

See commit messages below.

Closes #74.

Robyt3 added 2 commits October 1, 2022 18:21
Add `ExcHndlSetLogFileNameW` that accepts a Unicode wide character string as an alternative to `ExcHndlSetLogFileNameA`, which uses the current Windows code page.
Internally only the wide character string is stored. The file name passed to `ExcHndlSetLogFileNameA` is converted to wide characters using the current code page.

Use `GetModuleFileNameW` instead of `GetModuleFileNameA` to handle the case that the module file name contains Unicode characters.

Don't use `%s` in format strings to print the argument when it's a nullptr, as this is undefined behavior.
As a lot of testing code also had to be adjusted for usage of wide character strings, it was easiest to split the existing test code by adding another compile time flag `TEST_UNICODE`.
The flag is not named `UNICODE`, as this name is already used by the Windows API.

A macro `WFILE` is added that expands to a wide character string literal containing `__FILE__`.

The output file name for the unicode tests contains additional unicode characters to properly test the behavior.
@codecov-commenter
Copy link

Codecov Report

Merging #75 (f5a7978) into master (52c36c7) will decrease coverage by 2.18%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
- Coverage   50.66%   48.48%   -2.19%     
==========================================
  Files          15       15              
  Lines        1954     1941      -13     
  Branches      742      740       -2     
==========================================
- Hits          990      941      -49     
- Misses        743      770      +27     
- Partials      221      230       +9     
Impacted Files Coverage Δ
src/exchndl/exchndl.cpp 72.44% <58.33%> (-2.14%) ⬇️
src/common/log.cpp 58.00% <0.00%> (-6.04%) ⬇️
src/mgwhelp/mgwhelp.cpp 61.01% <0.00%> (-3.52%) ⬇️
src/common/debugger.cpp 42.94% <0.00%> (-2.85%) ⬇️
src/mgwhelp/dwarf_pe.cpp 83.33% <0.00%> (-1.63%) ⬇️
src/catchsegv/catchsegv.cpp 44.97% <0.00%> (-0.92%) ⬇️
src/mgwhelp/dwarf_find.cpp 48.17% <0.00%> (-0.61%) ⬇️
src/common/symbols.cpp 73.91% <0.00%> (-0.56%) ⬇️
src/addr2line/addr2line.cpp 50.98% <0.00%> (-0.48%) ⬇️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jrfonseca
Copy link
Owner

Looks great. Thanks for doing this!

@jrfonseca jrfonseca merged commit 5fdc641 into jrfonseca:master Oct 3, 2022
@Robyt3 Robyt3 deleted the ExcHndl-Unicode branch October 3, 2022 08:30
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.

Support minidump locations/filenames containing unicode (wide char) characters
3 participants