[PATCH] hidclass.sys: When processing reads fill all the buffers

[Sebastian Lackner]

On 26.01.2017 13:57, Aric Stewart wrote:
> This is a follow-up fix for continuing work on bug 42225
> 
> Signed-off-by: Aric Stewart <aric at codeweavers.com>
> ---
>  dlls/hidclass.sys/buffer.c | 34 +++++++++++++++++++++++++++++++++-
>  dlls/hidclass.sys/device.c |  2 +-
>  dlls/hidclass.sys/hid.h    |  3 ++-
>  3 files changed, 36 insertions(+), 3 deletions(-)
> 
> 
> 
> 0001-hidclass.sys-When-processing-reads-fill-all-the-buffer.txt
> 
> 
> diff --git a/dlls/hidclass.sys/buffer.c b/dlls/hidclass.sys/buffer.c
> index 0b29f97b69..41bbb8e241 100644
> --- a/dlls/hidclass.sys/buffer.c
> +++ b/dlls/hidclass.sys/buffer.c
> @@ -127,7 +127,7 @@ NTSTATUS RingBuffer_SetSize(struct ReportRingBuffer *ring, UINT size)
>      return STATUS_SUCCESS;
>  }
>  
> -void RingBuffer_Read(struct ReportRingBuffer *ring, UINT index, void *output, UINT *size)
> +void RingBuffer_ReadNew(struct ReportRingBuffer *ring, UINT index, void *output, UINT *size)
>  {
>      void *ret = NULL;
>  
> @@ -155,6 +155,38 @@ void RingBuffer_Read(struct ReportRingBuffer *ring, UINT index, void *output, UI
>      }
>  }
>  
> +void RingBuffer_Read(struct ReportRingBuffer *ring, UINT index, void *output, UINT *size)
> +{
> +    int pointer = ring->pointers[index];
> +    void *ret = NULL;
> +
> +    EnterCriticalSection(&ring->lock);
> +    if (index >= ring->pointer_alloc || ring->pointers[index] == POINTER_UNUSED
> +        || ring->end == ring->start)
> +    {
> +        LeaveCriticalSection(&ring->lock);
> +        *size = 0;
> +        return;
> +    }
> +
> +    if (pointer == ring->end)
> +        pointer--;
> +
> +    if (pointer < 0)
> +        pointer = ring->size - 1;
> +
> +    ret = &ring->buffer[pointer * ring->buffer_size];
> +    memcpy(output, ret, ring->buffer_size);
> +    if (pointer == ring->pointers[index])
> +    {
> +        ring->pointers[index]++;
> +        if (ring->pointers[index] == ring->size)
> +            ring->pointers[index] = 0;
> +    }
> +    LeaveCriticalSection(&ring->lock);
> +    *size = ring->buffer_size;

Have you been able to reproduce a similar behavior on Windows? The new code
looks very suspicious and I'm not sure if this is the right approach to fix
it. Also please note that this will effectively disable any asynchronous
Read requests - HID_Device_read() will always use the first code path when
there is at least one package in the ring buffer and never return
STATUS_PENDING.

> +}
> +
>  UINT RingBuffer_AddPointer(struct ReportRingBuffer *ring)
>  {
>      UINT idx;
> diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
> index b241016e72..d6a0a358e3 100644
> --- a/dlls/hidclass.sys/device.c
> +++ b/dlls/hidclass.sys/device.c
> @@ -662,7 +662,7 @@ NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp)
>      ptr = PtrToUlong( irp->Tail.Overlay.OriginalFileObject->FsContext );
>  
>      irp->IoStatus.Information = 0;
> -    RingBuffer_Read(ext->ring_buffer, ptr, packet, &buffer_size);
> +    RingBuffer_ReadNew(ext->ring_buffer, ptr, packet, &buffer_size);
>  
>      if (buffer_size)
>      {
> diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h
> index adc547cc43..1829319ccf 100644
> --- a/dlls/hidclass.sys/hid.h
> +++ b/dlls/hidclass.sys/hid.h
> @@ -62,7 +62,8 @@ typedef struct _BASE_DEVICE_EXTENSION {
>  void RingBuffer_Write(struct ReportRingBuffer *buffer, void *data) DECLSPEC_HIDDEN;
>  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;
> +void RingBuffer_Read(struct ReportRingBuffer *ring, UINT index, void *output, UINT *size) DECLSPEC_HIDDEN;
> +void RingBuffer_ReadNew(struct ReportRingBuffer *buffer, UINT index, void *output, UINT *size) DECLSPEC_HIDDEN;
>  UINT RingBuffer_GetBufferSize(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
>  UINT RingBuffer_GetSize(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
>  void RingBuffer_Destroy(struct ReportRingBuffer *buffer) DECLSPEC_HIDDEN;
> 
> 
>