LG: Fix uninitialised and never-updated Swing(H) previous state - #2287
Merged
Conversation
`updateSwingPrev()` copies `_swingv` and `_vaneswingv[]` across to their `_prev` counterparts, but never `_swingh`. As `updateSwingPrev()` is also what `stateReset()` uses to prime those values, `_swingh_prev` is never initialised at all, and never updated after a send either. That has two consequences for the AKB73757604 model, which is the only one that consults it in `send()`: * The first `send()` reads an uninitialised bool. UBSAN agrees: `ir_LG.cpp:280:24: runtime error: load of value 74, which is not a valid value for type 'bool'` * The `if (_swingh != _swingh_prev)` "only send it if it changed" check never settles, so Swing(H) is either re-sent on every single send, or never sent at all -- decided by whatever was on the stack that day. This is also why `TestIRac.LG2_AKB73757604` is flaky: it asserts six messages get sent, and the sixth is exactly that Swing(H) message. It passes or fails depending on that garbage value, which is a fun way to spend an afternoon when it fails on an unrelated PR. :-) Fixed by copying `_swingh` across in `updateSwingPrev()` like its siblings. Added `TestIRLgAcClass.SwingHPrevIsTracked`, which fails on master regardless of what the uninitialised value happens to be, since it checks a repeated send doesn't re-send Swing(H).
The rationale belongs in the commit message & PR, not wrapped across four lines above the test. Matches the rest of the file, where tests carry either no comment or a single line.
NiKiZe
approved these changes
Aug 14, 2026
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.
IRLgAc::updateSwingPrev()copies_swingvand_vaneswingv[]across to their_prevcounterparts, but never_swingh. SinceupdateSwingPrev()is also whatstateReset()uses to prime those values,_swingh_previs never initialised atall — and never updated after a send, either.
For the
AKB73757604model (the only one that consults it insend()) thatmeans:
The first
send()reads an uninitialisedbool. UBSAN, on current master:The
if (_swingh != _swingh_prev)"only send it if it changed" check neversettles, so the Swing(H) message is either re-sent on every send or never
sent at all, depending on whatever happened to be on the stack.
This is the
TestIRac.LG2_AKB73757604flakeThat test asserts six messages are sent, and the sixth is exactly this Swing(H)
message, so it passes or fails on the garbage value. It's green on this machine
and was green in CI on 2026-08-02, but failed on the identical runner image in
an unrelated PR of mine today (#2286), which is how I found it:
The fix
One line — copy
_swinghacross inupdateSwingPrev(), like its siblings.Also adds
TestIRLgAcClass.SwingHPrevIsTracked, which checks that a repeatedidentical send doesn't re-send Swing(H). It fails on master regardless of what
the uninitialised value happens to be (either the first or the second assertion
trips, depending on the garbage), and passes with the fix.
Testing
make runsuite passes;cpplintclean.-fsanitize=address,undefined: their_LG.cpp:280runtime error is gone.affects whether an already-existing Swing(H) message is emitted when the
setting hasn't changed, but a sanity check from someone with an AKB73757604
would be worth having before this is trusted.
Unrelated thing UBSAN also flagged
Not touched here, but while I had the sanitizer running:
That's
encodeLG()'saddress << 20—uint16_tpromotes toint, so a largeaddress overflows. Benign on the usual targets, but happy to send a follow-up
casting it to
uint32_tif you'd like it silenced.