docs(hid): note that reportSize() bounds a write into a 64-byte buffer

[nigelfenton]

Comment only — 14 lines, no code change.

HidEncoderManager::poll() passes m_parser->reportSize() straight to hid_read() as the length bound on a write into m_buf, a fixed uint8_t[64]:

uint8_t m_buf[64]{};                                          // HidEncoderManager.h:187
int res = hid_read(m_device, m_buf, m_parser->reportSize());  // .cpp:591

So overriding this virtual is quietly a memory-safety decision, and its declaration said nothing about that. This adds the note at the declaration, where someone adding a parser will actually read it.

Nothing to fix today. All six parsers return ≤ 64 (32, 6, 5, 5, 14, 64), so no overflow exists on the tree. The one thing worth flagging is that TMate2Parser returns exactly 64 — the headroom is zero, not comfortable.

The note also records the opposite direction, because it looks like a bug and isn't: StreamDeckPlusParser returns 14 while its device's HID descriptor advertises 512, deliberately, since only the first 14 bytes carry data we decode. Returning less than the real report size is fine and intended.

Why a comment rather than a clamp

I originally opened #4819 to add std::min(reportSize(), sizeof(m_buf)) with a regression test. I've closed it. The clamp guarded against a future parser returning a larger value — speculative hardening for a hazard nobody has hit, and untestable on any hardware I own. The StreamDeck+ case I cited as supporting evidence actually cuts the other way: that parser already handles a 512-byte descriptor correctly.

What's genuinely missing is the documentation, not the runtime check. If a device ever does need a bigger report, the note says to grow m_buf or clamp at the call site in the same change — which is the moment to make that decision, with a real device in hand.

Happy to revisit the clamp if a reviewer would rather have the belt as well as the braces.

Provenance

Came out of the Antares scanner comment on #4786. Its stated mechanism was wrong — hid_read's return is checked (<0 disconnect, ==0 break) and the parse is bounded by res, so the short-read overflow it described doesn't exist. The unclamped length argument is a different thing one step across from what it flagged.

[aethersdr-agent]

Comment-only, and every claim in it checks out against the tree at 3a1f59ea:

• HidEncoderManager.h:187 — uint8_t m_buf[64]{}; HidEncoderManager.cpp:591 — hid_read(m_device, m_buf, m_parser->reportSize()), with no clamp anywhere between. The note is describing a real invariant that the type system doesn't enforce.
• All six parsers are ≤ 64 (32 / 6 / 5 / 5 / 14 / 64), and TMate2Parser really is at exactly 64 — the "zero headroom" flag is worth having.
• The downward direction is right too: every parse() override guards on len (HidDeviceParser.cpp:58/97/121/161/211/251/277) and the call site passes res, not reportSize(), so a parser deliberately under-reporting (StreamDeck+ at 14 vs a 512-byte descriptor) is safe on hidapi across platforms — it copies min(length, bytes_read).

I agree with the call to document rather than clamp here, and with closing #4819. The hazard is entirely in the "someone adds a seventh parser" future, and a comment at the declaration is where that person actually looks; a std::min at the call site would silently truncate a genuinely-larger report instead of making anyone decide anything. Placing it above the pure virtual rather than at the call site is the right end.

Also appreciated that the description corrects the Antares scanner's stated mechanism instead of inheriting it — hid_read's return is checked (<0 disconnect, ==0 break) and the parse is bounded by res, so the short-read overflow it described doesn't exist. Flagging the adjacent-but-real thing is the more useful outcome.

Nothing to fix. Thanks for the careful writeup.

---

_{🤖 aethersdr-agent · cost: $4.1418 · model: claude-opus-5}