On 06.10.2016 04:51, Aric Stewart wrote:
Signed-off-by: Aric Stewart aric@codeweavers.com
dlls/winebus.sys/bus.h | 1 + dlls/winebus.sys/bus_udev.c | 1 + dlls/winebus.sys/main.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+)
0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt
diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h index 099558b..342a66b 100644 --- a/dlls/winebus.sys/bus.h +++ b/dlls/winebus.sys/bus.h @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 93e56f3..a01d88e 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry
udev_driver_obj = driver; driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
- driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
I guess you intentionally did not name it "common_*" because non-HID devices might need a different handler, is that correct?
if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) goto error;diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 09a59d6..ef19322 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -30,7 +30,9 @@ #include "winternl.h" #include "winreg.h" #include "setupapi.h" +#include "winioctl.h" #include "ddk/wdm.h" +#include "ddk/hidport.h" #include "wine/debug.h" #include "wine/unicode.h" #include "wine/list.h" @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) return status; }
+NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) +{
- NTSTATUS rc = irp->IoStatus.u.Status;
I would suggest to use rename the variable to "status".
- IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp);
- struct device_extension *extension = (struct device_extension *)device->DeviceExtension;
- TRACE("(%p, %p)\n", device, irp);
- switch(irpsp->Parameters.DeviceIoControl.IoControlCode)
There is a space missing after "switch".
- {
case IOCTL_HID_GET_DEVICE_ATTRIBUTES:{PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer;
It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix. Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure the buffer is big enough.
TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n");RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES));
Is there any specific reason to use RtlZeroMemory instead of memset? Something like memset(attr, 0, sizeof(*attr)) would be easier. ;)
attr->Size = sizeof(HID_DEVICE_ATTRIBUTES);attr->VendorID = extension->vid;attr->ProductID = extension->pid;attr->VersionNumber = extension->version;irp->IoStatus.u.Status = rc = STATUS_SUCCESS;irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES);break;}default:{ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode;FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n",code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3);break;}- }
- if (rc != STATUS_PENDING)
IoCompleteRequest( irp, IO_NO_INCREMENT );
Do you need to handle asynchronous requests in this function? If not you can remove the if check. For consistency reasons, you should probably remove the space after "(" and before ")" in the IoCompleteRequest call.
- return rc;
+}
NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) { static const WCHAR udevW[] = {'\','D','r','i','v','e','r','\','U','D','E','V',0};
On 10/6/16 7:00 AM, Sebastian Lackner wrote:
On 06.10.2016 04:51, Aric Stewart wrote:
Signed-off-by: Aric Stewart aric@codeweavers.com
dlls/winebus.sys/bus.h | 1 + dlls/winebus.sys/bus_udev.c | 1 + dlls/winebus.sys/main.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+)
0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt
diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h index 099558b..342a66b 100644 --- a/dlls/winebus.sys/bus.h +++ b/dlls/winebus.sys/bus.h @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 93e56f3..a01d88e 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry
udev_driver_obj = driver; driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
- driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
I guess you intentionally did not name it "common_*" because non-HID devices might need a different handler, is that correct?
That is correct.
-aric
if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) goto error;diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 09a59d6..ef19322 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -30,7 +30,9 @@ #include "winternl.h" #include "winreg.h" #include "setupapi.h" +#include "winioctl.h" #include "ddk/wdm.h" +#include "ddk/hidport.h" #include "wine/debug.h" #include "wine/unicode.h" #include "wine/list.h" @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) return status; }
+NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) +{
- NTSTATUS rc = irp->IoStatus.u.Status;
I would suggest to use rename the variable to "status".
- IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp);
- struct device_extension *extension = (struct device_extension *)device->DeviceExtension;
- TRACE("(%p, %p)\n", device, irp);
- switch(irpsp->Parameters.DeviceIoControl.IoControlCode)
There is a space missing after "switch".
- {
case IOCTL_HID_GET_DEVICE_ATTRIBUTES:{PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer;It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix. Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure the buffer is big enough.
TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n");RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES));Is there any specific reason to use RtlZeroMemory instead of memset? Something like memset(attr, 0, sizeof(*attr)) would be easier. ;)
attr->Size = sizeof(HID_DEVICE_ATTRIBUTES);attr->VendorID = extension->vid;attr->ProductID = extension->pid;attr->VersionNumber = extension->version;irp->IoStatus.u.Status = rc = STATUS_SUCCESS;irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES);break;}default:{ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode;FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n",code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3);break;}- }
- if (rc != STATUS_PENDING)
IoCompleteRequest( irp, IO_NO_INCREMENT );Do you need to handle asynchronous requests in this function? If not you can remove the if check. For consistency reasons, you should probably remove the space after "(" and before ")" in the IoCompleteRequest call.
- return rc;
+}
NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) { static const WCHAR udevW[] = {'\','D','r','i','v','e','r','\','U','D','E','V',0};
-
Aric Stewart -
Sebastian Lackner