[7/13](resend)hidclass.sys: Implement IRP_MJ_READ for HID Devices

[Andrew Eikum]

On Wed, Sep 02, 2015 at 07:02:11AM -0500, Aric Stewart wrote:
> diff --git a/dlls/hidclass.sys/buffer.c b/dlls/hidclass.sys/buffer.c
> index 2561235..6f09393 100644
> --- a/dlls/hidclass.sys/buffer.c
> +++ b/dlls/hidclass.sys/buffer.c
> @@ -80,6 +80,39 @@ void RingBuffer_Destroy(struct ReportRingBuffer *ring)
>      HeapFree(GetProcessHeap(), 0, ring);
>  }
>  
> +UINT RingBuffer_GetBufferSize(struct ReportRingBuffer *ring)
> +{
> +    return ring->buffer_size;
> +}
> +
> +void RingBuffer_Read(struct ReportRingBuffer *ring, UINT index, void *output, UINT *size)
> +{
> +    void *ret = NULL;
> +
> +    EnterCriticalSection(&ring->lock);
> +    if (index >= ring->pointer_alloc || ring->pointers[index] == 0xffffffff)
> +    {
> +        LeaveCriticalSection(&ring->lock);
> +        *size = 0;
> +        return;
> +    }
> +    if (ring->pointers[index] == ring->end)
> +    {
> +        LeaveCriticalSection(&ring->lock);
> +        *size = 0;
> +    }
> +    else
> +    {
> +        ret = &ring->buffer[ring->pointers[index] * ring->buffer_size];
> +        memcpy(output, ret, ring->buffer_size);

Is it necessary to make a copy of the data in the ring buffer? Seems
like returning a raw pointer would be more efficient. I flipped
through the rest of the patch sequence and all uses of RingBuffer_Read
seemed to be read-only.

> +        ring->pointers[index]++;
> +        if (ring->pointers[index] == ring->size)
> +            ring->pointers[index] = 0;
> +        LeaveCriticalSection(&ring->lock);
> +        *size = ring->buffer_size;
> +    }
> +}
> +
>  UINT RingBuffer_AddPointer(struct ReportRingBuffer *ring)
>  {
>      UINT idx;
> diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
> index a6660db..326c994 100644
> --- a/dlls/hidclass.sys/device.c
> +++ b/dlls/hidclass.sys/device.c
> @@ -36,6 +36,7 @@
>  #include "devguid.h"
>  
>  WINE_DEFAULT_DEBUG_CHANNEL(hid);
> +WINE_DECLARE_DEBUG_CHANNEL(hid_report);
>  
>  static const WCHAR device_name_fmtW[] = {'\\','D','e','v','i','c','e',
>      '\\','H','I','D','#','%','p','&','%','p',0};
> @@ -353,6 +354,48 @@ NTSTATUS WINAPI HID_Device_ioctl(DEVICE_OBJECT *device, IRP *irp)
>      return rc;
>  }
>  
> +NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp)
> +{
> +    HID_XFER_PACKET *packet;
> +    BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
> +    UINT buffer_size = RingBuffer_GetBufferSize(ext->ring_buffer);
> +    NTSTATUS rc = STATUS_SUCCESS;
> +    int ptr = -1;
> +
> +    packet = HeapAlloc(GetProcessHeap(), 0, buffer_size);
> +    ptr = (int)irp->Tail.Overlay.OriginalFileObject->FsContext;
> +
> +    irp->IoStatus.Information = 0;
> +    RingBuffer_Read(ext->ring_buffer, ptr, packet, &buffer_size);
> +
> +    if (buffer_size)
> +    {
> +        IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp );
> +        TRACE_(hid_report)("Got Packet %p %i\n", packet->reportBuffer, packet->reportBufferLen);
> +        if (irpsp->Parameters.Read.Length >= packet->reportBufferLen)
> +        {
> +            memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen);
> +            irp->IoStatus.Information = packet->reportBufferLen;
> +            irp->IoStatus.u.Status = STATUS_SUCCESS;
> +        }
> +        else
> +        {
> +            irp->IoStatus.Information = 0;
> +            irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW;
> +        }
> +        IoCompleteRequest( irp, IO_NO_INCREMENT );
> +    }
> +    else
> +    {
> +        TRACE_(hid_report)("Queue irp\n");
> +        InsertTailList(&ext->irp_queue, &irp->Tail.Overlay.ListEntry);
> +        rc = STATUS_PENDING;
> +    }
> +    HeapFree(GetProcessHeap(), 0, packet);
> +
> +    return rc;
> +}
> +
>  NTSTATUS WINAPI HID_Device_create(DEVICE_OBJECT *device, IRP *irp)
>  {
>      BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
> diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h
> index c9bc338..34ec241 100644
> --- a/dlls/hidclass.sys/hid.h
> +++ b/dlls/hidclass.sys/hid.h
> @@ -58,6 +58,8 @@ typedef struct _BASE_DEVICE_EXTENSTION {
>  
>  UINT RingBuffer_AddPointer(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
>  void RingBuffer_RemovePointer(struct ReportRingBuffer *ring, UINT index) DECLSPEC_HIDDEN;
> +void RingBuffer_Read(struct ReportRingBuffer *buffer, UINT index, void *output, UINT *size) DECLSPEC_HIDDEN;
> +UINT RingBuffer_GetBufferSize(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
>  void RingBuffer_Destroy(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
>  struct ReportRingBuffer* RingBuffer_Create(UINT buffer_size) DECLSPEC_HIDDEN;
>  
> @@ -81,6 +83,7 @@ NTSTATUS HID_LinkDevice(DEVICE_OBJECT *device, LPCWSTR serial, LPCWSTR index) DE
>  void HID_DeleteDevice(HID_MINIDRIVER_REGISTRATION *driver, DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
>  
>  NTSTATUS WINAPI HID_Device_ioctl(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
> +NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
>  NTSTATUS WINAPI HID_Device_create(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
>  NTSTATUS WINAPI HID_Device_close(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
>  
> diff --git a/dlls/hidclass.sys/main.c b/dlls/hidclass.sys/main.c
> index 1e0e802..4be4409 100644
> --- a/dlls/hidclass.sys/main.c
> +++ b/dlls/hidclass.sys/main.c
> @@ -69,6 +69,7 @@ NTSTATUS WINAPI HidRegisterMinidriver(HID_MINIDRIVER_REGISTRATION *registration)
>      registration->DriverObject->DriverUnload = UnloadDriver;
>  
>      registration->DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = HID_Device_ioctl;
> +    registration->DriverObject->MajorFunction[IRP_MJ_READ] = HID_Device_read;
>      registration->DriverObject->MajorFunction[IRP_MJ_CREATE] = HID_Device_create;
>      registration->DriverObject->MajorFunction[IRP_MJ_CLOSE] = HID_Device_close;
>  
> 

>