Signed-off-by: Serge Gautherie winehq-git_serge_180711@gautherie.fr --- dlls/kernelbase/registry.c | 26 ++++++++++++++++++++++++-- include/winreg.h | 4 +++- 2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c index 137c0ef..b4f540c 100644 --- a/dlls/kernelbase/registry.c +++ b/dlls/kernelbase/registry.c @@ -1739,13 +1739,24 @@ LSTATUS WINAPI RegGetValueW( HKEY hKey, LPCWSTR pszSubKey, LPCWSTR pszValue,
if (pvData && !pcbData) return ERROR_INVALID_PARAMETER; + if ((dwFlags & RRF_RT_REG_EXPAND_SZ) && !(dwFlags & RRF_NOEXPAND) && ((dwFlags & RRF_RT_ANY) != RRF_RT_ANY)) return ERROR_INVALID_PARAMETER;
+ if ((dwFlags & (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY)) == (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY)) + return ERROR_INVALID_PARAMETER; + if (pszSubKey && pszSubKey[0]) { - ret = RegOpenKeyExW(hKey, pszSubKey, 0, KEY_QUERY_VALUE, &hKey); + REGSAM samDesired = KEY_QUERY_VALUE; + + if (dwFlags & RRF_SUBKEY_WOW6464KEY) + samDesired |= KEY_WOW64_64KEY; + else if (dwFlags & RRF_SUBKEY_WOW6432KEY) + samDesired |= KEY_WOW64_32KEY; + + ret = RegOpenKeyExW(hKey, pszSubKey, 0, samDesired, &hKey); if (ret != ERROR_SUCCESS) return ret; }
@@ -1835,13 +1846,24 @@ LSTATUS WINAPI RegGetValueA( HKEY hKey, LPCSTR pszSubKey, LPCSTR pszValue,
if (pvData && !pcbData) return ERROR_INVALID_PARAMETER; + if ((dwFlags & RRF_RT_REG_EXPAND_SZ) && !(dwFlags & RRF_NOEXPAND) && ((dwFlags & RRF_RT_ANY) != RRF_RT_ANY)) return ERROR_INVALID_PARAMETER;
+ if ((dwFlags & (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY)) == (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY)) + return ERROR_INVALID_PARAMETER; + if (pszSubKey && pszSubKey[0]) { - ret = RegOpenKeyExA(hKey, pszSubKey, 0, KEY_QUERY_VALUE, &hKey); + REGSAM samDesired = KEY_QUERY_VALUE; + + if (dwFlags & RRF_SUBKEY_WOW6464KEY) + samDesired |= KEY_WOW64_64KEY; + else if (dwFlags & RRF_SUBKEY_WOW6432KEY) + samDesired |= KEY_WOW64_32KEY; + + ret = RegOpenKeyExA(hKey, pszSubKey, 0, samDesired, &hKey); if (ret != ERROR_SUCCESS) return ret; }
diff --git a/include/winreg.h b/include/winreg.h index c003791..665ea8b 100644 --- a/include/winreg.h +++ b/include/winreg.h @@ -80,7 +80,9 @@ typedef LONG LSTATUS; #define RRF_RT_REG_QWORD (1 << 6) #define RRF_RT_DWORD (RRF_RT_REG_BINARY | RRF_RT_REG_DWORD) #define RRF_RT_QWORD (RRF_RT_REG_BINARY | RRF_RT_REG_QWORD) -#define RRF_RT_ANY 0xffff +#define RRF_RT_ANY 0x0000ffff +#define RRF_SUBKEY_WOW6464KEY (1 << 16) +#define RRF_SUBKEY_WOW6432KEY (1 << 17) #define RRF_NOEXPAND (1 << 28) #define RRF_ZEROONFAILURE (1 << 29)
On 2/19/20 7:10 AM, Serge Gautherie wrote:
- if ((dwFlags & (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY)) == (RRF_SUBKEY_WOW6464KEY | RRF_SUBKEY_WOW6432KEY))
return ERROR_INVALID_PARAMETER;if (pszSubKey && pszSubKey[0]) {
ret = RegOpenKeyExW(hKey, pszSubKey, 0, KEY_QUERY_VALUE, &hKey);
REGSAM samDesired = KEY_QUERY_VALUE;if (dwFlags & RRF_SUBKEY_WOW6464KEY)samDesired |= KEY_WOW64_64KEY;else if (dwFlags & RRF_SUBKEY_WOW6432KEY)samDesired |= KEY_WOW64_32KEY;ret = RegOpenKeyExW(hKey, pszSubKey, 0, samDesired, &hKey); if (ret != ERROR_SUCCESS) return ret; }
What does RegOpenKeyExW return for KEY_WOW64_64KEY | KEY_WOW64_32KEY? If it's ERROR_INVALID_PARAMETER, it's better to fix it there, in one place.
On 19/02/2020 06:42, Nikolay Sivov wrote:
What does RegOpenKeyExW return for KEY_WOW64_64KEY | KEY_WOW64_32KEY? If
I agree to review tests (and code) about those flags separately/later.
it's ERROR_INVALID_PARAMETER, it's better to fix it there, in one place.
Not in this case: flags are different, so validating them cannot be done elsewhere.
On 2/19/20 7:23 PM, Serge Gautherie wrote:
On 19/02/2020 06:42, Nikolay Sivov wrote:
What does RegOpenKeyExW return for KEY_WOW64_64KEY | KEY_WOW64_32KEY? If
I agree to review tests (and code) about those flags separately/later.
it's ERROR_INVALID_PARAMETER, it's better to fix it there, in one place.
Not in this case: flags are different, so validating them cannot be done elsewhere.
See attached diff. Specifying both flags returns ERROR_INVALID_PARAMETER. My point is that RegGetValue() could simply convert flags and rely on RegOpenKeyEx() to validate them, instead of validation in two places.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65515
Your paranoid android.
=== w864 (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1507 (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809 (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87) registry.c:3424: Test failed: expected ERROR_SUCCESS, got 6 registry.c:3552: Test failed: expected WAIT_OBJECT_0, got 258
=== w1064v1809_2scr (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809_ar (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809_he (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809_ja (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809_zh_CN (32 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w864 (64 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1507 (64 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
=== w1064v1809 (64 bit report) ===
advapi32: registry.c:1100: Test failed: RegOpenKeyEx with KEY_WOW64_64KEY failed (err=87)
On 19/02/2020 18:35, Nikolay Sivov wrote:
See attached diff. Specifying both flags returns ERROR_INVALID_PARAMETER.
I think this is exactly why that should never be done intentionally, but on tests.
My point is that RegGetValue() could simply convert flags and rely on RegOpenKeyEx() to validate them, instead of validation in two places.
I would agree with you if flags were implicitly passed through, which is not the case here.
My point is: *No redundancy here: 2 functions, 2 sets of flags, 2 validations. *What if flag conversion is broken? *What when RegOpenKeyEx() is not called? (See tests on PATCH 2/2.)
-
Marvin -
Nikolay Sivov -
Serge Gautherie