[PATCH 0/1] MR11204: hidclass: Return the full report length from IOCTL_HID_WRITE_REPORT.

[Thomas Portal (＠OursCodeur)]

### What

WriteFile on a numbered HID output report returns one byte less than the buffer the application submitted. hidclass subtracts the report ID byte from the transfer length on the IOCTL_HID_WRITE_REPORT path (`params->padding = 1 - offset`). Windows returns the full length with the report ID byte included.

### Why it matters

An application that watches the WriteFile return value to advance an upload never makes progress. The Elgato Stream Deck desktop app hits this: its per-key image upload resends the first page forever because the count it gets back is always one short, so the keys never render. Removing the subtraction lets the upload finish and the keys draw.

### How I checked the Windows behaviour

A small Win32 probe writes an OutputReportByteLength buffer to the device and reads back the count. On Windows OutputReportByteLength is 1024 and WriteFile returns 1024. hidapi uses the same convention: hid_write returns report length + 1 (the report ID byte) for a numbered report.

### The test

test_write_file already asserts the Windows value (a numbered report returns report_len), and it passes on Windows. It also passed on Wine, for the wrong reason. The mock bus driver returns report_len + 1, and the hidclass subtraction cancelled the extra byte. A real minidriver returns the bytes it wrote, which is report_len, so nothing cancels the subtraction and the WriteFile result is short by one.

This sets the mock's returned length to report_len, matching what a real bus reports, and drops the padding. The asserted WriteFile results do not change (report_len for both numbered and unnumbered reports). Only the mock's intermediate value and the hidclass compensation change.

The hid test passes with both changes and no failures. Reverting the hidclass change on its own turns test_write_file red, so the test guards the behaviour directly.

For history: 94e5945102 (2021) introduced the subtraction, and a7d67c1c90 (2022) carried it forward as the padding field during the async rewrite.

Signed-off-by: Thomas Portal [e-mail]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11204

[PATCH 1/1] hidclass: Return the full report length from IOCTL_HID_WRITE_REPORT.

[Thomas Portal]

From: Thomas Portal <[e-mail]>

Writing a numbered HID output report with WriteFile returns one byte less
than the application passed in. hid_device_xfer_report subtracts the
report ID byte from the completed transfer length. Windows returns the
full length with the report ID byte included, so an application that
follows the returned count to drive an upload stalls when the count comes
up short. The Elgato Stream Deck image upload does exactly that.

The dinput test did not catch this. Its mock bus driver returns a transfer
length of report_len + 1, and the subtraction cancelled the extra byte. A
real minidriver such as winebus returns the number of bytes written, which
is the full report length, so against a real device the subtraction leaves
WriteFile one byte short.

Drop the padding and have the mock return a realistic transfer length.

Signed-off-by: Thomas Portal <[e-mail]>
---
 dlls/dinput/tests/hid.c    | 14 +++-----------
 dlls/hidclass.sys/device.c |  4 ----
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c
index 7c42ac54f74..b7b743009d7 100644
--- a/dlls/dinput/tests/hid.c
+++ b/dlls/dinput/tests/hid.c
@@ -1427,7 +1427,7 @@ static void test_write_file( HANDLE file, int report_id, ULONG report_len )
         .report_id = report_id,
         .report_len = report_len - (report_id ? 0 : 1),
         .report_buf = {report_id ? report_id : 0xcd,0xcd,0xcd,0xcd,0xcd},
-        .ret_length = 3,
+        .ret_length = report_len,
         .ret_status = STATUS_SUCCESS,
     };
 
@@ -1463,16 +1463,8 @@ static void test_write_file( HANDLE file, int report_id, ULONG report_len )
         ret = WriteFile( file, report, report_len, &length, NULL );
     }
 
-    if (report_id)
-    {
-        ok( ret, "WriteFile failed, last error %lu\n", GetLastError() );
-        ok( length == 2, "WriteFile wrote %lu\n", length );
-    }
-    else
-    {
-        ok( ret, "WriteFile failed, last error %lu\n", GetLastError() );
-        ok( length == 3, "WriteFile wrote %lu\n", length );
-    }
+    ok( ret, "WriteFile failed, last error %lu\n", GetLastError() );
+    ok( length == report_len, "WriteFile wrote %lu\n", length );
 
     set_hid_expect( file, NULL, 0 );
 }
diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
index 5b7a675db7f..5624fd08e24 100644
--- a/dlls/hidclass.sys/device.c
+++ b/dlls/hidclass.sys/device.c
@@ -416,7 +416,6 @@ static const WCHAR *find_device_string( const WCHAR *device_id, ULONG index )
 struct completion_params
 {
     HID_XFER_PACKET packet;
-    ULONG padding;
     IRP *irp;
 };
 
