[PATCH v3] ntdll: Add RtlGetDeviceFamilyInfoEnum and RtlConvertDeviceFamilyInfoToString
Zebediah Figura (she/her)
zfigura at codeweavers.com
Sun Jul 3 15:05:15 CDT 2022
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 at 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;
>
More information about the wine-devel
mailing list