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
>
> 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;