Re: [PATCH v3] ntdll: Add RtlGetDeviceFamilyInfoEnum and RtlConvertDeviceFamilyInfoToString

[Zebediah Figura]

On 7/4/22 17:03, Kacper Rączy wrote:
> Thanks for the feedback.
> > Wiadomość napisana przez Zebediah Figura (she/her)  w dniu 03.07.2022, o godz. 22:05:
> >
> > 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
> >
> > 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.
>
> Microsoft’s GameServices libraries which are mostly used by games packaged as UWP in Microsoft Store - these libs contain Xbox specific segments, and use this APIs to differentiate between platforms. I’m aware that UWP support may not be priority right now, but this is first step to get UWP packaged games to even link/launch (although most of them may still not work, but thats something I’m also working on one step at a time).

Thanks, that strikes me at least as reasonable.
> > > @@ -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.
>
> I think the APIs above have those. It is convention for new APIs?

I believe we're still adding the three-line header for new exports 
(well, mostly, although some people have been leaving it out), but the 
"NOTES" section seems superfluous; we generally don't care about Windows 
version.
> > > - */
> > >
> > > +void WINAPI RtlGetDeviceFamilyInfoEnum( ULONGLONG *uap_info, DWORD *device_family, DWORD *device_form )
> > > +{
> >
> > Can you please add a TRACE to any new functions?
>
> Ok
> > > - if (device_form)
> > > -
> > >    *device_form = DEVICEFAMILYDEVICEFORM_UNKNOWN;
> >
> > Does Windows really return this?
>
> This seem to depend on OEM - there’s special registry key for that Software\Microsoft\Windows NT\CurrentVersion\OEM\DeviceForm. If value is not there, windows returns Unknown - this is also what is happening on desktop PCs with OS manually installed.
>
> I’ve seen this API used to differentiate whether host is an Xbox or PC, that’s why I thought leaving it fixed to values returned by most PCs is sufficient.

Okay, that makes sense. I hadn't researched anything; it just seemed 
conceptually surprising that Windows would return UNKNOWN here, but it 
makes sense to emulate what a desktop would return.