On 2025/4/29 07:03, John Stultz wrote:
On Sun, Apr 27, 2025 at 4:35 AM Su Hui <suhui@xxxxxxxxxxxx> wrote:'clockid' only can be ALARM_REALTIME and ALARM_BOOTTIME. It's impossible to return -1 and callers never check the value of -1. Only alarm_clock_get_timespec(), alarm_clock_get_ktime(), alarm_timer_create() and alarm_timer_nsleep() call clock2alarm(). These callers using clockid_to_kclock() to get 'struct k_clock', this ensures clock2alarm() never returns -1. Signed-off-by: Su Hui <suhui@xxxxxxxxxxxx> --- v2: - No Change. kernel/time/alarmtimer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index 0ddccdff119a..e5450a77ada9 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -515,9 +515,9 @@ static enum alarmtimer_type clock2alarm(clockid_t clockid) { if (clockid == CLOCK_REALTIME_ALARM) return ALARM_REALTIME; - if (clockid == CLOCK_BOOTTIME_ALARM) - return ALARM_BOOTTIME; - return -1; + + /* CLOCK_BOOTTIME_ALARM case */ + return ALARM_BOOTTIME;So, I think your change is a good one, as the error case of -1 is never checked. But it might be worth adding a WARN_ON_ONCE() if the clockid isn't CLOCK_REALTIME_ALARM or CLOCK_BOOTTIME_ALARM, just so there aren't any surprises if someone misuses the function.
Yes, it's better to add WARN_ON_ONCE(). Will update in v3 patch. Thanks for your suggestion. Su Hui
