Re: [PATCH 4/4] winehid: Implement handing internal ioctls

[Sebastian Lackner]

On 12.10.2016 12:31, Aric Stewart wrote:
> Mostly by passing them to the bus driver where platform specific actions
> happen
>
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
>  dlls/winehid.sys/main.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> 0004-winehid-Implement-handing-internal-ioctls.txt
>
> diff --git a/dlls/winehid.sys/main.c b/dlls/winehid.sys/main.c
> index e8c91bb..7c17321 100644
> --- a/dlls/winehid.sys/main.c
> +++ b/dlls/winehid.sys/main.c
> @@ -25,6 +25,7 @@
>  #include "windef.h"
>  #include "winbase.h"
>  #include "winternl.h"
> +#include "winioctl.h"
>  #include "ddk/wdm.h"
>  #include "ddk/hidport.h"
>  #include "wine/debug.h"
> @@ -33,6 +34,52 @@ WINE_DEFAULT_DEBUG_CHANNEL(hid_minidriver);
>
> static HID_MINIDRIVER_REGISTRATION registration;
>
> +static NTSTATUS WINAPI internal_ioctl(DEVICE_OBJECT *device, IRP *irp)
> +{
> +    IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp);
> +

Some tracing wouldn't hurt here.
> +    irp->IoStatus.Information = 0;
> +
> +    switch (irpsp->Parameters.DeviceIoControl.IoControlCode)
> +    {
> +        case IOCTL_GET_PHYSICAL_DESCRIPTOR:
> +            TRACE("IOCTL_GET_PHYSICAL_DESCRIPTOR\n");
> +            break;
> +        case IOCTL_HID_ACTIVATE_DEVICE:
> +            TRACE("IOCTL_HID_ACTIVATE_DEVICE\n");
> +            break;
> +        case IOCTL_HID_DEACTIVATE_DEVICE:
> +            TRACE("IOCTL_HID_DEACTIVATE_DEVICE\n");
> +            break;
> +        case IOCTL_HID_GET_INDEXED_STRING:
> +            TRACE("IOCTL_HID_GET_INDEXED_STRING\n");
> +            break;

I assume there are no plans (yet) to implement those. Shouldn't they be marked as FIXME then?
> +        case IOCTL_HID_GET_DEVICE_ATTRIBUTES:
> +        case IOCTL_HID_GET_DEVICE_DESCRIPTOR:
> +        case IOCTL_HID_GET_REPORT_DESCRIPTOR:
> +        case IOCTL_HID_GET_STRING:
> +        case IOCTL_HID_GET_INPUT_REPORT:
> +        case IOCTL_HID_READ_REPORT:
> +        case IOCTL_HID_SET_OUTPUT_REPORT:
> +        case IOCTL_HID_WRITE_REPORT:
> +        case IOCTL_HID_GET_FEATURE:
> +        case IOCTL_HID_SET_FEATURE:
> +        {
> +            /* All these are handled by the lower level driver */
> +            IoSkipCurrentIrpStackLocation(irp);
> +            return IoCallDriver(((HID_DEVICE_EXTENSION*)device->DeviceExtension)->NextDeviceObject, irp);
> +        }
> +        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);
> +        }
> +    }
> +    IoCompleteRequest( irp, IO_NO_INCREMENT );
> +    return irp->IoStatus.u.Status;

After IoCompleteRequest the IRP struct should not be accessed anymore.
> +}
> +
>  static NTSTATUS WINAPI add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *device)
>  {
>      TRACE("\n");
> @@ -44,6 +91,7 @@ NTSTATUS WINAPI DriverEntry(DRIVER_OBJECT *driver, UNICODE_STRING *path)
>  {
>      TRACE("%s\n", debugstr_w(path->Buffer));
>
> +    driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = internal_ioctl;
>      driver->DriverExtension->AddDevice = add_device;
>
> registration.DriverObject = driver;