Conversation
Widen PacketContext and TransactionContext generation fields from uint8_t to uint16_t to preserve the full IEEE 1394 bus generation value. The internal tracker uses 16-bit generations for wrap-safe uniqueness; truncating to uint8_t silently discards the high byte, causing incorrect response matching across bus-reset cycles.
gly11
marked this pull request as ready for review
April 22, 2026 08:42
Contributor
Author
|
Superseded by #9. I folded the 16-bit generation fix into the new async/bus foundation PR so the related transaction-generation and completion-handling fixes can be reviewed together. |
alicankaralar
pushed a commit
to alicankaralar/ASFireWire
that referenced
this pull request
Jul 12, 2026
…ation 44100.md: - §12: reverse-engineer RME FirefaceAudioDriver's 44.1 cadence — DCL-branch Bresenham (increment = rate/8000), closed-loop RX clock recovery via the +276 per-group target table and a 1/16 IIR estimate, and per-rate startup latency anchors. Behavioral/algorithm observations only (proprietary binary). - Extend the IDA evidence index and add RME-specific open questions. - Mark all open questions with status: mrmidi#6 RESOLVED (Linux cross-check), mrmidi#8 RESOLVED-by-decision (resync branch not needed under replay), mrmidi#12 RESOLVED design / capture-gated value; flag #1–#5 as legacy-DCL archaeology, not blockers. SAMPLE_RATE_EXPANSION.md: - §3: minimal TX cadence contract ({data/no-data, SYT}; DBC free; SYT = delta replay + per-rate phase re-anchor; payload separate; RX carries none) and what "device-as-master" does and does not remove. - §8: consolidate the capture-gated items into one measurement (per-rate presentation lead + startup-prefix tolerance) in one gated session, partly self-instrumented via [TxSyt]/[Zts] traces; mrmidi#9/mrmidi#10 moot under replay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
forkt69
pushed a commit
to forkt69/ASFireWire
that referenced
this pull request
Aug 28, 2026
…ation 44100.md: - §12: reverse-engineer RME FirefaceAudioDriver's 44.1 cadence — DCL-branch Bresenham (increment = rate/8000), closed-loop RX clock recovery via the +276 per-group target table and a 1/16 IIR estimate, and per-rate startup latency anchors. Behavioral/algorithm observations only (proprietary binary). - Extend the IDA evidence index and add RME-specific open questions. - Mark all open questions with status: mrmidi#6 RESOLVED (Linux cross-check), mrmidi#8 RESOLVED-by-decision (resync branch not needed under replay), mrmidi#12 RESOLVED design / capture-gated value; flag mrmidi#1–mrmidi#5 as legacy-DCL archaeology, not blockers. SAMPLE_RATE_EXPANSION.md: - §3: minimal TX cadence contract ({data/no-data, SYT}; DBC free; SYT = delta replay + per-rate phase re-anchor; payload separate; RX carries none) and what "device-as-master" does and does not remove. - §8: consolidate the capture-gated items into one measurement (per-rate presentation lead + startup-prefix tolerance) in one gated session, partly self-instrumented via [TxSyt]/[Zts] traces; mrmidi#9/mrmidi#10 moot under replay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mrmidi
pushed a commit
that referenced
this pull request
Sep 15, 2026
…eardown
FCPTransportTests.RejectsResponseForInvalidatedRouteAfterRebind and
RejectsWriteCompletionFromInvalidatedRoute crashed with SEGFAULT.
Diagnosed with AddressSanitizer as stack-use-after-return, not the stack
overflow the fault address and unwind failure first suggested:
ERROR: AddressSanitizer: stack-use-after-return
#0 ...TestBody()::$_0::operator() FCPTransportTests.cpp:129
#7 FCPTransport::Shutdown() FCPTransport.cpp:301
#8 FCPTransportTests::TearDown() FCPTransportTests.cpp:74
Both tests invalidate the route on purpose, so their command never completes and
is still pending when TearDown() runs Shutdown(). Shutdown() then completes
every pending and queued command with kTransportError -- correct behaviour, it
must not leak outstanding work -- which invokes a completion that captured
`&completionCount`, a TestBody() local whose frame is already gone.
The driver is not at fault and is unchanged. The other tests in this file are
safe only incidentally: their commands complete inside the test body, so
Shutdown() finds nothing pending.
Fix: own the counter on the fixture, so its lifetime spans TearDown.
Verified: both tests pass under ASan with no sanitizer findings, and the full
host suite is 1653/1653 (previously 1651/1653 with these two failing on
unmodified origin/main).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 171e4afe510114d4b8e152fc5687513f042969c2)
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
1 participant
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.
Summary
PacketContext::generationandTransactionContext::generationfromuint8_ttouint16_tstatic_assertguards to prevent future narrowinguint8_tsilently discards the high byte, causing incorrect response matching across bus-reset cyclesDepends on #7
Test plan
./build.sh --test-only) — 67 async-related tests all greenstatic_assertcompiles cleanly with C++23