[PATCH v2 1/3] user32: Retrieve the usage page and usage directly from the hid_preparsed_data structure.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/user32/rawinput.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index a014066f988..961efecd075 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -177,7 +177,6 @@ void rawinput_update_device_list(void) SP_DEVICE_INTERFACE_DATA iface = { sizeof(iface) }; struct device *device; HIDD_ATTRIBUTES attr; - HIDP_CAPS caps; GUID hid_guid; HDEVINFO set; DWORD idx; @@ -201,6 +200,8 @@ void rawinput_update_device_list(void)
for (idx = 0; SetupDiEnumDeviceInterfaces(set, NULL, &hid_guid, idx, &iface); ++idx) { + const struct hid_preparsed_data *preparsed; + if (!(device = add_device(set, &iface))) continue;
@@ -215,12 +216,10 @@ void rawinput_update_device_list(void)
if (!HidD_GetPreparsedData(device->file, &device->data)) WARN("Failed to get preparsed data.\n"); + preparsed = (struct hid_preparsed_data *)device->data;
- if (!HidP_GetCaps(device->data, &caps)) - WARN("Failed to get caps.\n"); - - device->info.hid.usUsagePage = caps.UsagePage; - device->info.hid.usUsage = caps.Usage; + device->info.hid.usUsagePage = preparsed->usage_page; + device->info.hid.usUsage = preparsed->usage; }
SetupDiDestroyDeviceInfoList(set);
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/user32/rawinput.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index 961efecd075..518396038d6 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -38,6 +38,7 @@ #include "user_private.h"
#include "initguid.h" +#include "ddk/hidclass.h" #include "devpkey.h" #include "ntddmou.h" #include "ntddkbd.h" @@ -177,14 +178,11 @@ void rawinput_update_device_list(void) SP_DEVICE_INTERFACE_DATA iface = { sizeof(iface) }; struct device *device; HIDD_ATTRIBUTES attr; - GUID hid_guid; HDEVINFO set; DWORD idx;
TRACE("\n");
- HidD_GetHidGuid(&hid_guid); - EnterCriticalSection(&rawinput_devices_cs);
/* destroy previous list */ @@ -196,9 +194,9 @@ void rawinput_update_device_list(void) } rawinput_devices_count = 0;
- set = SetupDiGetClassDevsW(&hid_guid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); + set = SetupDiGetClassDevsW(&GUID_DEVINTERFACE_HID, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT);
- for (idx = 0; SetupDiEnumDeviceInterfaces(set, NULL, &hid_guid, idx, &iface); ++idx) + for (idx = 0; SetupDiEnumDeviceInterfaces(set, NULL, &GUID_DEVINTERFACE_HID, idx, &iface); ++idx) { const struct hid_preparsed_data *preparsed;
Signed-off-by: Zebediah Figura zfigura@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; }
SetupDiDestroyDeviceInfoList(set); @@ -760,12 +765,12 @@ UINT WINAPI GetRawInputDeviceInfoW(HANDLE handle, UINT command, void *data, UINT break;
case RIDI_PREPARSEDDATA: - if (!(preparsed = (struct hid_preparsed_data *)device->data)) len = 0; + if (!(preparsed = device->data)) len = 0; else len = preparsed->caps_size + FIELD_OFFSET(struct hid_preparsed_data, value_caps[0]) + preparsed->number_link_collection_nodes * sizeof(struct hid_collection_node);
- if (device->data && len <= data_len && data) - memcpy(data, device->data, len); + if (preparsed && len <= data_len && data) + memcpy(data, preparsed, len); *data_size = len; break;
On 5/19/22 07:05, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura zfigura@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.
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@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.
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@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.
-
Rémi Bernon -
Zebediah Figura