fix(spectrum): QPointer the flag-timer window watcher (#3769)#3770
Conversation
m_flagTimerFilteredWindow (the top-level watched for minimize/restore, added in #3764) was a raw QWidget*. Safe on today's teardown paths — prepareForTopLevelChange() detaches the filter and nulls it, and Qt auto-removes the event filter when either object is destroyed — but the removeEventFilter() calls in attachFlagTimerWindowWatcher() and prepareForTopLevelChange() would be UB if a future path ever destroyed the watched window without routing through prepareForTopLevelChange() (the raw pointer would dangle non-null). Switch it to QPointer<QWidget>: a destroyed window reads back as null, so the guarded removeEventFilter() calls become automatic no-ops. No behavior change — the existing comparison / assignment / null-check sites work unchanged via QPointer's implicit conversion to QWidget*. Closes #3769. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Looks good — clean, correct, and minimal. Thanks @ten9876.
I verified all six usage sites against the current .cpp and they each work unchanged through QPointer's implicit operator QWidget*() / operator->() / assignment:
SpectrumWidget.cpp:840-843(prepareForTopLevelChange) — guardedremoveEventFilter+= nullptrSpectrumWidget.cpp:1207-1213(attachFlagTimerWindowWatcher) —top == …, guardedremoveEventFilter,= topSpectrumWidget.cpp:6661(eventFilter) —watched == m_flagTimerFilteredWindow(QObject* vs QPointer compares fine)
The safety argument holds: today every teardown routes through prepareForTopLevelChange(), but if a future path ever destroys the watched top-level without detaching first, the raw pointer would dangle non-null and sail through the if (m_flagTimerFilteredWindow && …) guard into UB. With QPointer, a destroyed window reads back null, so the guard short-circuits and the removeEventFilter() becomes an automatic no-op — exactly the intended behavior. The early-return in attachFlagTimerWindowWatcher also stays correct since window() never returns null, so a nulled-out watcher just falls through and re-attaches.
No behavior change on current paths, zero .cpp churn, well-scoped defensive hardening. Nice follow-up to the #3764 review.
🤖 aethersdr-agent · cost: $2.0913 · model: claude-opus-4-8
There was a problem hiding this comment.
Reviewed — this is a clean, well-scoped defensive fix. Thanks @ten9876.
I verified all six m_flagTimerFilteredWindow sites against the current head-of-main SpectrumWidget.cpp and they all compile and behave identically under QPointer<QWidget>:
prepareForTopLevelChange()(:840-843) —if (m_flagTimerFilteredWindow && … != this)uses QPointer's bool/!=conversions;= nullptris fine.attachFlagTimerWindowWatcher()(:1207-1213) —top == …,… != this,= topall work via implicitQWidget*conversion.eventFilter()(:6661) —watched == m_flagTimerFilteredWindow(QObject*vsQPointer<QWidget>) resolves through QPointer's comparison operators. Bonus: once the window is destroyed the QPointer reads null, so this guard can no longer spuriously match a stale address.
The core safety claim holds: with the raw pointer, a future teardown path that destroyed the watched window without routing through prepareForTopLevelChange() would leave a dangling non-null pointer that passes the if guard and calls removeEventFilter() on freed memory. QPointer collapses that to an automatic no-op. Today's paths are safe, but the member's lifetime is no longer coupled to that assumption — a worthwhile hardening for ~zero cost.
No behavior change on existing paths, scope is exactly the one member + the <QPointer> include. LGTM.
🤖 aethersdr-agent · cost: $1.5191 · model: claude-opus-4-8
Summary
Closes #3769. Defensive hardening raised in the #3764 review.
SpectrumWidget::m_flagTimerFilteredWindow— the top-level window watched for minimize/restore (added in #3764 for the #3746 idle-pause) — was a rawQWidget*. It's safe on today's teardown paths:prepareForTopLevelChange()detaches the event filter and nulls it, and Qt auto-removes the filter when either object is destroyed. But theremoveEventFilter()calls inattachFlagTimerWindowWatcher()andprepareForTopLevelChange()would be UB if a future code path ever destroyed the watched window without first routing throughprepareForTopLevelChange()— the raw pointer would dangle non-null and pass theif (m_flagTimerFilteredWindow && ...)guard.QPointer<QWidget>fixes that for ~zero cost: a destroyed window reads back as null, so the guardedremoveEventFilter()calls become automatic no-ops.What changed
src/gui/SpectrumWidget.h—m_flagTimerFilteredWindowis nowQPointer<QWidget>; added#include <QPointer>..cppchanges: every site (== window(),!= this,= top,= nullptr,->removeEventFilter,watched == ...) works unchanged via QPointer's implicit conversion toQWidget*.No behavior change — purely a dangling-pointer safety improvement.
Test plan
cmake --build build --target AetherSDR) — all comparison/assignment sites compile unchanged🤖 Generated with Claude Code