Properly determine libusb read size for large reports (Fixes #274) #728
No reviewers
Labels
No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-libusb/hidapi!728
Loading…
Reference in a new issue
No description provided.
Delete branch "sudobash1/libusb_input_size"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
wMaxPacketSizeare delivered whole.wMaxPacketSizeas the minimum buffer size for devices that pad short reports, and fall back to it when a descriptor cannot be read or parsed safely.hid_get_report_descriptor()._real.rpt_descfixtures plus malformed, overflow, Report ID, Push/Pop, long-item, and boundary cases.Verification
-Wall -Wextra -pedantic -Werror: 26 runnable tests passed; hardware-only DeviceIO test skipped.-Wall -Wextra -Werror: 26 runnable tests passed; hardware-only DeviceIO test skipped.HIDAPI_WITH_TESTS=ONand registers all 26 libusb tests.Fixes #274
Related to #796
Related to #833
Assisted-by: codex:gpt-5.6-sol
Assisted-by: claude-code:claude-fable-5
PR updated to remove different signedness comparison compiler warning when compiling with
-Wall.Thank you for finally taking care of this longstanding bug.
Thanks for this, I'll perform a thorrow review a bit later.
@Be-ing can you confirm it fixes the issue with long reports too?
lets make this conversion earlier, and store the value already in bytes
This function seems to be the functional the same as
InputReportByteLengthfrom https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/hidpi/ns-hidpi-_hidp_caps on Windows.We have a lot of test data for the Windows unit test, which stores the result of this function:
github.com/libusb/hidapi@0ab6c14264/windows/test/data/045E_02FF_0005_0001.pp_data (L21)together with the real ReportDescriptor https://github.com/libusb/hidapi/blob/master/windows/test/data/045E_02FF_0005_0001_real.rpt_desc
What do you think about creating a unit test for the new
get_max_input_sizefunction using the_real.rpt_descfiles as input and compare the result with the value stored in the.pp_data.Nice! That does look like a great dataset. I'll see if I can figure a clean way to make a unit test, and see how it fares with that data.
Currently the
get_max_input_sizeis astaticfunction, but I need a way to use it externally in the unit test. It seems that the windows library has some extensions prefixed withwinapi. Should I rename the get max function tohid_libusbapi_get_max_input_report_sizeand mark itHID_API_EXPORT_CALL?I will also need to change how it gets the report descriptor, but that won't be a problem.
Making it with
HID_API_EXPORT_CALLwould make it a public (at least on binary level) function, which is undesirable, if it is required for tests only. Maybe export it only when building unit-tests etc. or try to avoid it by using static linking or smth else (or have it in an internal header file?)Have a look at the Windows unit test ( https://github.com/libusb/hidapi/tree/master/windows/test ) for the report descriptor reconstructor. There we had exactly the same challenges, hid_winapi_descriptor_reconstruct_pp_data was the internal function to test there.
@JoergAtGithub I did actually look at
hid_winapi_descriptor_reconstruct_pp_data, and it is marked asHID_API_EXPORT_CALLin the header file:github.com/libusb/hidapi@0ab6c14264/windows/hidapi_winapi.h (L68)Although, now that I am looking, it is not marked in the C file:
github.com/libusb/hidapi@0ab6c14264/windows/hidapi_descriptor_reconstruct.c (L181)WRT @Youw's suggestion. I could simplify the function a bit so that it doesn't depend on anything in
hid.cand then move it tohid_max_input_report.h. If that is acceptable, it is probably the easiest solution.Right - that is a public API function. That is also the reason why there is a
winapiprefix in the name.Not required, if it is marked in the header.
Yes, sounds like simples solution for now. Go for it.
I have a test, but it isn't passing. It could be that I am misunderstanding the Windows HID structures. I have only a vague understanding of them.
For example, in the
046A_0011_0006_0001test data, it gives a max input size of 9 bytes:github.com/libusb/hidapi@0ab6c14264/windows/test/data/046A_0011_0006_0001.pp_data (L16-L19)But two input caps (
cap[0]andcap[1]) are:github.com/libusb/hidapi@0ab6c14264/windows/test/data/046A_0011_0006_0001.pp_data (L37-L38)github.com/libusb/hidapi@0ab6c14264/windows/test/data/046A_0011_0006_0001.pp_data (L83-L84)According to the documentation I read, the report size is just
BitSize * ReportCount(plus a byte for the report number I assume).This gives report sizes of
(1*8)/8 + 1 = 2and(8*6)/8 + 1 = 7. So I don't know where theReportByteLength = 9value is coming from.Do either of you know what I am missing here?
BTW, there is a typo here.
@Youw and @JoergAtGithub.
github.com/libusb/hidapi@0ab6c14264/windows/test/data/046A_0011_0006_0001.pp_data (L4)I put
046A_0011_0006_0001_real.rpt_descinto https://eleccelerator.com/usbdescreqparser/ and got:The InputReport contains:
This sums to 8byte, but
InputReportByteLengthis defined as follows:Specifies the maximum size, in bytes, of all the input reports. Includes the report ID, which is prepended to the report data. If report ID is not used, the ID value is zero.
So we've to add a byte for the ReportID and are at 9 bytes.
There is something missing, as this is autogenerated by pp_data_dump, I guess there was something in the manufacturer_string , that pp_data_dump couldn't handle. Could you please open a dedicated issue for this, as this is unrelated to this PR.
This would mean a corrupt ReportDescriptor
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)You need to parse the
_real.rpt_desc, not the_expected.rpt_desc. The real is what is dumped from the device, and the expected is what the Windows ReportDescriptor-Reconstructor generates out of the .pp_data.@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)I see. I assumed that the
realandexpectedshould contain equivalent reports.The
_real.rpt_descfiles do not follow a consistent format, so should I parse them manually (should be pretty doable with a bit of regex) into new files and add them to the repo?I wasn't consistent with whether I was using
size_torssize_t. In my last commit, I settled onsize_t, but perhaps I should change it back tossize_tso that we can more clearly differentiate between errors and a zero value (if there are no feature reports, it will return 0, which is not an error).Thanks @JoergAtGithub for walking me through that. I was misreading the descriptor.
I have added tests for libusb using the same data as the windows tests. And I extended the functionality of the libusb method to be able to calculate the maximum output and feature report sizes as well. This has no functional use currently, but it lets us run three times as many tests since the pp_data files have all three max sizes available.
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)Maybe you can put them into https://eleccelerator.com/usbdescreqparser/ to unify the format. That makes them also human readable.
Independent of this PR, I think it would generally make sense to store these 3 values in the device structure. On Windows we would have to use the values
InputReportByteLength,OutputReportByteLengthandFeatureReportByteLengthfrom https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/hidpi/ns-hidpi-_hidp_caps . On all other backends we could use your parser to determine them - and with the testcase you proved that they are the same as on Windows.In this way, we were able to programmatically determine the required buffer sizes for read/write operations on all platforms. @Youw What do you think about this?
Makes sense!
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)Good idea.
also need to update builds.yml
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)@ -0,0 +43,4 @@)foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES})set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data")maybe move the test data to
<root>/test_data?I don't have a strong opinion on test implementation (I trust @JoergAtGithub on this one).
libusb implementation seem fine.
Lets make sure it runs with CI on Github Actions and we're good to go here.
Lets continue here: https://github.com/libusb/hidapi/issues/731
This PR does not seem to work.
Test device is the same as the one used in Issue #274. The FW is a mod of Jan Axelson's FX2HID example and codes are included in the following #274 comment.
I can reproduce the issue reported in #274 with hidapi git libusb backend, hidraw backend is okay.
With this PR, no change in hidraw backend behavior, which is good.
But the libusb backend fix is not working.
HID Report Descriptor.
hidtest is okay.
If I have time, I'll try to address @JoergAtGithub and @Youw's comments on Monday.
@mcuee I tried feeding the report descriptor that you sent into
get_max_report_sizeand it returned129, which seems correct. I'll try to look into this on Monday as well.I did notice a typo in your test. When you ran
./hidapitester_libusb_pr728, you specified-l 128which should have been-l 129, but I think it should have returned data anyway.Let me try again.
Same problem under FreeBSD 14.1 Release.
This is with a physical machine, Chuwi mini PC, Intel J4125 CPU, 8GB/256GB configuration.
Without this PR, hidapi git has the problem mentioned in #274.
But then somehow this PR does not work.
If I use the original fx2hid example (loop back of two bytes output report and input report), it seems to me this PR works fine. But that just means this PR has no regression.
http://janaxelson.com/hidpage.htm
http://janaxelson.com/files/fx2hid.zip
Just want to make sure the FW works fine, here is the test under Windows (native Windows HID backend).
Original fx2hid FW.
My mod fx2hid FW (128 bytes report).
@Be-ing and @JoergAtGithub
Just wondering if you can carry out the test using your test device with long reports as well. Thanks.
@sudobash1
Just wondering which test device you are using. Thanks.
I am using two test devices, one is a simple EZ-USB FX2LP board which is pretty cheap from many places.

