Uh oh!
There was an error while loading. Please reload this page.
[tick, timeout] Add support for a 64 bit tick source - #53
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in 64-bit tick mode for the whal_Timeout abstraction so boards can use wider tick counters while preserving the existing 32-bit default behavior.
Changes:
- Introduces
WHAL_CFG_64BIT_TICKto switchwhal_Timeout’s tick fields andGetTickcallback betweenuint32_tanduint64_t. - Adds
WHAL_TICK_MAXand updatesWHAL_TIMEOUT_EXPIRED()to use the configured tick width. - Updates the timeout unit test to work in both 32-bit and 64-bit tick configurations (including wraparound behavior).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wolfHAL/timeout.h | Adds a build-time option for 64-bit tick sources and adjusts timeout calculations accordingly. |
| tests/core/test_timeout.c | Adapts fake tick source + wrap test to use the configured tick width (WHAL_TICK_MAX). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
wolfHAL/timeout.h:89
- The casts in WHAL_TIMEOUT_EXPIRED are currently applied to the result of the comparison (0/1) rather than to the tick subtraction. This makes the cast ineffective/misleading and defeats the intent of explicitly evaluating the elapsed tick delta at the configured width.
#ifdef WHAL_CFG_64BIT_TICK
#define WHAL_TIMEOUT_EXPIRED(t) \
((t) && ((uint64_t)((t)->GetTick() - (t)->startTick) >= (t)->timeoutTicks))
#else
#define WHAL_TIMEOUT_EXPIRED(t) \
((t) && ((uint32_t)((t)->GetTick() - (t)->startTick) >= (t)->timeoutTicks))
#endif
docs/writing_a_driver.md:342
- The 64-bit tick guidance should also mention atomicity: on 32-bit MCUs, reading/updating a 64-bit tick counter is often not atomic and can yield torn reads (non-monotonic GetTick results), which can break timeout behavior.
When the macro is defined, the board's tick source must match the width: its
`g_tick` counter and `GetTick` callback (typically `Board_GetTick`) must be
`uint64_t`, since `whal_Timeout.GetTick` becomes `uint64_t (*)(void)`. The
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #53
Scan targets checked:wolfhal-bugs, wolfhal-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Uh oh!
There was an error while loading. Please reload this page.
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes:review + review-security
Overall recommendation:COMMENT
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] [review+review-security] WHAL_CFG_64BIT_TICK duplicates whal_Timeout and WHAL_TIMEOUT_EXPIRED per width and no in-tree board can enable it; a whal_Tick typedef fixes both —
wolfHAL/timeout.h:41-89 - [Medium] [review] 64-bit tick config is never exercised with a timeout that actually requires 64-bit arithmetic —
tests/core/test_timeout.c:43-46 - [Medium] [review] 64-bit tick callbacks require an atomic snapshot contract —
wolfHAL/timeout.h:72-85 - [Low] [review-security] Mixed-width builds silently corrupt whal_Timeout across library boundaries —
wolfHAL/timeout.h:47-57 - [Info] [review-security] CI matrix does not cover WHAL_CFG_64BIT_TICK combined with WHAL_CFG_NO_TIMEOUT —
.github/workflows/core-tests.yml:14
Review generated by Skoll
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This PR adds optional support for a 64 bit tick source by setting the config macro
WHAL_CFG_64BIT_TICK.