From 755525761e2bcb71433efa2ee86b84ad2af2156e Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Tue, 26 May 2026 08:15:53 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20post-Trail=20review=20=E2=80=94=20al?= =?UTF-8?q?ert=20visible=20everywhere=20+=20writeTo=20error=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from a code review pass on the Trail feature: - showAlert overlay was gated to `curr == home`, so feedback like "Tracking started", "Trail saved", "GPX N B (USB)" never rendered on the Trail screen (or QuickMsg / Settings / Auto-Advert, for that matter). Drop the gate — the overlay shows on whichever screen the caller invoked from. - TrailStore::writeTo only verified the first write; subsequent point writes could partially fail (full filesystem, mid-write power loss) and the method would still return true. Check every write return value so handleSave's "Trail saved" only fires on actual success. - gpxPoint: clamp snprintf's intended-length return against the buffer size and skip the point if gmtime returns null. Avoids any chance of out.write reading past buf[120]. - TrailScreen::_act_map sized exactly to today's 8 actions — bump to 12 so adding another popup item later doesn't cause an OOB write into the surrounding members. Co-Authored-By: Claude Sonnet 4.6 --- examples/companion_radio/Trail.h | 17 ++++++++++------- examples/companion_radio/ui-new/TrailScreen.h | 2 +- examples/companion_radio/ui-new/UITask.cpp | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/examples/companion_radio/Trail.h b/examples/companion_radio/Trail.h index d8abb10a..2a4bc61d 100644 --- a/examples/companion_radio/Trail.h +++ b/examples/companion_radio/Trail.h @@ -171,17 +171,17 @@ public: template bool writeTo(F& file) { uint32_t magic = SAVE_MAGIC; - if (file.write((uint8_t*)&magic, sizeof(magic)) != sizeof(magic)) return false; uint8_t ver = SAVE_VERSION; uint8_t res = 0; uint16_t cnt = (uint16_t)_count; - file.write(&ver, 1); - file.write(&res, 1); - file.write((uint8_t*)&cnt, sizeof(cnt)); uint32_t accum = currentAccumulatedMs(); - file.write((uint8_t*)&accum, sizeof(accum)); + if (file.write((uint8_t*)&magic, sizeof(magic)) != sizeof(magic)) return false; + if (file.write(&ver, 1) != 1) return false; + if (file.write(&res, 1) != 1) return false; + if (file.write((uint8_t*)&cnt, sizeof(cnt)) != sizeof(cnt)) return false; + if (file.write((uint8_t*)&accum, sizeof(accum)) != sizeof(accum)) return false; for (int i = 0; i < _count; i++) { - file.write((uint8_t*)&at(i), sizeof(TrailPoint)); + if (file.write((uint8_t*)&at(i), sizeof(TrailPoint)) != sizeof(TrailPoint)) return false; } return true; } @@ -253,12 +253,15 @@ public: char buf[120]; time_t t = (time_t)p.ts; struct tm* gt = ::gmtime(&t); + if (!gt) return n; // defensive: skip malformed timestamps int len = snprintf(buf, sizeof(buf), "\n", p.lat_1e6 / 1.0e6, p.lon_1e6 / 1.0e6, gt->tm_year + 1900, gt->tm_mon + 1, gt->tm_mday, gt->tm_hour, gt->tm_min, gt->tm_sec); - n += out.write((const uint8_t*)buf, len); + if (len < 0) return n; + if ((size_t)len > sizeof(buf)) len = sizeof(buf); // snprintf returns intended size + n += out.write((const uint8_t*)buf, (size_t)len); return n; } diff --git a/examples/companion_radio/ui-new/TrailScreen.h b/examples/companion_radio/ui-new/TrailScreen.h index fbfb888d..5e7e00d8 100644 --- a/examples/companion_radio/ui-new/TrailScreen.h +++ b/examples/companion_radio/ui-new/TrailScreen.h @@ -25,7 +25,7 @@ class TrailScreen : public UIScreen { // and after every LEFT/RIGHT cycle. enum ActionId { ACT_MIN_DIST, ACT_UNITS, ACT_TOGGLE, ACT_SAVE, ACT_LOAD, ACT_RESET, ACT_EXPORT, ACT_EXPORT_SAVED }; PopupMenu _action_menu; - uint8_t _act_map[8]; + uint8_t _act_map[12]; // 8 used today; pad so adding an action doesn't OOB uint8_t _act_count = 0; char _act_min_dist_label[24]; char _act_units_label[24]; diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index 4874da5f..5e524c72 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -1695,7 +1695,7 @@ void UITask::loop() { } else if (!_locked && millis() >= _next_refresh && curr) { _display->startFrame(); int delay_millis = curr->render(*_display); - if (millis() < _alert_expiry && curr == home) { // render alert only on home screen + if (millis() < _alert_expiry) { // alert overlay on top of any screen _display->setTextSize(1); int lh = _display->getLineHeight(); int pad = 3;