[PATCH v4 0/1] MR766: advapi32/test: Add tests for RegLoadAppKey.
-- v4: advapi32/tests: Add tests for RegLoadAppKey. https://gitlab.winehq.org/wine/wine/-/merge_requests/766
From: Santino Mazza <smazza(a)codeweavers.com> Signed-off-by: Santino Mazza <smazza(a)codeweavers.com> --- dlls/advapi32/tests/registry.c | 123 ++++++++++++++++++++++++++++++++- dlls/kernelbase/registry.c | 6 +- 2 files changed, 122 insertions(+), 7 deletions(-) diff --git a/dlls/advapi32/tests/registry.c b/dlls/advapi32/tests/registry.c index df56b0968ee..4ca412459aa 100644 --- a/dlls/advapi32/tests/registry.c +++ b/dlls/advapi32/tests/registry.c @@ -41,6 +41,7 @@ static DWORD GLE; static const char * sTestpath1 = "%LONGSYSTEMVAR%\\subdir1"; static const char * sTestpath2 = "%FOO%\\subdir1"; +static char temppath[MAX_PATH]; static const DWORD ptr_size = 8 * sizeof(void*); static DWORD (WINAPI *pRegGetValueA)(HKEY,LPCSTR,LPCSTR,DWORD,LPDWORD,PVOID,LPDWORD); @@ -1600,7 +1601,58 @@ static void test_reg_unload_key(void) DeleteFileA("saved_key.LOG"); } -/* tests that show that RegConnectRegistry and +static void test_reg_load_app_key(void) +{ + DWORD ret; + DWORD size1; + DWORD size2; + char hivefilepath[MAX_PATH]; + HANDLE hivefile; + HKEY childkey; + HKEY key1 = NULL; + + GetTempFileNameA(temppath, "key", 0, hivefilepath); + DeleteFileA(hivefilepath); + + if (!set_privileges(SE_BACKUP_NAME, TRUE) || + !set_privileges(SE_RESTORE_NAME, FALSE)) + { + win_skip("Failed to set SE_BACKUP_NAME privileges, skipping tests\n"); + return; + } + + ret = RegSaveKeyA(hkey_main, hivefilepath, NULL); + if (ret != ERROR_SUCCESS) + { + win_skip("Failed to save test key 0x%lx\n", ret); + return; + } + + set_privileges(SE_BACKUP_NAME, FALSE); + set_privileges(SE_RESTORE_NAME, FALSE); + + /* Test simple key load */ + /* Check if the changes are saved to the file */ + hivefile = CreateFileA(hivefilepath, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + size1 = GetFileSize(hivefile, NULL); + CloseHandle(hivefile); + + ret = RegLoadAppKeyA(hivefilepath, &key1, KEY_READ | KEY_WRITE, 0, 0); + todo_wine ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); + todo_wine ok(key1 != NULL, "got a null key\n"); + + ret = RegCreateKeyA(key1, "testkey", &childkey); + RegCloseKey(key1); + + hivefile = CreateFileA(hivefilepath, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + size2 = GetFileSize(hivefile, NULL); + todo_wine ok(size2 > size1, "Expected file to increase size. %ld <= %ld\n", size2, size1); + CloseHandle(hivefile); +} + +/* tests that show that RegConnectRegistry and OpenSCManager accept computer names without the \\ prefix (what MSDN says). */ static void test_regconnectregistry( void) @@ -4459,12 +4511,61 @@ static void test_RegRenameKey(void) RegCloseKey(key); } +static void init_temp_folder(void) +{ + GetTempPathA(sizeof(temppath), temppath); + sprintf(temppath, "%sadvapi32_tests\\", temppath); + CreateDirectoryA(temppath, NULL); +} + +static void delete_temp_folder(void) +{ + WIN32_FIND_DATAA findfile; + HANDLE search; + char temppath_wildcard[MAX_PATH], tempfilepath[MAX_PATH]; + sprintf(temppath_wildcard, "%s*.tmp*", temppath); + + if((search = FindFirstFileA(temppath_wildcard, &findfile)) != INVALID_HANDLE_VALUE) + { + do + { + int attempts = 0; + sprintf(tempfilepath, "%s%s", temppath, findfile.cFileName); + while (!DeleteFileA(tempfilepath) && attempts++ <= 10) + { + if (winetest_debug > 1) + printf("Failed at deleting %s attempt %d/10\n", tempfilepath, attempts); + Sleep(500); + } + } while (FindNextFileA(search, &findfile)); + } + + RemoveDirectoryA(temppath); +} + START_TEST(registry) { + static const char *test_names[] = + { + "RegLoadAppKey" + }; + char pathname[MAX_PATH]; + char **argv; + int argc; /* Load pointers for functions that are not available in all Windows versions */ InitFunctionPtrs(); - + init_temp_folder(); + argc = winetest_get_mainargs(&argv); setup_main_key(); + + if (argc >= 3) + { + if (!strcmp(argv[2], "RegLoadAppKey")) + test_reg_load_app_key(); + + return; + } + check_user_privs(); test_set_value(); create_test_entries(); @@ -4500,8 +4601,24 @@ START_TEST(registry) test_perflib_key(); test_RegRenameKey(); + for (int i = 0; i < ARRAY_SIZE(test_names); ++i) + { + STARTUPINFOA startup; + PROCESS_INFORMATION info; + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + + sprintf(pathname, "%s registry %s", argv[0], test_names[i]); + ok(CreateProcessA(NULL, pathname, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info), + "Process creation failed\n"); + wait_child_process(info.hProcess); + CloseHandle(info.hProcess); + CloseHandle(info.hThread); + } + /* cleanup */ delete_key( hkey_main ); - + delete_temp_folder(); test_regconnectregistry(); } diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c index 91462d80e06..b05d5b2cf9d 100644 --- a/dlls/kernelbase/registry.c +++ b/dlls/kernelbase/registry.c @@ -3092,8 +3092,7 @@ LSTATUS WINAPI RegLoadAppKeyA(const char *file, HKEY *result, REGSAM sam, DWORD if (!file || reserved) return ERROR_INVALID_PARAMETER; - *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + return STATUS_NOT_IMPLEMENTED; } /****************************************************************************** @@ -3107,8 +3106,7 @@ LSTATUS WINAPI RegLoadAppKeyW(const WCHAR *file, HKEY *result, REGSAM sam, DWORD if (!file || reserved) return ERROR_INVALID_PARAMETER; - *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + return STATUS_NOT_IMPLEMENTED; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/766
Huw Davies (@huw) commented about dlls/advapi32/tests/registry.c:
+ FILE_ATTRIBUTE_NORMAL, NULL); + size1 = GetFileSize(hivefile, NULL); + CloseHandle(hivefile); + + ret = RegLoadAppKeyA(hivefilepath, &key1, KEY_READ | KEY_WRITE, 0, 0); + todo_wine ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); + todo_wine ok(key1 != NULL, "got a null key\n"); + + ret = RegCreateKeyA(key1, "testkey", &childkey); + RegCloseKey(key1); + + hivefile = CreateFileA(hivefilepath, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + size2 = GetFileSize(hivefile, NULL); + todo_wine ok(size2 > size1, "Expected file to increase size. %ld <= %ld\n", size2, size1); + CloseHandle(hivefile); You're leaking `childkey`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/766#note_7779
Huw Davies (@huw) commented about dlls/advapi32/tests/registry.c:
+{ + WIN32_FIND_DATAA findfile; + HANDLE search; + char temppath_wildcard[MAX_PATH], tempfilepath[MAX_PATH]; + sprintf(temppath_wildcard, "%s*.tmp*", temppath); + + if((search = FindFirstFileA(temppath_wildcard, &findfile)) != INVALID_HANDLE_VALUE) + { + do + { + int attempts = 0; + sprintf(tempfilepath, "%s%s", temppath, findfile.cFileName); + while (!DeleteFileA(tempfilepath) && attempts++ <= 10) + { + if (winetest_debug > 1) + printf("Failed at deleting %s attempt %d/10\n", tempfilepath, attempts); Use `trace()` in tests. However I'm not convinced that this is particularly useful. It would be more interesting to add a test that `DeleteFile()` eventually succeeds, so that we'd notice if we weren't waiting long enough.
On that note, I think sleeping for 100ms would be better. On my Win 7 VM it typically succeeds on the 2nd attempt with a 100ms wait. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/766#note_7780
Huw Davies (@huw) commented about dlls/kernelbase/registry.c:
if (!file || reserved) return ERROR_INVALID_PARAMETER;
- *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + return STATUS_NOT_IMPLEMENTED;
Changing the implementation like this might introduce a regression - please leave the implementation alone at this point. You'll have to adjust your new tests to cope with this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/766#note_7781
On Tue Sep 6 12:08:53 2022 +0000, Huw Davies wrote:
Use `trace()` in tests. However I'm not convinced that this is particularly useful. It would be more interesting to add a test that `DeleteFile()` eventually succeeds, so that we'd notice if we weren't waiting long enough. On that note, I think sleeping for 100ms would be better. On my Win 7 VM it typically succeeds on the 2nd attempt with a 100ms wait. In my machine and also in the testbot it succeeds after 7 attempts and sometimes it can't make it with 10 attempts when using 100ms, I think 200ms it's good.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/766#note_7812
participants (3)
-
Huw Davies (＠huw) -
Santino Mazza
-
Santino Mazza (＠tati1454)