[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