From e6d0738dd9e8d583503477fee348d8f490f0e982 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 00:43:57 +0000 Subject: [PATCH] Fix code review issues: overflow protection and parameter handling Co-authored-by: BernardXiong <1241087+BernardXiong@users.noreply.github.com> --- components/drivers/clock_time/src/clock_time.c | 16 ++++++++++++++-- components/drivers/clock_time/src/hrtimer.c | 14 ++++++++++++-- components/drivers/include/drivers/clock_time.h | 4 ++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/components/drivers/clock_time/src/clock_time.c b/components/drivers/clock_time/src/clock_time.c index d03eca0723..11458c2698 100644 --- a/components/drivers/clock_time/src/clock_time.c +++ b/components/drivers/clock_time/src/clock_time.c @@ -52,8 +52,20 @@ rt_err_t rt_clock_time_device_register(struct rt_clock_time_device *dev, rt_uint64_t freq = dev->ops->get_freq(); if (freq > 0) { - /* res_scale = (1e9 * RT_CLOCK_TIME_RESMUL) / freq */ - dev->res_scale = ((1000000000ULL * RT_CLOCK_TIME_RESMUL) / freq); + /* res_scale = (1e9 * RT_CLOCK_TIME_RESMUL) / freq + * To avoid overflow, we check if freq is very small. + * For freq >= 1000, this calculation is safe on 64-bit. + * For very small frequencies, limit the scale factor. + */ + if (freq >= 1000) + { + dev->res_scale = ((1000000000ULL * RT_CLOCK_TIME_RESMUL) / freq); + } + else + { + /* For very low frequencies, calculate more carefully */ + dev->res_scale = (1000000ULL * RT_CLOCK_TIME_RESMUL) / freq * 1000; + } } else { diff --git a/components/drivers/clock_time/src/hrtimer.c b/components/drivers/clock_time/src/hrtimer.c index a5e9488ac1..630f148fe0 100644 --- a/components/drivers/clock_time/src/hrtimer.c +++ b/components/drivers/clock_time/src/hrtimer.c @@ -80,7 +80,17 @@ rt_weak rt_err_t rt_clock_hrtimer_settimeout(unsigned long cnt) static unsigned long _cnt_convert(unsigned long cnt) { unsigned long rtn = 0; - unsigned long count = cnt - rt_clock_cputimer_getcnt(); + unsigned long current_cnt = rt_clock_cputimer_getcnt(); + + /* Check for overflow/underflow - if cnt is in the past or wrapped around */ + if (cnt <= current_cnt) + { + return 0; + } + + unsigned long count = cnt - current_cnt; + + /* Sanity check: if count is too large, it might be a wrap-around */ if (count > (_HRTIMER_MAX_CNT / 2)) return 0; @@ -289,7 +299,7 @@ rt_err_t rt_clock_hrtimer_control(rt_clock_hrtimer_t timer, int cmd, void *arg) *(unsigned long *)arg = timer->timeout_cnt; break; case RT_TIMER_CTRL_GET_FUNC: - arg = (void *)timer->timeout_func; + *(void **)arg = (void *)timer->timeout_func; break; case RT_TIMER_CTRL_SET_FUNC: diff --git a/components/drivers/include/drivers/clock_time.h b/components/drivers/include/drivers/clock_time.h index 7f8bb21c68..69c288a4c9 100644 --- a/components/drivers/include/drivers/clock_time.h +++ b/components/drivers/include/drivers/clock_time.h @@ -256,6 +256,10 @@ rt_err_t rt_clock_hrtimer_detach(rt_clock_hrtimer_t timer); * @brief Keep errno in timer structure * @param timer Timer structure * @param err Error code to keep + * + * Note: This function negates err when setting errno to convert RT-Thread + * error codes to POSIX-style errno values. This maintains compatibility + * with the original ktime implementation. */ rt_inline void rt_clock_hrtimer_keep_errno(rt_clock_hrtimer_t timer, rt_err_t err) {