[PATCH v3 0/1] MR11204: hidclass: Return the full report length from IOCTL_HID_WRITE_REPORT.
### 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 portal.thomas@protonmail.com -- v3: hidclass: Return the full report length from IOCTL_HID_WRITE_REPORT. https://gitlab.winehq.org/wine/wine/-/merge_requests/11204
From: Thomas Portal <portal.thomas@protonmail.com> WriteFile on a HID output report returns the output report length on Windows, regardless of the transfer length the minidriver reports. hid_device_xfer_report instead forwarded the minidriver count after subtracting the report ID byte, so on a numbered report WriteFile came up one byte short. An application that follows the returned count to drive an upload then stalls; the Elgato Stream Deck image upload does exactly that. Report the output report length on completion instead of adjusting the minidriver count. The dinput test masked this: its mock bus driver returned report_len + 1 and the subtraction cancelled the extra byte. Have the mock return an unrelated length so the test checks that hidclass returns the output report length and not the count the minidriver reported. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59891 Signed-off-by: Thomas Portal <portal.thomas@protonmail.com> --- dlls/dinput/tests/hid.c | 14 +++----------- dlls/hidclass.sys/device.c | 7 ++++--- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index 7c42ac54f74..dfd35156f22 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 = 5, .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..4a28ec01344 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -416,7 +416,7 @@ static const WCHAR *find_device_string( const WCHAR *device_id, ULONG index ) struct completion_params { HID_XFER_PACKET packet; - ULONG padding; + ULONG report_len; IRP *irp; }; @@ -428,7 +428,7 @@ 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; + if (params->report_len) orig_irp->IoStatus.Information = params->report_len; IoCompleteRequest( orig_irp, IO_NO_INCREMENT ); free( params ); @@ -498,7 +498,8 @@ 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; + /* WriteFile returns the output report length, not the minidriver count */ + params->report_len = report_len; /* fallthrough */ case IOCTL_HID_SET_FEATURE: case IOCTL_HID_SET_OUTPUT_REPORT: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11204
On Thu Jun 25 07:12:53 2026 +0000, Thomas Portal wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/11204/diffs?diff_id=277821&start_sha=149057d6ab2d16e318162a77435e3cd1fbc3accb#1d6f44518922f013f44583f62c641b5e4316857e_1430_1430) Woops, you're right thanks. Reworked :
hidclass now reports the output report length on the WRITE_REPORT completion rather than forwarding the minidriver's count. The test keeps an unrelated .ret_length = 5 and still asserts length == report_len, so it checks the fix-up directly instead of cancelling a subtraction. Verified locally, hid passes with 0 failures, and reverting the hidclass change with .ret_length = 5 in place turns test_write_file red ("WriteFile wrote 5") for both the numbered and unnumbered reports. Thanks for catching that ! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11204#note_144196
participants (2)
-
Thomas Portal -
Thomas Portal (@OursCodeur)