Skip to content

LG: Fix uninitialised and never-updated Swing(H) previous state - #2287

Merged
NiKiZe merged 2 commits into
crankyoldgit:masterfrom
JayyyXp:fix-lg-swingh-prev
Aug 14, 2026
Merged

LG: Fix uninitialised and never-updated Swing(H) previous state#2287
NiKiZe merged 2 commits into
crankyoldgit:masterfrom
JayyyXp:fix-lg-swingh-prev

Conversation

@JayyyXp

@JayyyXp JayyyXp commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

IRLgAc::updateSwingPrev() copies _swingv and _vaneswingv[] across to their
_prev counterparts, but never _swingh. Since updateSwingPrev() is also what
stateReset() uses to prime those values, _swingh_prev is never initialised at
all — and never updated after a send, either.

For the AKB73757604 model (the only one that consults it in send()) that
means:

  1. The first send() reads an uninitialised bool. UBSAN, on current master:

    ../src/ir_LG.cpp:280:24: runtime error: load of value 74, which is not a
    valid value for type 'bool'
    
  2. The if (_swingh != _swingh_prev) "only send it if it changed" check never
    settles, 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_AKB73757604 flake

That 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:

IRac_test.cpp:1490: Failure
Expected equality of these values:
  361
  ac._irsend.capture.rawlen
    Which is: 301

The fix

One line — copy _swingh across in updateSwingPrev(), like its siblings.

Also adds TestIRLgAcClass.SwingHPrevIsTracked, which checks that a repeated
identical 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

  • Full make run suite passes; cpplint clean.
  • Verified the new test fails on master and passes with the fix.
  • Re-ran the LG tests under -fsanitize=address,undefined: the
    ir_LG.cpp:280 runtime error is gone.
  • Not tested against physical hardware — I don't own an LG unit. The change only
    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:

../src/ir_LG.cpp:133:20: runtime error: left shift of 65535 by 20 places
cannot be represented in type 'int'

That's encodeLG()'s address << 20uint16_t promotes to int, so a large
address overflows. Benign on the usual targets, but happy to send a follow-up
casting it to uint32_t if you'd like it silenced.

`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
NiKiZe merged commit 1aa31ea into crankyoldgit:master Aug 14, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants