fix(usb_host): clamp copied input-report length; document the failed-teardown state - #793
Merged
Conversation
…teardown state Two follow-ups from the Copilot review of #792 that concern code now on main: - on_interface_event(): never trust the driver-reported input-report length beyond the buffer we handed it -- clamp `len` to the copy capacity (logging the truncation) so Event::data() can never form an out-of-bounds span. - deinitialize(): when hid_host_uninstall() keeps failing the root port is deliberately left powered off (powering it back up would only make the driver re-track the device a retry needs gone). Document that state in the header and the error log: event delivery is stopped, devices are retired, and the only valid next steps are retrying deinitialize() or destroying the object. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
✅Static analysis result - no issues found! ✅ |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Follow-up fixes to espp::UsbHost to guard against out-of-bounds report spans and to clarify the teardown failure behavior when hid_host_uninstall() cannot complete.
Changes:
- Clamp copied HID input-report length to the configured buffer capacity with a truncation warning.
- Expand documentation and logging around the “failed teardown” state, noting the root port remains powered off and what recovery steps are valid.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/usb_host/src/usb_host.cpp | Adds input-report length clamping and expands failed-uninstall logging to describe the powered-off root port behavior. |
| components/usb_host/include/usb_host.hpp | Documents the non-usable “failed teardown” state and the only valid next steps (retry deinitialize or destroy). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…r log Only ESP_ERR_INVALID_STATE means the driver still tracks a device; report any other error as-is instead of guessing at a cause. Keeps the "root port left powered off, retry deinitialize()" guidance. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small follow-ups to
espp::UsbHost(#791) raised by the Copilot review on #792 againstusb_hostcode that is now onmain:on_interface_event):lenfromhid_host_device_get_raw_input_report_data()is now bounded to the buffer capacity we handed the driver (with a truncation warning), soEvent::data()can never form an out-of-bounds span even if the driver ever reported the full report length instead of the copied length.deinitialize): whenhid_host_uninstall()keeps failing, the root port is deliberately left powered off — powering it back up would make the driver re-enumerate and re-track the very device a retry needs gone. The header docs and the error log now state that in this state event delivery is stopped, devices are retired, and the only valid next steps are retryingdeinitialize()or destroying the object.No behaviour change on the success path.
components/usb_host/examplebuilds clean (IDF v6.1, esp32s3).🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU