[PATCH v2 3/3] user32: Use HID ioctls directly.
Zebediah Figura
zfigura at codeweavers.com
Wed May 25 13:47:46 CDT 2022
On 5/19/22 05:48, Rémi Bernon wrote:
> On 5/19/22 10:42, Rémi Bernon wrote:
>> On 5/19/22 07:05, Zebediah Figura wrote:
>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>>> ---
>>> v2: Keep the "preparsed" local variable.
>>>
>>> dlls/user32/Makefile.in | 2 +-
>>> dlls/user32/rawinput.c | 45 +++++++++++++++++++++++------------------
>>> 2 files changed, 26 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/dlls/user32/Makefile.in b/dlls/user32/Makefile.in
>>> index 4a413746de7..c266d90c045 100644
>>> --- a/dlls/user32/Makefile.in
>>> +++ b/dlls/user32/Makefile.in
>>> @@ -3,7 +3,7 @@ MODULE = user32.dll
>>> IMPORTLIB = user32
>>> IMPORTS = $(PNG_PE_LIBS) gdi32 version sechost advapi32
>>> kernelbase win32u
>>> EXTRAINCL = $(PNG_PE_CFLAGS)
>>> -DELAYIMPORTS = hid setupapi imm32
>>> +DELAYIMPORTS = setupapi imm32
>>> C_SRCS = \
>>> button.c \
>>> diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c
>>> index 518396038d6..b3e83cf9d02 100644
>>> --- a/dlls/user32/rawinput.c
>>> +++ b/dlls/user32/rawinput.c
>>> @@ -26,11 +26,11 @@
>>> #include "windef.h"
>>> #include "winbase.h"
>>> #include "wingdi.h"
>>> +#include "winioctl.h"
>>> #include "winnls.h"
>>> #include "winreg.h"
>>> #include "winuser.h"
>>> #include "setupapi.h"
>>> -#include "ddk/hidsdi.h"
>>> #include "wine/debug.h"
>>> #include "wine/server.h"
>>> #include "wine/hid.h"
>>> @@ -53,7 +53,7 @@ struct device
>>> HANDLE file;
>>> HANDLE handle;
>>> RID_DEVICE_INFO info;
>>> - PHIDP_PREPARSED_DATA data;
>>> + struct hid_preparsed_data *data;
>>> };
>>> static struct device *rawinput_devices;
>>> @@ -146,7 +146,7 @@ static struct device *add_device(HDEVINFO set,
>>> SP_DEVICE_INTERFACE_DATA *iface)
>>> if (device)
>>> {
>>> TRACE("Updating device %x / %s.\n", handle,
>>> debugstr_w(detail->DevicePath));
>>> - HidD_FreePreparsedData(device->data);
>>> + free(device->data);
>>> CloseHandle(device->file);
>>> free(device->detail);
>>> }
>>> @@ -177,7 +177,6 @@ void rawinput_update_device_list(void)
>>> {
>>> SP_DEVICE_INTERFACE_DATA iface = { sizeof(iface) };
>>> struct device *device;
>>> - HIDD_ATTRIBUTES attr;
>>> HDEVINFO set;
>>> DWORD idx;
>>> @@ -188,7 +187,7 @@ void rawinput_update_device_list(void)
>>> /* destroy previous list */
>>> for (idx = 0; idx < rawinput_devices_count; ++idx)
>>> {
>>> - HidD_FreePreparsedData(rawinput_devices[idx].data);
>>> + free(rawinput_devices[idx].data);
>>> CloseHandle(rawinput_devices[idx].file);
>>> free(rawinput_devices[idx].detail);
>>> }
>>> @@ -198,26 +197,32 @@ void rawinput_update_device_list(void)
>>> for (idx = 0; SetupDiEnumDeviceInterfaces(set, NULL,
>>> &GUID_DEVINTERFACE_HID, idx, &iface); ++idx)
>>> {
>>> - const struct hid_preparsed_data *preparsed;
>>> + HID_COLLECTION_INFORMATION info;
>>> + IO_STATUS_BLOCK io;
>>> + NTSTATUS status;
>>> if (!(device = add_device(set, &iface)))
>>> continue;
>>> - attr.Size = sizeof(HIDD_ATTRIBUTES);
>>> - if (!HidD_GetAttributes(device->file, &attr))
>>> - WARN("Failed to get attributes.\n");
>>> + status = NtDeviceIoControlFile( device->file, NULL, NULL,
>>> NULL, &io, IOCTL_HID_GET_COLLECTION_INFORMATION,
>>> + NULL, 0, &info, sizeof(info) );
>>> + if (status)
>>> + ERR( "Failed to get collection information, status
>>> %#x.\n", status );
>>> device->info.dwType = RIM_TYPEHID;
>>> - device->info.hid.dwVendorId = attr.VendorID;
>>> - device->info.hid.dwProductId = attr.ProductID;
>>> - device->info.hid.dwVersionNumber = attr.VersionNumber;
>>> + device->info.hid.dwVendorId = info.VendorID;
>>> + device->info.hid.dwProductId = info.ProductID;
>>> + device->info.hid.dwVersionNumber = info.VersionNumber;
>>> - if (!HidD_GetPreparsedData(device->file, &device->data))
>>> - WARN("Failed to get preparsed data.\n");
>>> - preparsed = (struct hid_preparsed_data *)device->data;
>>> + device->data = malloc( info.DescriptorSize );
>>> - device->info.hid.usUsagePage = preparsed->usage_page;
>>> - device->info.hid.usUsage = preparsed->usage;
>>> + status = NtDeviceIoControlFile( device->file, NULL, NULL,
>>> NULL, &io, IOCTL_HID_GET_COLLECTION_DESCRIPTOR,
>>> + NULL, 0, device->data,
>>> info.DescriptorSize );
>>> + if (status)
>>> + ERR( "Failed to get collection descriptor, status
>>> %#x.\n", status );
>>> +
>>> + device->info.hid.usUsagePage = device->data->usage_page;
>>> + device->info.hid.usUsage = device->data->usage;
>>> }
>>
>> You lose the checks that HidD_GetPreparsedData was doing here,
>> although there wasn't really much error handling in the first place,
>> it now gets worse.
>>
>> If IOCTL_HID_GET_COLLECTION_INFORMATION fails, this calls malloc with
>> uninitialized value. Then malloc may also fail, silently now, when it
>> was at least printing a warning before.
>>
>
> I think something like the attached patches could make error handling a
> bit better, before changing the calls.
>
I'll add more complete error handling to this patch. FWIW, I already
have patches similar to 1/3 and 2/3 locally.
More information about the wine-devel
mailing list