[PATCH 0/1] MR5911: ntdll: Don't special-case default values in RtlQueryRegistryValues.
Additional tests reveal that RTL_QUERY_REGISTRY_DIRECT does work with non-string default values, it just requires a pointer to the value and it doesn't infer the size automatically. That means that the same code can be used to handle both default and non-default values. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5911
From: Alex Henrie <alexhenrie24(a)gmail.com> Additional tests reveal that RTL_QUERY_REGISTRY_DIRECT does work with non-string default values, it just requires a pointer to the value and it doesn't infer the size automatically. That means that the same code can be used to handle both default and non-default values. --- dlls/ntdll/reg.c | 102 ++++++++++++++++++++--------------------- dlls/ntdll/tests/reg.c | 89 ++++++++++++++++++++++++++--------- 2 files changed, 118 insertions(+), 73 deletions(-) diff --git a/dlls/ntdll/reg.c b/dlls/ntdll/reg.c index d39c1a34bb7..934124a0360 100644 --- a/dlls/ntdll/reg.c +++ b/dlls/ntdll/reg.c @@ -243,58 +243,59 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, PRTL_QUERY_REGISTRY_TABLE pQuery, PVOID pContext, PVOID pEnvironment) { PUNICODE_STRING str = pQuery->EntryContext; + ULONG type, len, offset, count, res; UNICODE_STRING src, dst; + WCHAR *data, *wstr; LONG *bin; - ULONG offset; - PWSTR wstr; - DWORD res; NTSTATUS status = STATUS_SUCCESS; - ULONG len; - if (pInfo == NULL) + if (pInfo) { - ULONG default_size = pQuery->DefaultLength; - if (!default_size && pQuery->DefaultType == REG_SZ && pQuery->DefaultData) - default_size = (wcslen(pQuery->DefaultData) + 1) * sizeof(WCHAR); + type = pInfo->Type; + data = (WCHAR*)((char*)pInfo + pInfo->DataOffset); + len = pInfo->DataLength; + } + else + { + type = pQuery->DefaultType; + data = pQuery->DefaultData; + len = pQuery->DefaultLength; - if (pQuery->Flags & RTL_QUERY_REGISTRY_DIRECT) - { - if (pQuery->QueryRoutine) - return STATUS_INVALID_PARAMETER; + if (!data) + return STATUS_DATA_OVERRUN; - if (pQuery->DefaultType == REG_SZ || pQuery->DefaultType == REG_EXPAND_SZ || - pQuery->DefaultType == REG_MULTI_SZ || pQuery->DefaultType == REG_LINK) + if (!len) + { + switch (type) { - if (!pQuery->DefaultData) - return STATUS_DATA_OVERRUN; - if (str->MaximumLength < default_size) - return STATUS_BUFFER_TOO_SMALL; - memcpy(str->Buffer, pQuery->DefaultData, default_size); - str->Length = default_size - sizeof(WCHAR); - } + case REG_SZ: + case REG_EXPAND_SZ: + case REG_LINK: + len = (wcslen(data) + 1) * sizeof(WCHAR); + break; - return STATUS_SUCCESS; - } - else if (pQuery->QueryRoutine) - { - status = pQuery->QueryRoutine(pQuery->Name, pQuery->DefaultType, pQuery->DefaultData, - default_size, pContext, pQuery->EntryContext); + case REG_MULTI_SZ: + for (wstr = data; *wstr; wstr += count) + { + count = wcslen(wstr) + 1; + len += count * sizeof(WCHAR); + } + break; + } } - return status; } - len = pInfo->DataLength; if (pQuery->Flags & RTL_QUERY_REGISTRY_DIRECT) { if (pQuery->QueryRoutine) return STATUS_INVALID_PARAMETER; - switch(pInfo->Type) + switch (type) { case REG_EXPAND_SZ: if (!(pQuery->Flags & RTL_QUERY_REGISTRY_NOEXPAND)) { - RtlInitUnicodeString(&src, (WCHAR*)(((CHAR*)pInfo) + pInfo->DataOffset)); + RtlInitUnicodeString(&src, data); res = 0; dst.MaximumLength = 0; RtlExpandEnvironmentStrings_U(pEnvironment, &src, &dst, &res); @@ -307,12 +308,12 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, case REG_SZ: case REG_LINK: if (str->Buffer == NULL) - RtlCreateUnicodeString(str, (WCHAR*)(((CHAR*)pInfo) + pInfo->DataOffset)); + RtlCreateUnicodeString(str, data); else { if (str->MaximumLength < len) return STATUS_BUFFER_TOO_SMALL; - memcpy(str->Buffer, (char*)pInfo + pInfo->DataOffset, len); + memcpy(str->Buffer, data, len); str->Length = len - sizeof(WCHAR); } break; @@ -330,28 +331,27 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, else if (str->MaximumLength < len) return STATUS_BUFFER_TOO_SMALL; len -= sizeof(WCHAR); - memcpy(str->Buffer, ((CHAR*)pInfo) + pInfo->DataOffset, len); + memcpy(str->Buffer, data, len); str->Buffer[len / sizeof(WCHAR)] = 0; str->Length = len; break; default: bin = pQuery->EntryContext; - if (pInfo->DataLength <= sizeof(ULONG)) - memcpy(bin, ((CHAR*)pInfo) + pInfo->DataOffset, - pInfo->DataLength); + if (len <= sizeof(ULONG)) + memcpy(bin, data, len); else { if (bin[0] < 0) { - if (pInfo->DataLength <= -bin[0]) - memcpy(bin, (char*)pInfo + pInfo->DataOffset, pInfo->DataLength); + if (len <= -bin[0]) + memcpy(bin, data, len); } - else if (pInfo->DataLength <= bin[0]) + else if (len <= bin[0]) { bin[0] = len; - bin[1] = pInfo->Type; - memcpy(bin + 2, (char*)pInfo + pInfo->DataOffset, len); + bin[1] = type; + memcpy(bin + 2, data, len); } } break; @@ -360,15 +360,13 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, else if (pQuery->QueryRoutine) { if((pQuery->Flags & RTL_QUERY_REGISTRY_NOEXPAND) || - (pInfo->Type != REG_EXPAND_SZ && pInfo->Type != REG_MULTI_SZ)) + (type != REG_EXPAND_SZ && type != REG_MULTI_SZ)) { - status = pQuery->QueryRoutine(pQuery->Name, pInfo->Type, - ((CHAR*)pInfo) + pInfo->DataOffset, pInfo->DataLength, - pContext, pQuery->EntryContext); + status = pQuery->QueryRoutine(pQuery->Name, type, data, len, pContext, pQuery->EntryContext); } - else if (pInfo->Type == REG_EXPAND_SZ) + else if (type == REG_EXPAND_SZ) { - RtlInitUnicodeString(&src, (WCHAR*)(((CHAR*)pInfo) + pInfo->DataOffset)); + RtlInitUnicodeString(&src, data); res = 0; dst.MaximumLength = 0; RtlExpandEnvironmentStrings_U(pEnvironment, &src, &dst, &res); @@ -381,11 +379,11 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, } else /* REG_MULTI_SZ */ { - for (offset = 0; offset < pInfo->DataLength; offset += len) + for (offset = 0; offset < len; offset += count) { - wstr = (WCHAR*)((char*)pInfo + pInfo->DataOffset + offset); - len = (wcslen(wstr) + 1) * sizeof(WCHAR); - status = pQuery->QueryRoutine(pQuery->Name, REG_SZ, wstr, len, pContext, pQuery->EntryContext); + wstr = (WCHAR*)((char*)data + offset); + count = (wcslen(wstr) + 1) * sizeof(WCHAR); + status = pQuery->QueryRoutine(pQuery->Name, REG_SZ, wstr, count, pContext, pQuery->EntryContext); if (status != STATUS_SUCCESS && status != STATUS_BUFFER_TOO_SMALL) return status; } diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index e1ca45b9320..bacc3dad282 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2632,7 +2632,7 @@ static ULONG query_reg_values_direct_int; static union { ULONG size; - char data[16]; + char data[32]; } query_reg_values_direct_sized; @@ -2640,7 +2640,7 @@ static struct { ULONG size; ULONG type; - char data[16]; + char data[32]; } query_reg_values_direct_typed; @@ -2708,7 +2708,7 @@ static struct query_reg_values_test query_reg_values_tests[] = }, { {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"MeaningOfLife32", &query_reg_values_direct_int }}, - STATUS_SUCCESS, 0, 0, REG_NONE, (WCHAR*)42 + STATUS_SUCCESS, 0, 0, REG_NONE, L"\x2a" }, { {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"MeaningOfLife64", &query_reg_values_direct_sized }}, @@ -2783,11 +2783,11 @@ static struct query_reg_values_test query_reg_values_tests[] = }, { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_EXPAND_SZ, (WCHAR*)L"%SYSTEMDRIVE%" }}, - STATUS_SUCCESS, 1, WINE_TODO_TYPE | WINE_TODO_SIZE, REG_SZ, L"C:" + STATUS_SUCCESS, 1, 0, REG_SZ, L"C:" }, { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_MULTI_SZ, (WCHAR*)L"Brussels\0Paris\0%PATH%\0" }}, - STATUS_SUCCESS, 3, EXPECT_DEFAULT_DATA | SPLIT_MULTI | WINE_TODO_CALLS | WINE_TODO_TYPE | WINE_TODO_SIZE + STATUS_SUCCESS, 3, EXPECT_DEFAULT_DATA | SPLIT_MULTI }, { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_DWORD, (WCHAR*)0xdeadbeef }}, @@ -2811,23 +2811,68 @@ static struct query_reg_values_test query_reg_values_tests[] = { {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", &query_reg_values_direct_str, REG_EXPAND_SZ, (WCHAR*)L"%SYSTEMDRIVE%" }}, - STATUS_SUCCESS, 0, WINE_TODO_SIZE | WINE_TODO_DATA, REG_NONE, L"C:" + STATUS_SUCCESS, 0, 0, REG_NONE, L"C:" }, { {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", &query_reg_values_direct_str, REG_EXPAND_SZ, (WCHAR*)L"%SYSTEMDRIVE%" }}, - STATUS_SUCCESS, 0, WINE_TODO_SIZE, REG_NONE, L"\x2323", 0, 2 * sizeof(WCHAR) + STATUS_SUCCESS, 0, 0, REG_NONE, L"\x2323", 0, 2 * sizeof(WCHAR) + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_str, REG_NONE, (WCHAR*)L"Some default", 4 * sizeof(WCHAR) }}, + STATUS_SUCCESS, 0, WINE_TODO_SIZE, REG_NONE, NULL, -1 + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_int, REG_DWORD, (WCHAR*)0xdeadbeef }}, + STATUS_SUCCESS, 0 + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_int, REG_DWORD, (WCHAR*)L"\x2a", sizeof(DWORD) }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_sized, REG_DWORD, (WCHAR*)L"Some default", sizeof(L"Some default") }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_sized, REG_DWORD, (WCHAR*)L"Some default", sizeof(L"Some default") }}, + STATUS_SUCCESS, 0, 0, REG_NONE, L"\xff", 1, 1 + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_typed, REG_DWORD, (WCHAR*)L"Some default", sizeof(L"Some default") }}, + STATUS_SUCCESS, 0, WINE_TODO_TYPE | WINE_TODO_SIZE, 0x23, NULL, -1 }, - /* DIRECT with a multi-string default value crashes on Windows */ + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_typed, REG_QWORD, (WCHAR*)L"\x2a\0\0", sizeof(UINT64) }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_typed, REG_QWORD, (WCHAR*)L"\x2a\0\0", sizeof(UINT64) }}, + STATUS_SUCCESS, 0, 0, 0x23, L"\x23", 1, 1 + }, + /* DIRECT with a multi-string default value crashes on Windows without NOEXPAND */ /* { {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", - &query_reg_values_direct_str, REG_NONE, (WCHAR*)L"A\0B\0C\0", sizeof(L"A\0B\0C\0") }}, - STATUS_SUCCESS, EXPECT_DEFAULT_DATA + &query_reg_values_direct_str, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C\0", sizeof(L"A\0B\0C\0") }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA }, */ + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOEXPAND, (WCHAR*)L"I don't exist", + &query_reg_values_direct_str, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C\0", sizeof(L"A\0B\0C\0") }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA | WINE_TODO_SIZE + }, /* The default value is not used if it is not valid */ { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_SZ }}, - STATUS_DATA_OVERRUN, 0, EXPECT_DEFAULT_DATA | WINE_TODO_RET | WINE_TODO_CALLS + STATUS_DATA_OVERRUN, 0, EXPECT_DEFAULT_DATA }, { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_NONE, (WCHAR*)L"Some default" }}, @@ -2843,12 +2888,6 @@ static struct query_reg_values_test query_reg_values_tests[] = &query_reg_values_direct_str, REG_NONE, (WCHAR*)L"Some default" }}, STATUS_SUCCESS, 0, 0, REG_NONE, NULL, -1 }, - /* DIRECT additionally requires the default value to be a string */ - { - {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", - &query_reg_values_direct_int, REG_DWORD, (WCHAR*)0xdeadbeef }}, - STATUS_SUCCESS, 0 - }, /* REQUIRED fails if the value doesn't exist and there is no default */ { {{ query_routine, RTL_QUERY_REGISTRY_REQUIRED, (WCHAR*)L"I don't exist", @@ -2985,12 +3024,12 @@ static void test_RtlQueryRegistryValues(void) { if (expected_data) { - ok(!memcmp(&query_reg_values_direct_int, &expected_data, expected_size), + ok(!memcmp(&query_reg_values_direct_int, expected_data, expected_size), "Data does not match\n"); } else { -- ok(query_reg_values_direct_int == 1, + ok(query_reg_values_direct_int == 1, "Expected data to not change, got %lu\n", query_reg_values_direct_int); } } @@ -3001,14 +3040,22 @@ static void test_RtlQueryRegistryValues(void) } else if (query->EntryContext == &query_reg_values_direct_typed) { + if (expected_size == -1) + expected_size = sizeof(query_reg_values_direct_typed.data); + + todo_wine_if(test->flags & WINE_TODO_SIZE) ok(query_reg_values_direct_typed.size == expected_size, "Expected size %lu, got %lu\n", expected_size, query_reg_values_direct_typed.size); + todo_wine_if(test->flags & WINE_TODO_TYPE) ok(query_reg_values_direct_typed.type == expected_type, "Expected type %lu, got %lu\n", expected_type, query_reg_values_direct_typed.type); - ok(!memcmp(query_reg_values_direct_typed.data, expected_data, expected_size), - "Data does not match\n"); + if (expected_data) + { + ok(!memcmp(query_reg_values_direct_typed.data, expected_data, expected_size), + "Data does not match\n"); + } } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5911
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146505 Your paranoid android. === debian11 (32 bit report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit ar:MA report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit de report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit fr report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit he:IL report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit hi:IN report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit ja:JP report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11 (32 bit zh:CN report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11b (32 bit WoW report) === Report validation errors: ntdll:reg crashed (c0000005) === debian11b (64 bit WoW report) === Report validation errors: ntdll:reg crashed (c0000005)
participants (3)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Marvin