[PATCH v2 0/1] MR5911: ntdll: Don't special-case default values in RtlQueryRegistryValues.

[Alex Henrie (＠alexhenrie)]

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.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5911

[PATCH v2 1/1] ntdll: Don't special-case default values in RtlQueryRegistryValues.

[Alex Henrie]

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..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");
+                    }
                 }
             }
         }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5911