From 15716d2b03d90538c39dae727fd412d6bd9a6882 Mon Sep 17 00:00:00 2001 From: MarekZegare4 Date: Mon, 29 Jun 2026 18:42:59 +0200 Subject: [PATCH] fix(companion): nullptr-init screen pointers so a forgotten new() fails safe Adding a screen touches 4 sites; 3 (member decl, gotoX decl, gotoX def) are compile-checked, but a missed `new XScreen()` in begin() left the pointer uninitialised and crashed at first navigation. Give every screen member an in-class nullptr initialiser and bail early in setCurrScreen(nullptr) so the mistake is an inert no-op instead of a null deref. Document the 4-site registration contract on the member block. A full registry table was considered and rejected: the named gotoXScreen() methods are a depended-upon API (~30 call sites, menu dispatch + back-nav), so a table would add an enum + indirection without removing them. Co-Authored-By: Claude Opus 4.8 --- examples/companion_radio/ui-new/UITask.cpp | 7 +++- examples/companion_radio/ui-new/UITask.h | 39 ++++++++++++---------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index c1e8a070..e848d171 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -1748,8 +1748,13 @@ void UITask::userLedHandler() { } void UITask::setCurrScreen(UIScreen* c) { + // Fail safe on a null target: a screen pointer left uninitialised (member + // declared + navigator wired, but the `new XScreen()` line forgotten in + // begin()) stays nullptr thanks to the in-class initialisers. Bail here so + // that mistake is an inert no-op instead of a null deref in render()/poll(). + if (!c) return; curr = c; - if (c) c->onShow(); // central per-visit reset hook (see UIScreen::onShow) + c->onShow(); // central per-visit reset hook (see UIScreen::onShow) _next_refresh = 100; } diff --git a/examples/companion_radio/ui-new/UITask.h b/examples/companion_radio/ui-new/UITask.h index d4e72e0a..510f30d3 100644 --- a/examples/companion_radio/ui-new/UITask.h +++ b/examples/companion_radio/ui-new/UITask.h @@ -68,23 +68,28 @@ class UITask : public AbstractUITask { unsigned long _analogue_pin_read_millis = millis(); #endif - UIScreen* splash; - UIScreen* home; - UIScreen* settings; - UIScreen* quick_msg; - UIScreen* tools_screen; - UIScreen* ringtone_edit; - UIScreen* bot_screen; - UIScreen* nearby_screen; - UIScreen* dashboard_config; - UIScreen* auto_advert_screen; - UIScreen* live_share_screen; - UIScreen* locator_screen; - UIScreen* trail_screen; - UIScreen* compass_screen; - UIScreen* diag_screen; - UIScreen* repeater_screen; - UIScreen* curr; + // Registering a new screen touches 4 sites: (1) the member below, (2) the + // `new XScreen()` in begin(), (3) the gotoXScreen() declaration further down, + // (4) its one-line definition in UITask.cpp. Sites 1/3/4 are compile-checked; + // only a forgotten (2) can slip through — the nullptr initialisers here turn + // that into an inert no-op (see UITask::setCurrScreen) rather than a crash. + UIScreen* splash = nullptr; + UIScreen* home = nullptr; + UIScreen* settings = nullptr; + UIScreen* quick_msg = nullptr; + UIScreen* tools_screen = nullptr; + UIScreen* ringtone_edit = nullptr; + UIScreen* bot_screen = nullptr; + UIScreen* nearby_screen = nullptr; + UIScreen* dashboard_config = nullptr; + UIScreen* auto_advert_screen = nullptr; + UIScreen* live_share_screen = nullptr; + UIScreen* locator_screen = nullptr; + UIScreen* trail_screen = nullptr; + UIScreen* compass_screen = nullptr; + UIScreen* diag_screen = nullptr; + UIScreen* repeater_screen = nullptr; + UIScreen* curr = nullptr; CayenneLPP _dash_lpp; TrailStore _trail; WaypointStore _waypoints;