@@ -428,7 +427,6 @@ static NTSTATUS CALLBACK xfer_completion( DEVICE_OBJECT *device, IRP *irp, void
     TRACE( "device %p, irp %p, context %p\n", device, irp, context );
 
     orig_irp->IoStatus = irp->IoStatus;
-    orig_irp->IoStatus.Information -= params->padding;
     IoCompleteRequest( orig_irp, IO_NO_INCREMENT );
 
     free( params );
@@ -498,8 +496,6 @@ static NTSTATUS hid_device_xfer_report( struct phys_device *pdo, ULONG code, IRP
                                              sizeof(params->packet), TRUE, NULL, NULL );
         break;
     case IOCTL_HID_WRITE_REPORT:
-        params->padding = 1 - offset;
-        /* fallthrough */
     case IOCTL_HID_SET_FEATURE:
     case IOCTL_HID_SET_OUTPUT_REPORT:
         params->packet.reportBufferLen = report_len - offset;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/11204

[Thomas Portal (＠OursCodeur)]

Dropping the probe I built there for reference, if anyone's curious about how I've made sure I'm not breaking things :grinning:

```
/* deck_writefile_probe.c


 * Build (MinGW):  x86_64-w64-mingw32-gcc deck_writefile_probe.c -o deck_writefile_probe.exe -lhid -lsetupapi
 * Run on the Windows partition with the Elgato Stream Deck software CLOSED.
 * Decides 1024 (option A, Wine padding is the bug) vs 1023 (option B, stall has another cause).
 */
#include <windows.h>
#include <setupapi.h>
#include <hidsdi.h>
#include <stdio.h>

#define DECK_VID 0x0FD9
#define DECK_PID 0x0063  /* Stream Deck Mini */

int main(void)
{
    GUID hidGuid;
    HidD_GetHidGuid(&hidGuid);

    HDEVINFO devInfo = SetupDiGetClassDevsW(&hidGuid, NULL, NULL,
                                            DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
    if (devInfo == INVALID_HANDLE_VALUE) { printf("SetupDiGetClassDevs failed\n"); return 1; }

    SP_DEVICE_INTERFACE_DATA ifData = { .cbSize = sizeof(ifData) };
    HANDLE deck = INVALID_HANDLE_VALUE;
    USHORT outLen = 0;

    for (DWORD i = 0; SetupDiEnumDeviceInterfaces(devInfo, NULL, &hidGuid, i, &ifData); i++)
    {
        DWORD need = 0;
        SetupDiGetDeviceInterfaceDetailW(devInfo, &ifData, NULL, 0, &need, NULL);
        SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail = malloc(need);
        if (!detail) continue;
        detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W);
        if (!SetupDiGetDeviceInterfaceDetailW(devInfo, &ifData, detail, need, NULL, NULL))
        { free(detail); continue; }

        HANDLE h = CreateFileW(detail->DevicePath, GENERIC_READ | GENERIC_WRITE,
                               FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
                               OPEN_EXISTING, 0, NULL);
        free(detail);
        if (h == INVALID_HANDLE_VALUE) continue;

        HIDD_ATTRIBUTES attrs = { .Size = sizeof(attrs) };
        if (HidD_GetAttributes(h, &attrs) &&
            attrs.VendorID == DECK_VID && attrs.ProductID == DECK_PID)
        {
            PHIDP_PREPARSED_DATA pp = NULL;
            if (HidD_GetPreparsedData(h, &pp)) {
                HIDP_CAPS caps;
                if (HidP_GetCaps(pp, &caps) == HIDP_STATUS_SUCCESS)
                    outLen = caps.OutputReportByteLength;
                HidD_FreePreparsedData(pp);
            }
            deck = h;
            break;
        }
        CloseHandle(h);
    }
    SetupDiDestroyDeviceInfoList(devInfo);

    if (deck == INVALID_HANDLE_VALUE) { printf("Stream Deck Mini (VID %04X PID %04X) not found\n", DECK_VID, DECK_PID); return 1; }
    printf("OutputReportByteLength = %u\n", outLen);
    if (outLen == 0) outLen = 1024;

    /* Build a numbered output report: byte[0] = report id 0x02, rest benign. */
    BYTE *buf = calloc(1, outLen);
    if (!buf) { CloseHandle(deck); return 1; }
    buf[0] = 0x02;          /* numbered report id */
    /* byte[1]=command, byte[2]=page index (0), byte[5]=key index - shape per Mini HID;
     * exact image payload is irrelevant: we only read back the WriteFile count. */

    DWORD written = 0;
    SetLastError(0);
    BOOL ok = WriteFile(deck, buf, outLen, &written, NULL);
    DWORD err = GetLastError();

    printf("WriteFile ok=%d written=%lu lastError=%lu\n", ok, written, err);
    if (ok && written == outLen)
        printf("VERDICT: RETURNED %u (== OutputReportByteLength) -> option (A): Wine padding=1 is the bug, drop-padding is correct.\n", (unsigned)outLen);
    else if (ok && written == (DWORD)(outLen - 1))
        printf("VERDICT: RETURNED %u (== OutputReportByteLength-1) -> option (B): Deck stall is NOT the return count; reinvestigate.\n", (unsigned)(outLen - 1));
    else
        printf("VERDICT: inconclusive (ok=%d written=%lu err=%lu) - ensure Stream Deck software is closed.\n", ok, written, err);

    free(buf);
    CloseHandle(deck);
    return 0;
}
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11204#note_143683