[PATCH v2 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.
-- v2: ntdll: Don't special-case default values in RtlQueryRegistryValues.
From: Alex Henrie alexhenrie24@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..b9047c41972 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_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_NONE, (WCHAR*)L"Some default", sizeof(L"Some default") }}, + STATUS_SUCCESS, 0, WINE_TODO_TYPE | WINE_TODO_SIZE, 0x23, NULL, -1 + }, + { + {{ NULL, RTL_QUERY_REGISTRY_DIRECT, (WCHAR*)L"I don't exist", + &query_reg_values_direct_typed, REG_QWORD, (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"); + } } } }
-
Alex Henrie -
Alex Henrie (＠alexhenrie)