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 #11802: Compile bug - RegQueryValueExA changed to RegQueryValueEx #11803

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

sheldonrobinson
Copy link
Contributor

Make sure to read the contributing guidelines before submitting a PR

fix #11802

@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Feb 11, 2025
@slaren
Copy link
Collaborator

slaren commented Feb 11, 2025

Won't this cause the value to be read as a wchar_t as well? I believe the correct solution here would be to continue using RegQueryValueA, but remove the use of the TEXT macro to pass the key name.

@sheldonrobinson
Copy link
Contributor Author

The TEXT is also handled. If UNICODE, TEXT is L##, other TEXT is ANSI string. See TEXT macro (winnt.h)

@slaren
Copy link
Collaborator

slaren commented Feb 11, 2025

RegQueryValueExA always takes an ANSI string (that's what the A suffix means), so there is no need to use the TEXT macro here.

@sheldonrobinson
Copy link
Contributor Author

@sheldonrobinson
Copy link
Contributor Author

It should be RegQueryValueEx and TEXT.

@slaren
Copy link
Collaborator

slaren commented Feb 11, 2025

This is getting tiring. We cannot use the UNICODE version of this function because the string read is expected to be in ANSI format. For this reason we need to use RegQueryValueExA always, because RegQueryValueExW will return an UNICODE string. However, because we are using RegQueryValueExA, the TEXT macro is not necessary here.

@sheldonrobinson
Copy link
Contributor Author

Ok, agreed. Keep RegQueryValueExA, remove TEXT

Copy link
Collaborator

@slaren slaren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@slaren slaren merged commit 90e4dba into ggerganov:master Feb 11, 2025
46 checks passed
tinglou pushed a commit to tinglou/llama.cpp that referenced this pull request Feb 13, 2025
…ryValueEx (ggerganov#11803)

* Fix ggerganov#11802: Compile bug - RegQueryValueExA changed to RegQueryValueEx

* Fix ggerganov#11802: PR ggerganov#11803 - keep RegQueryValueExA, remove TEXT macro, description needs to be ANSI string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ggml changes relating to the ggml tensor library for machine learning
Projects
None yet
2 participants