Re: [PATCH 1/6] winebus.sys: Report WINEBUS\WINE_COMP_HID compatible id.

[Zebediah Figura (she/her)]

On 8/26/21 12:58 AM, Rémi Bernon wrote:
> And match it in winehid.sys instead of individual bus ids.
> Signed-off-by: Rémi Bernon
>
> dlls/winebus.sys/main.c      | 26 ++++++++++++++++++++++++--
>   dlls/winehid.sys/winehid.inf |  7 +------
>   2 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
> index 41aef8ab018..c09ea1c2828 100644
> --- a/dlls/winebus.sys/main.c
> +++ b/dlls/winebus.sys/main.c
> @@ -226,7 +226,7 @@ static WCHAR *get_device_id(DEVICE_OBJECT *device)
>       return dst;
>   }
>
> -static WCHAR *get_compatible_ids(DEVICE_OBJECT *device)
> +static WCHAR *get_hardware_ids(DEVICE_OBJECT *device)
>   {
>       struct device_extension *ext = (struct device_extension *)device->DeviceExtension;
>       WCHAR *dst;
> @@ -240,6 +240,28 @@ static WCHAR *get_compatible_ids(DEVICE_OBJECT *device)
>       return dst;
>   }
>
> +static WCHAR *get_compatible_ids(DEVICE_OBJECT *device)
> +{
> - static const WCHAR hid_compat[] =
> - {
> -
>    'W','I','N','E','B','U','S','\\','W','I','N','E','_','C','O','M','P','_','H','I','D',0
> - };
> - DWORD pos = 0, len = 0, hid_compat_len;
> - WCHAR *dst;
> -
> - hid_compat_len = strlenW(hid_compat);
> -
> - len += hid_compat_len + 1;
> -
> - if ((dst = ExAllocatePool(PagedPool, (len + 1) * sizeof(WCHAR))))
> - {
> -
>    pos += snprintfW(dst + pos, len - pos, hid_compat) + 1;

Why snprintfW() and not memcpy()? Note that memcpy() would also allow 
you to use sizeof().
> -
>    dst[pos] = 0;
> - }
> -
> - return dst;
>
> +}
> -

There are a few redundancies in this function. It's kind of justified by 
2/6, but maybe it makes more sense to leave off "pos" until that function.
> static void remove_pending_irps(DEVICE_OBJECT *device)
>   {
>       struct device_extension *ext = device->DeviceExtension;
> @@ -444,7 +466,7 @@ static NTSTATUS handle_IRP_MN_QUERY_ID(DEVICE_OBJECT *device, IRP *irp)
>       {
>           case BusQueryHardwareIDs:
>               TRACE("BusQueryHardwareIDs\n");
> -
>        irp->IoStatus.Information = (ULONG_PTR)get_compatible_ids(device);
> -
>        irp->IoStatus.Information = (ULONG_PTR)get_hardware_ids(device);
>         break;
>     case BusQueryCompatibleIDs:
>         TRACE("BusQueryCompatibleIDs\n");
>
> diff --git a/dlls/winehid.sys/winehid.inf b/dlls/winehid.sys/winehid.inf
> index f9ed4091217..c1d8a1cf999 100644
> --- a/dlls/winehid.sys/winehid.inf
> +++ b/dlls/winehid.sys/winehid.inf
> @@ -7,12 +7,7 @@ Class=HIDClass
>   Wine=mfg_section
>
> [mfg_section]
> -Wine hidraw device=device_section,HIDRAW
> -Wine IOHID device=device_section,IOHID
> -Wine libevent device=device_section,LNXEV
> -Wine SDL HID device=device_section,SDLJOY
> -Wine mouse device=device_section,WINEMOUSE
> -Wine keyboard device=device_section,WINEKEYBOARD
> +Wine HID compatible device=device_section,WINEBUS\WINE_COMP_HID
>
> [device_section.Services]
>   AddService = winehid,0x2,svc_section