[PATCH resend] ntdll: Add RtlGetDeviceFamilyInfoEnum and RtlGetDeviceFamilyInfoEnum

Paul Gofman pgofman at codeweavers.com
Wed Jun 22 09:16:33 CDT 2022


Hi, thanks for the patch!

This could use a test which includes checks for return data. Without 
that it is hard to say if the patch is correct. Version data is not 
constant of course but it could be checked if it matches 
RtlGetVersion(). A few thoughts regarding on some possible improvements 
to the patch regardless (once there is a test which verifies it general 
correctness):


On 6/22/22 08:49, Kacper Raczy wrote:
>   
> +/*********************************************************************
> + *  RtlGetDeviceFamilyInfoEnum (NTDLL.@)
> + *
> + * NOTES
> + * Introduced in Windows 10 (NT10.0)
> + */
> +VOID WINAPI RtlGetDeviceFamilyInfoEnum(ULONGLONG *uap_info, DWORD *device_family, DWORD *device_form)
I think we currently prefer 'void' over 'VOID' and such.

Formatting: spaces around function arguments (see, e. g., 
RtlVerifyVersionInfo above).


> +{
> +    ULONGLONG uap_info_temp;
> +

Do you really need this temporary variable, any reason not to use 
*uap_info directly?


> +    if (device_form != NULL)
We usually check such just as 'if (device_form) ...'
> +    uap_info_temp |= (((ULONGLONG) current_version->dwMajorVersion & 0xffff) << 48); /* os version major */
Space after (ULONGLONG)
> +    uap_info_temp |= (((ULONGLONG) current_version->dwMinorVersion & 0xffff) << 32); /* os version minor */
> +    uap_info_temp |= (((ULONGLONG) current_version->dwBuildNumber & 0xffff) << 16); /* current build number */
> +    /* UBR not available */
> +    *uap_info = uap_info_temp;
> +}
> +
> +/*********************************************************************
> + *  RtlConvertDeviceFamilyInfoToString (NTDLL.@)
> + *
> + * NOTES
> + * Introduced in Windows 10 (NT10.0)
> + */
> +VOID WINAPI RtlConvertDeviceFamilyInfoToString(
> +    DWORD *device_family_bufsize,
> +    DWORD *device_form_bufsize,
> +    const LPWSTR device_family,
> +    const LPWSTR device_form
Parameters formatting. Also I think we currently prefer const WCHAR * 
over LPWSTR.
> +) {
> +    if (device_family_bufsize != NULL)
> +        *device_family_bufsize = (DWORD)wcslen(DEVICEFAMILYDEVICEFORM_DESKTOP_STR) + 1;
> +    if (device_family != NULL)
> +        wcscpy(device_family, DEVICEFAMILYDEVICEFORM_DESKTOP_STR);

This looks suspicious. Does Windows really only sets buffer size but 
does not check it before copying the output? Also, not apparent without 
the test if bufsize is wide character count or buffer size in bytes.

Formatting; " wcscpy(device_family, 
DEVICEFAMILYDEVICEFORM_DESKTOP_STR);" -> " wcscpy( device_family, 
DEVICEFAMILYDEVICEFORM_DESKTOP_STR );"


> +    if (device_form_bufsize != NULL)
> +        *device_form_bufsize = (DWORD)wcslen(DEVICEFAMILYINFOENUM_DESKTOP_STR) + 1;
(DWORD) cast is not needed




More information about the wine-devel mailing list