Hello Kacper, thanks for the patch! I have a few comments and questions inlined.
On 6/23/22 15:56, Kacper Raczy wrote:
APIs introduced in Windows 10 (NT10.0). Device form and family are hardcoded to Unknown and Windows.Desktop respectively. Unit tests included in dlls/ntdll/tests/version.c
Signed-off-by: Kacper Raczy gfw.kra@gmail.com
Is there a known application which requires this patch? We tend to avoid introducing new code when not necessary, since it's more work to maintain and can cause regressions.
dlls/ntdll/ntdll.spec | 2 + dlls/ntdll/rtl.c | 2 +- dlls/ntdll/tests/Makefile.in | 3 +- dlls/ntdll/tests/version.c | 87 ++++++++++++++++++++++++++++++++++++ dlls/ntdll/version.c | 48 +++++++++++++++++++- include/winnt.h | 13 ++++++ 6 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 dlls/ntdll/tests/version.c
diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index b1650ab4306..4539a9bd0ed 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -712,6 +712,8 @@ @ stdcall RtlGetCurrentProcessorNumberEx(ptr) @ stdcall RtlGetCurrentTransaction() @ stdcall RtlGetDaclSecurityDescriptor(ptr ptr ptr ptr) +@ stdcall RtlGetDeviceFamilyInfoEnum(ptr ptr ptr) +@ stdcall RtlConvertDeviceFamilyInfoToString(ptr ptr ptr ptr) @ stdcall RtlGetElementGenericTable(ptr long) # @ stub RtlGetElementGenericTableAvl @ stdcall RtlGetEnabledExtendedFeatures(int64)
There
diff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c index 11067f44941..421cf377d32 100644 --- a/dlls/ntdll/rtl.c +++ b/dlls/ntdll/rtl.c @@ -2211,4 +2211,4 @@ char WINAPI RtlQueryProcessPlaceholderCompatibilityMode(void) { FIXME("stub\n"); return PHCM_APPLICATION_DEFAULT; -} +} \ No newline at end of file
This looks like a stray whitespace change; please try to avoid those.
diff --git a/dlls/ntdll/tests/Makefile.in b/dlls/ntdll/tests/Makefile.in index 90deb5865f8..e64b70b15e4 100644 --- a/dlls/ntdll/tests/Makefile.in +++ b/dlls/ntdll/tests/Makefile.in @@ -25,5 +25,6 @@ C_SRCS = \ thread.c \ threadpool.c \ time.c \
- version.c \
In general we also try to avoid adding a new test file just for a few tests. env.c is probably a reasonable place to put these.
virtual.c \
- wow64.c
- wow64.c
\ No newline at end of file
Another stray whitespace change here.
diff --git a/dlls/ntdll/tests/version.c b/dlls/ntdll/tests/version.c new file mode 100644 index 00000000000..296889efc8e --- /dev/null +++ b/dlls/ntdll/tests/version.c @@ -0,0 +1,87 @@ +/*
- Unit test suite for Rtl* Version API functions
- Copyright 2022 Kacper Rączy
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
- You should have received a copy of the GNU Lesser General Public
- License along with this library; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
- */
+#include "ntdll_test.h" +#include "wine/test.h"
+static const WCHAR DeviceForms[][32] = {
- L"Unknown", L"Phone", L"Tablet", L"Desktop", L"Notebook",
- L"Convertible", L"Detachable", L"All-in-One", L"Stick PC", L"Puck",
- L"Surface Hub", L"Head-mounted display", L"Industry handheld", L"Industry tablet", L"Banking",
- L"Building automation", L"Digital signage", L"Gaming", L"Home automation", L"Industrial automation",
- L"Kiosk", L"Maker board", L"Medical", L"Networking", L"Point of Service",
- L"Printing", L"Thin client", L"Toy", L"Vending", L"Industry other"
+};
+static void test_RtlGetDeviceFamilyInfoEnum(void) +{
- DWORD family, form;
- ULONGLONG uap;
- RTL_OSVERSIONINFOEXW version;
- RtlGetVersion(&version);
- RtlGetDeviceFamilyInfoEnum(&uap, &family, &form);
- ok( ((uap >> 48) & 0xffff) == version.dwMajorVersion,
"First 16-bit chunk of UAP does not match major system version %llx\n", uap );- ok( ((uap >> 32) & 0xffff) == version.dwMinorVersion,
"Second 16-bit chunk of UAP does not match minor system version %llx\n", uap );- ok( ((uap >> 16) & 0xffff) == version.dwBuildNumber,
"Third 16-bit chunk of UAP does not match build number %llx\n", uap );- ok( family <= DEVICEFAMILYINFOENUM_MAX,
"Device family is not valid: %lx\n", family );- ok( form <= DEVICEFAMILYDEVICEFORM_MAX,
"Device form is not valid: %lx\n", form );+}
+static void test_RtlConvertDeviceFamilyInfoToString(void) +{
- DWORD family_bufsize = 0, form_bufsize = 0;
- WCHAR *family, *form;
- BOOL form_valid = FALSE;
- RtlConvertDeviceFamilyInfoToString(&family_bufsize, &form_bufsize, NULL, NULL);
- ok( family_bufsize == sizeof(WCHAR) * 16, /* Windows.Desktop length */
"Device family bufsize does not match: %lu\n", family_bufsize );- ok( form_bufsize > 0, "Device form bufsize is invalid: %lu\n", form_bufsize );
- family = (WCHAR*)malloc(family_bufsize);
- form = (WCHAR*)malloc(form_bufsize);
No need to cast these.
- RtlConvertDeviceFamilyInfoToString(&family_bufsize, &form_bufsize, family, form);
- ok( wcscmp(family, L"Windows.Desktop") == 0,
"Device family string is not equal to Windows.Desktop: %ls\n", family );- /* Device form depends on OEM setting in registry,
* lets check all possible values to make it work on Windows */- for (int i = 0; i < sizeof(DeviceForms) / sizeof(WCHAR*); i++) {
if (wcscmp(form, DeviceForms[i]) == 0) {form_valid = TRUE;break;}- }
- ok( form_valid, "Device form string is not valid or known: %ls\n", form);
- free(family);
- free(form);
+}
+START_TEST(version) +{
- test_RtlGetDeviceFamilyInfoEnum();
- test_RtlConvertDeviceFamilyInfoToString();
+} diff --git a/dlls/ntdll/version.c b/dlls/ntdll/version.c index 492c24cc636..d474635ac2f 100644 --- a/dlls/ntdll/version.c +++ b/dlls/ntdll/version.c @@ -609,7 +609,6 @@ void WINAPI RtlGetNtVersionNumbers( LPDWORD major, LPDWORD minor, LPDWORD build if (build) *build = (0xF0000000 | current_version->dwBuildNumber); }
- /******************************************************************************
*/
- RtlGetNtProductType (NTDLL.@)
Stray whitespace change.
@@ -760,6 +759,53 @@ NTSTATUS WINAPI RtlVerifyVersionInfo( const RTL_OSVERSIONINFOEXW *info, return STATUS_SUCCESS; }
+/*********************************************************************
- RtlGetDeviceFamilyInfoEnum (NTDLL.@)
- NOTES
- Introduced in Windows 10 (NT10.0)
I don't think there's any need for this comment. Same below.
- */
+void WINAPI RtlGetDeviceFamilyInfoEnum( ULONGLONG *uap_info, DWORD *device_family, DWORD *device_form ) +{
Can you please add a TRACE to any new functions?
- if (device_form)
*device_form = DEVICEFAMILYDEVICEFORM_UNKNOWN;
Does Windows really return this?
- if (device_family)
*device_family = DEVICEFAMILYINFOENUM_DESKTOP;- if (!uap_info)
return;
Is it valid to call this function with NULL parameters?
- /**
* UAP info is 64 bit unsigned integer which contains four 16-bit chunks:* 1. os version major* 2. os version minor* 3. current build number* 4. update build revision- */
- *uap_info = 0;
- *uap_info |= (((ULONGLONG)current_version->dwMajorVersion & 0xffff) << 48); /* os version major */
- *uap_info |= (((ULONGLONG)current_version->dwMinorVersion & 0xffff) << 32); /* os version minor */
- *uap_info |= (((ULONGLONG)current_version->dwBuildNumber & 0xffff) << 16); /* current build number */
These comments seem superfluous; they're just repeating field names.
- /* UBR not available */
+}
+/*********************************************************************
- RtlConvertDeviceFamilyInfoToString (NTDLL.@)
- NOTES
- Introduced in Windows 10 (NT10.0)
- */
+void WINAPI RtlConvertDeviceFamilyInfoToString( DWORD *device_family_bufsize, DWORD *device_form_bufsize,
const WCHAR *device_family, const WCHAR *device_form)+{
- DWORD device_family_len = (wcslen( L"Windows.Desktop" ) + 1) * sizeof(WCHAR);
- DWORD device_form_len = (wcslen( L"Unknown" ) + 1) * sizeof(WCHAR);
Or more simply "sizeof". These can also be static const.
- if (*device_family_bufsize >= device_family_len)
wcscpy( device_family, L"Windows.Desktop" );- if (*device_form_bufsize >= device_form_len)
wcscpy( device_form, L"Unknown" ); > + *device_family_bufsize = device_family_len;- *device_form_bufsize = device_form_len;
+}
/******************************************************************************
VerSetConditionMask (NTDLL.@)diff --git a/include/winnt.h b/include/winnt.h index 87c4b4da92d..88948122b81 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -6224,6 +6224,19 @@ typedef struct _SYSTEM_CPU_SET_INFORMATION } DUMMYUNIONNAME; } SYSTEM_CPU_SET_INFORMATION, *PSYSTEM_CPU_SET_INFORMATION;
+/* Windows 10 Rtl apis */
This comment seems superfluous. We don't sort by version.
+#define DEVICEFAMILYINFOENUM_DESKTOP 0x00000003
+#define DEVICEFAMILYINFOENUM_MAX 0x00000011
+#define DEVICEFAMILYDEVICEFORM_UNKNOWN 0x00000000
+#define DEVICEFAMILYDEVICEFORM_MAX 0x00000021
+NTSYSAPI VOID WINAPI RtlGetDeviceFamilyInfoEnum(ULONGLONG*, DWORD*, DWORD*); +NTSYSAPI VOID WINAPI RtlConvertDeviceFamilyInfoToString(DWORD*, DWORD*, const WCHAR*, const WCHAR*);
My copy of the Windows SDK shows the latter function as returning DWORD, and having non-const WCHAR parameters. "const" doesn't make sense anyway since they're output.
- /* Threadpool things */ typedef DWORD TP_VERSION,*PTP_VERSION;