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

Fix Windows dialog folder titles #12018

Merged
merged 1 commit into from
Jan 19, 2025

Conversation

Semphriss
Copy link
Contributor

Description

Fix titles being copied without carrying the terminating NULL byte.

Same fix as in cf946e3, which was not done for the folder implementation.

Existing Issue(s)

Fixes #12000 (comment)

Same fix as in cf946e3, which was not done for the folder implementation.
@slouken
Copy link
Collaborator

slouken commented Jan 18, 2025

Can you review all the string handling in both subsystems?

@slouken
Copy link
Collaborator

slouken commented Jan 18, 2025

Also, is there a reason you're not using WIN_UTF8ToStringW()?

@Semphriss
Copy link
Contributor Author

Also, is there a reason you're not using WIN_UTF8ToStringW()?

There's one place (the filters) where it's easier to use MultiByteToWideChar, because the string that must be converted contains null bytes. For this place in particular, it's just because I didn't know about that function when I first wrote the code, and in bugfixing PRs I often go for the most straightforward fix. I'll change it to use WIN_UTF8ToStringW.

I'm sorry for the multitude of basic mistakes, I've been distracted by personal stuff lately. I'll try to review the string handling in the upcoming days, but I'll soon be unavailable for a while, so I would be more comfortable if someone else could cover for trays and dialogs in the meantime. If you wish, we can talk about it privately.

@slouken
Copy link
Collaborator

slouken commented Jan 19, 2025

I'll take over on the tray and dialog work. Good luck with your personal stuff!

@slouken slouken merged commit 43b54b3 into libsdl-org:main Jan 19, 2025
40 checks passed
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