Fix for deprecated RegEnumKey API usage#8751
Conversation
| return std::nullopt; | ||
| } | ||
|
|
||
| if (max_name_length > 255) { |
There was a problem hiding this comment.
Yes, this is incorrect. Should be 16,383 as max value name limit not key name.
There was a problem hiding this comment.
I think we are probably ready to proceed with this PR once this is updated. Thanks!
There was a problem hiding this comment.
I attempted a fix. I pushed it into this PR
bc65102 to
4baba24
Compare
zwass
left a comment
There was a problem hiding this comment.
Generally looking good, but let's at least fix the close.
| auto ret = RegOpenKeyExW( | ||
| HKEY_LOCAL_MACHINE, kRegProfileKey.c_str(), 0, KEY_READ, &hkey); |
There was a problem hiding this comment.
This needs a matching RegCloseKey call I think (scope_guard ideally).
| // Note: max_subkey_length is at position 6 (lpcbMaxSubKeyLen). | ||
| // Position 9 would be lpcbMaxValueNameLen which has a different limit. |
There was a problem hiding this comment.
This seems confusing as a code comment since it lacks the old context. Seems more like a PR comment, explaining why the change from the old code.
There was a problem hiding this comment.
The value here hasn't changed purpose. It was and is for storing the length of the subkey name string. Maybe the variable name should reflect that. And other variables named "key_" ought to be "subkey_" as well then.
| } | ||
|
|
||
| subkeys_names.emplace_back(wstringToString(key_name)); | ||
| subkeys_names.emplace_back(wstringToString(key_name.c_str())); |
There was a problem hiding this comment.
The wStringToString takes a std::wstring reference here, so the .c_str() seems incorrect.
|
|
||
| std::wstring key_name; | ||
| key_name.resize(max_key_length); | ||
| key_name.resize(max_subkey_length + 1); |
There was a problem hiding this comment.
Good catch 😀 the null terminator wasn't being accounted for. Note: In cases where the subkey name was of the max length, the API call would have previously errored with ERROR_MORE_DATA.
| // Note: max_subkey_length is at position 6 (lpcbMaxSubKeyLen). | ||
| // Position 9 would be lpcbMaxValueNameLen which has a different limit. |
There was a problem hiding this comment.
The value here hasn't changed purpose. It was and is for storing the length of the subkey name string. Maybe the variable name should reflect that. And other variables named "key_" ought to be "subkey_" as well then.
The getRoamingProfileSids method uses the deprecated Windows registry API RegEnumKey.
This PR updates the API to RegEnumKeyEx without changing any other logic or functionality.
The change has been tested and debugged locally, and no issues found,