The other is a very old Microchip USB demo board (PIC18F87J50 PIM), not worth buying now.

There are also github action build failure.
I lost track on this one a bit. Can someone remind where do we stand right now? Except for failing CI - are there active change requests/fixes?
It just does not work based on my testing, tested under Linux and FreeBSD using hidapitester. I also tested under Windows (with a minor hack on the hidapitester Makefile and hidapi libusb backend).
@sudobash1
Just wondering what is the test device you are using. Thanks. Please share the test results as well. I'd like to see if I can use similar test env as yours.
Sorry for going dark on this.
@mcuee I made these changes to support a Chameleon 20 braille display in USB HID mode. Long story short I needed it to work with an embedded Linux system that is stuck on the 4.x kernel which predates HIDIOCSINPUT. So I switched to the libusb backend and ran into the report size issue. I also tested the libusb changes on a Fedora 39 machine.
I am getting a EZ-USB FX2LP board, so if you point me towards the firmware, I can try out that device. (Much cheaper than the Chameleon 😁)
For me this grew out of a project for my work (the Chameleon with embedded Linux). Things have gotten pretty hectic, but I'm putting a reminder on my calendar to review this again in June.
@sudobash1
FW binary and source codes are here.
It is a quick minor modification of Jan Axelson's FX2HID example from here.
http://janaxelson.com/hidpage.htm
http://janaxelson.com/files/fx2hid.zip
There may be a bit of challenge if you want to rebuild the source code as it may require a full version of Keil C51 compiler. I happen to have access to a pretty old version of Keil C51 compiler at work. I am not so sure about the efforts to port the code to use the free sdcc toolchain. It may be possible because of the nice sdcc based libraries.
Jan's HID page has some other FW as well, for example, for Microchip USB PIC18. I was able to use that as a base for my other HIDAPI testing but I have not figured out how to increase the HID report size above 64 bytes with that FW. I have the PIC18F87J50 USB PIM (free tool from Microchip many years ago).
More testing device discussions:
Update with the main git branch to trigger another CI run.
@aokblast
Would you please run your test device against this PR for hidspi libusb backend under your FreeBSD (as well as other OS you have access to)? Thanks.
@cederom and @dl8dtl
Since you two are the FreeBSD power user I know of, please help to check if you can give this PR a try.
FreeBSD HIDAPI is now based on libusb, so it is affected by the Issue #274.
#274 has an impact on avrdude as well.
@mcuee builds okay, could you please provide test steps, its a long thread :-P
Great.
Test steps: using the normal libusb backend of HIDAPI and this PR.
Test with your normal generic USB HID device using hidapi's built-in hidtest and hidapitester. This should be okay.
https://github.com/todbot/hidapitester
Test with USB Full Speed HID device with >64 Bytes HID report size. This may fail.
Test with USB High Speed HID device with >64 Bytes HID report side. This may fail.
I have mentioned my test device in the comments above (EZ-USB FX2LP) but you can use other test device if you can.
@mcuee
Thanks. I finally got my EZ-USB FX2LP dev board from ebay. I've never worked with these before. How do I flash the hex file onto them? This is as far as I got:
No errors, but nothing seems to happen. No hid device shows up & there is no USB re-enumeration. (Fedora 42, Linux kernel 6.14.11)
I also ordered EZ-USB FX2LP will arrive next week will report back test results :-)
You are almost there. Just need to add
-t fx2lpin the command line to specify the type to be EZ-USB FX2LP.FX --> EZ-USB FX
FX2 --> EZ-USB FX2
FX2LP --> EZ-USB FX2LP
FX3 --> EZ-USB FX3
@sudobash1
Fedora 42's fxload (kind of original version of fxload) is a bit different and it does not support FX3. But it does support FX2LP.
Interestingly Arch Linux AUR fxload is actually packing libusb project's fxload example.
https://github.com/libusb/libusb/tree/master/examples
https://aur.archlinux.org/packages/fxload
Git Clone URL: https://aur.archlinux.org/fxload.git (read-only, click to copy)
Package Base: fxload
Description: fxload firmware loader. Used with udev or devfs/hotplug.
Upstream URL: http://libusb.info/
Interestingly there is a Fedora request to package libusb's fxload example (probably as libusb-fxload) but it is not implemented in the end.
https://bugzilla.redhat.com/show_bug.cgi?id=1907078
Take note libusb's fxload does not support EEPROM. So the suggeston from @jwrdegoede is good in the above discussion. libusb's fxload example cannot replace the original fxload, it is better to have both packages in Fedora.
Initial commit in libusb project
github.com/libusb/libusb@05975333c5FX3 support:
github.com/libusb/libusb@b74b7f7390@mcuee Thanks, I have it running now. I am experiencing the same things you are. I added a quick
printftolibusb/hid.cto check thedev->max_input_report_size. It reports 129:So, this PR is computing what I would have expected for the max input report size from the device descriptor. But I notice with hidraw, it is only reading 128 bytes (not 129). So as a quick kludge test, I removed the
+ 1from the report size, and it works:It looks like the input report doesn't use a report number, so there are only 128 bytes sent. Right now, I'm guessing that the issue is something to do with the fact that 128 is an even multiple of the packet size, so USB isn't sending any short packet.
I've got a cold right now, so my head has been a bit fuzzy. I will have to revisit this at a later time with a clearer head.
tl;dr
It looks like the code in this PR to parse the report descriptor is working, but libusb isn't reading a 128 byte message when it expected a 129-byte one.
Anybody have more insight into this?
Cheers! 🍻
For interupt transfers, which are send without requests from the device to the computer, the ReportId byte ist only prepend to the report data, if the device uses ReportIDs.
Therefore you need to determine from the report descriptor, if the device uses ReportIDs. If anywhere in the report descriptor REPORT_ID occurs, you need, to increase all transfer sizes by one byte for the ReportID.
@sudobash1
I think you are close to find the root cause of the issue.
https://github.com/sudobash1/hidapi/blob/libusb_input_size/libusb/hid.c
It is kind of a complicated topic. Please refer to the following discussion.
Good comments in that issue by @JoergAtGithub
You can also refer to my comment.
Thank you @JoergAtGithub and @mcuee. That is clearly the issue. I reviewed the HID spec again and indeed it says:
The Windows behavior when there is no ReportID was tripping me up.
I've pushed a simple commit to address this, and the FX2LP 128-byte report test works fine for me now.
The tests also needed to be fixed for the cases when there is no ReportID (
ReportId = 0x00in thepp_datafile).Am I forgetting anything, or does that finish everything outstanding on this PR (provided CI completes and @mcuee's & @cederom's tests pass)?
@sudobash1 great news! I will be able to provide soonest feedback around Monday sorry if you are ready before and want to go ahead then go ahead :-) I am waiting for the board and will have to see how to set things up but then I will be able to help you in other cases :-)
Is there any sort of bad-usb device out there that could provide us various test cases from hardware point of view (i.e. simulate bad descriptors, strange transfers, etc)?
@sudobash1
Other than testing, @Youw will need to review the codes and then approve this PR.
But now the first thing is to fix the CI build -- two builds failed because of the same issue.
Oops. I thought I had fixed that. I guess not. I have pushed e2391b6 to address compiler warnings. It compiles cleanly with
-Wall -Wextraon gcc-15 and clang-20 on my Fedora box now. Hopefully all the CI compile now.Yes, CI builds are okay now.
Ah, it looks like there a few comments from @Youw that I need to address too. Most significant is probably updating builds.yml (to make sure the tests are enabled).
I'll look through this on Friday.
It would be good if it could clarify in the comments what exactly the “report size” returned by get_max_report_size comprises.
In other code of mine, I introduced the unofficial term payload-size for the report size without the ReportID-byte. There I add 1 depending on the context where it is used. There I've also a boolean that indicates, if the device uses ReportIDs.
@sudobash1
First test under Windows -- it seems the reported write/read length is not consistent with native API, one byte higher.
Native Windows HID backend-- wrote 129 bytes, read 128 bytes
libusb backend -- wrote 130 bytes, read 129 bytes
But this is getting close. I will test under Linux and FreeBSD later.
Dirty fix to be able to build under Windows.
hidapitester Makefile hack and binaries (against hidapi git and this PR, Windows native backend and libusb backend).
hidapitester_win.zip
Some findings when testing this patch (on Android) - It looks like report of
MAX_SIZEbytes works great now, however, it seems like smaller reports are either slow to be received or sometime even gets discardedSorry for the delay, extremely overloaded, I have the CY7C680 board, working on FreeBSD.
Had to power on board with button pressed to be able to flash firmware:
Now it shows as:
I realised I forgot to update my previously flawed comment - the issue was obviously on my end.
Tested this patch on Android and work like a charm! (ref)
@sudobash1
Just wondering if you have any updates on this one. I think it is pretty close now.
Just update the branch to trigger a new build.
It will be good to move this PR forward. May need more testing and reviews.
@Youw
Just wondering if it is worth asking Github Copilot to review this PR. It seems to be pretty close to be okay.
Coming back to this after dropping it for a while — did a spec/kernel cross-check pass and a code re-read. Leaving hardware testing to those with the devices.
Algorithm validation
get_max_report_sizematches HID 1.11 (§6.2.2.7, §8 for Report ID semantics) and is structurally identical to Linux's report-descriptor parsing indrivers/hid/hid-core.c:hid_add_field():report->size += parser->global.report_size * parser->global.report_count;hid_register_report():if (id != 0) report_enum->numbered = 1;numbered— which is exactly the+ report_id_usedsemantic here.The FX2 128-byte fix is the subtle one: the hang wasn't the size math, it was the unconditional
+1making the buffer a non-multiple ofwMaxPacketSizeand suppressing the libusb short-packet terminator. Good catch on making it conditional on the Report ID tag actually being used.Reusing
windows/test/data/*.pp_datafor a cross-backend parity test is a nice consistency proof.Suggestions before merge
wrote 130 / read 129vs native HIDwrote 129 / read 128) — worth a sentence in the PR body pointing to #731 so users don't re-file it as a regression.Algorithm + spec alignment LGTM on the core change.
Extended version:
Comparison with the spec and other implementations
HID 1.11 spec (§6.2.2.7 & §5.6 / §8): "If a Report ID tag is used anywhere in Report descriptor, all data reports for the device are preceded by a single byte ID field." Report ID = 0 is a placeholder meaning "no report IDs". Reports are sized as Σ(report_count × report_size) bits, rounded up to bytes. The PR's algorithm and the conditional +report_id_used byte match the spec exactly.
Windows (HIDP_CAPS): InputReportByteLength/OutputReportByteLength/FeatureReportByteLength — these always include the Report-ID byte (value 0 when unused). That's the source of the read/write-length platform asymmetry and is exactly what follow-up issue #731 is about. The PR's test case subtracts 1 byte from these values when ReportID == 0x00 to reconcile with the libusb convention (see parse_max_input_report_size in max_input_report_size_test.c).
Linux kernel (drivers/hid/hid-core.c): Uses the same algorithm. hid_add_field() accumulates report->size += parser->global.report_size * parser->global.report_count. In hid_register_report(), if (id != 0) report_enum->numbered = 1;. In hid_report_raw_event, when numbered is true the first data byte is consumed as the ID (cdata++; csize--;). Size is bounded by max_buffer_size - 1 bits when numbered. This is structurally identical to what the PR does — so the PR's approach is directly validated by the reference kernel implementation.
hidapi/linux (hidraw backend): Avoids parsing entirely — delegates to the kernel: read(dev->device_handle, data, length) with user-supplied length, because the kernel has already sized the per-report buffer. That's why hidraw "just works" for the 128-byte FX2 firmware and why the libusb backend is the outlier.
Strengths
Correct algorithm. Matches HID 1.11 and Linux kernel's parsing exactly, including the subtle conditional Report-ID byte.
Good test coverage via reuse of 25 real-world Windows pp_data fixtures — this is a nice cross-backend consistency proof.
Safe fallback to input_ep_max_packet_size when the descriptor is corrupt/unparseable — no regression risk for devices already working.
Fixes the classic "exact multiple of max packet size hangs" libusb gotcha — the conditional +1 was the critical insight; without it devices with payloads that happen to be multiples of 64 would hang forever waiting for a short packet.
Remaining concerns worth flagging on review
Re-read of report descriptor. hidapi_initialize_device now calls hid_get_report_descriptor_libusb during init (new control transfer on every open). This is a second descriptor fetch in addition to any caller-driven hid_get_report_descriptor(). Consider caching.
HID_API_MAX_REPORT_DESCRIPTOR_SIZE buffer on stack (4096 bytes typically). libusb/hid.c:1314 in the diff allocates this on the stack — acceptable, but worth noting for embedded targets.
cur_size reset timing. In the parser, cur_size is reset on each Report ID encounter. For descriptors that interleave INPUT/OUTPUT items between Report ID changes this is correct; just note that the max_size comparison happens only at Report-ID boundaries and at the end — which matches the spec semantics where each Report ID opens a fresh report namespace.
Platform-length asymmetry (mcuee's Windows observation) is intentionally out of scope — tracked in #731. Worth linking from the PR description.
PR description still says "Fixes #274" but #274 has broader asks (detection of USB Full-Speed misconfiguration, speed reporting to app). This PR fixes the "long reports get truncated" aspect; the speed/error-reporting aspect remains open.
Typo in test comment (.dpt_desc → .rpt_desc) in libusb/test/CMakeLists.txt.
Bottom line
The PR is technically sound, aligned with HID 1.11 and the Linux kernel reference, and has been hardware-validated on Linux, FreeBSD and Android. The earlier "doesn't work" reports were a spec-compliance bug in the Report-ID handling that sudobash1 fixed. Primary blockers now are administrative: final Youw approval + merge, plus perhaps addressing the minor concerns above. The broader cross-platform length-consistency question is correctly deferred to #731.
Drafted with Claude Code.
Two minor things in this block:
HID_API_MAX_REPORT_DESCRIPTOR_SIZE) — fine for desktop, may be tight on some embedded targets. Worth heap-allocating to match the rest of the backend.hid_open(one here, and another whenever a caller later invokeshid_get_report_descriptor()). Consider caching the bytes onhid_deviceso both consumers share them.@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)Two typos:
dpt_desc→rpt_desc, andrescriptor→descriptor.This is the result of the review by Claude Opus 4.7 (locally).
1 - totally dismiss, 4KB is fine on any platform where HIDAPI shall be considered to be used
2 - minor
I think this statement is not correct. The USB standard specifies the max read size per speed category. With this PR every read size should work and therefore every USB speed.
@Youw
Nice reviews. Just want to check if I read the comments correctly.
In that case, I think this PR can be merged.
Latest test results with hidapi git.
This PR is OK.
Windows
FreeBSD
Latest results under Linux.
This PR is okay.
@ -0,0 +43,4 @@)foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES})set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data")#796
There is still a number of unresolved comments across the code. Need to either handle or a reason to dismiss, before merge this. (Or move out into separate issue/ticket.
No regression for the following HID device under Linux (with non-Zero Input/Output/Feature reports IDs).
https://github.com/mcuee/picusb/tree/master/lvr_genhid_mod
No regression under Windows either.
Minor modification needs to be used to get hidapitester to build with hidapi git or this PR, using libusb backend.
Using WinUSB driver.
@sudobash1 It would be great if you could address the lost open comments in the code review.
@sudobash1 It would be great if you could address the last remaining comments in this PR.
Addressed in
39e58aeand the follow-up hardening commits. Codex moved the parser into the privatelibusb/hidapi_libusb_report_descriptor.hheader and addedlibusb/test/max_input_report_size_test.c. The test suite now runs against all 25 Windows report-descriptor fixtures, compares computed input/output/feature lengths with the corresponding.pp_data, and includes focused malformed and boundary cases.Generated by Codex (GPT-5.6 Sol).
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)Addressed. The libusb tests now read each
<name>_real.rpt_descfixture—not_expected.rpt_desc—and support the three formats present in the existing data:hid-decodeR:records, normalized0xNNlists, and trailing hexadecimal dumps. Expected report lengths continue to come from the matching.pp_data.Generated by Codex (GPT-5.6 Sol).
After latest fixes from codex - this needs to be tested
Addressed.
get_max_report_size()now returns signedssize_tresults:-1for malformed descriptors,0when the requested report type is absent, and a positive byte count on success. Missing Report Size/Count state, truncation, overflow, invalid Report IDs, and unsafe sizes are rejected, with regression coverage in the parser self-test.Generated by Codex (GPT-5.6 Sol).
Addressed in the current branch. The libusb parser tests are registered through
HIDAPI_WITH_TESTS; the Linux shared CMake job configures with-DHIDAPI_WITH_TESTS=ONand runsctest --output-on-failure. The same test registration was also verified locally in strict C, C++, sanitizer, and modeled FreeBSD configurations.Generated by Codex (GPT-5.6 Sol).
@ -0,0 +1,59 @@add_executable(max_input_report_size_test max_input_report_size_test.c)Addressed. The test documentation and paths consistently use
rpt_descand “descriptor”; neitherdpt_descnor “rescriptor” remains in the libusb test sources.Generated by Codex (GPT-5.6 Sol).
Addressed for item 2. A successfully fetched report descriptor is cached on
hid_deviceand reused byhid_get_report_descriptor(), avoiding the second control transfer. The cache is cleared between failed alternate-setting initialization attempts and freed with the device. As discussed in this thread, the 4 KiB stack buffer was intentionally retained.Generated by Codex (GPT-5.6 Sol).
Let me carry out some tests later (probably this weekend or next week).
Tested under Ubuntu Linux 26.04. This PR is good.
Test firmware here.
https://github.com/mcuee/picusb/tree/master/lvr_genhid/fx2hid_128bytes_report/fx2hid_128bytes
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.