On Wed, 12 Aug 2026 14:08:40 GMT, Matthias Baesken <[email protected]> wrote:

>> There is a RegOpenKeyExA call present, but we do not close the handle. This 
>> should be changed.
>> The MS docu says 
>> https://learn.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regopenkeyexa
>> 'A pointer to a variable that receives a handle to the opened key. If the 
>> key is not one of the predefined registry keys, call the 
>> [RegCloseKey](https://learn.microsoft.com/en-us/windows/desktop/api/winreg/nf-winreg-regclosekey)
>>  function after you have finished using the handle.'
>> And in this coding we call
>> `LONG lRet = ::RegOpenKeyExA(hRootKey, lpszSubKey, 0, KEY_ALL_ACCESS, &hKey);
>> `
>> with a non_NULL `lpszSubKey `so the RegCloseKey has to be done.
>> 
>> Also fixed a typo while at it.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> @macarte afaik you are from Microsoft, is this true? Can you or someone else 
> from MS comment on the RegOpenKeyExA  / RegCloseKey topic ?

@MBaesken - yes, if the subKey is non-null and not empty then you have to close 
the key to free the handle.  GetCodePageSubkey does appear to return only NULL 
or not empty, perhaps maybe add an assert to make sure and document that we 
should not return empty strings here; otherwise if it is non-null but empty, 
then it is indeed a shared key and should not be closed.

Two points:

1) As we're only reading the value, consider using KEY_QUERY_VALUE or KEY_READ 
(stricter) instead of KEY_ALL_ACCESS; see: 
https://learn.microsoft.com/en-us/windows/win32/sysinfo/registry-key-security-and-access-rights

2) While you are potentially reading two values, using RegGetValueA (avoiding 
explicit open/close calls) could simplify the code and remove the chance of 
handle leaks with future code changes.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/32249#issuecomment-5270081065

Reply via email to