This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 51bb9e04173affafdd1d4f4de0ddeb5ecce6f8e2 Author: jiangtao16 <[email protected]> AuthorDate: Mon Aug 11 16:22:10 2025 +0800 sched/timer: Fix timer multiple return. Fix timer multiple return. Signed-off-by: jiangtao16 <[email protected]> --- sched/timer/timer_create.c | 92 +++++++++++++------------ sched/timer/timer_delete.c | 4 +- sched/timer/timer_getitimer.c | 20 +++--- sched/timer/timer_getoverrun.c | 11 +-- sched/timer/timer_gettime.c | 18 +++-- sched/timer/timer_release.c | 25 ++++--- sched/timer/timer_setitimer.c | 51 +++++++------- sched/timer/timer_settime.c | 150 +++++++++++++++++++++-------------------- 8 files changed, 196 insertions(+), 175 deletions(-) diff --git a/sched/timer/timer_create.c b/sched/timer/timer_create.c index 4f42d8902c2..65c6c6b7dba 100644 --- a/sched/timer/timer_create.c +++ b/sched/timer/timer_create.c @@ -158,7 +158,7 @@ static FAR struct posix_timer_s *timer_allocate(void) int timer_create(clockid_t clockid, FAR struct sigevent *evp, FAR timer_t *timerid) { - FAR struct posix_timer_s *ret; + FAR struct posix_timer_s *ret = NULL; FAR struct tcb_s *tcb = this_task(); /* Sanity checks. */ @@ -169,57 +169,61 @@ int timer_create(clockid_t clockid, FAR struct sigevent *evp, !GOOD_SIGNO(evp->sigev_signo))) { set_errno(EINVAL); - return ERROR; - } - - /* Allocate a timer instance to contain the watchdog */ - - ret = timer_allocate(); - if (!ret) - { - set_errno(EAGAIN); - return ERROR; - } - - /* Initialize the timer instance */ - - ret->pt_clock = clockid; - ret->pt_crefs = 1; - ret->pt_owner = tcb->pid; - ret->pt_delay = 0; - ret->pt_expected = 0; - - /* Was a struct sigevent provided? */ - - if (evp) - { - /* Yes, copy the entire struct sigevent content */ - - memcpy(&ret->pt_event, evp, sizeof(struct sigevent)); } else { - /* "If the evp argument is NULL, the effect is as if the evp argument - * pointed to a sigevent structure with the sigev_notify member - * having the value SIGEV_SIGNAL, the sigev_signo having a default - * signal number, and the sigev_value member having the value of the - * timer ID." - */ - - ret->pt_event.sigev_notify = SIGEV_SIGNAL; - ret->pt_event.sigev_signo = SIGALRM; - ret->pt_event.sigev_value.sival_ptr = ret; + /* Allocate a timer instance to contain the watchdog */ + + ret = timer_allocate(); + if (!ret) + { + set_errno(EAGAIN); + } + else + { + /* Initialize the timer instance */ + + ret->pt_clock = clockid; + ret->pt_crefs = 1; + ret->pt_owner = tcb->pid; + ret->pt_delay = 0; + ret->pt_expected = 0; + + /* Was a struct sigevent provided? */ + + if (evp) + { + /* Yes, copy the entire struct sigevent content */ + + memcpy(&ret->pt_event, evp, sizeof(struct sigevent)); + } + else + { + /* "If the evp argument is NULL, the effect is as if the evp + * argument pointed to a sigevent structure with the + * sigev_notify member having the value SIGEV_SIGNAL, + * the sigev_signo having a default signal number, + * and the sigev_value member having the value of the + * timer ID." + */ + + ret->pt_event.sigev_notify = SIGEV_SIGNAL; + ret->pt_event.sigev_signo = SIGALRM; + ret->pt_event.sigev_value.sival_ptr = ret; #ifdef CONFIG_SIG_EVTHREAD - ret->pt_event.sigev_notify_function = NULL; - ret->pt_event.sigev_notify_attributes = NULL; + ret->pt_event.sigev_notify_function = NULL; + ret->pt_event.sigev_notify_attributes = NULL; #endif - } + } - /* Return the timer */ + /* Return the timer */ + + *timerid = ret; + } + } - *timerid = ret; - return OK; + return ret ? OK : ERROR; } #endif /* CONFIG_DISABLE_POSIX_TIMERS */ diff --git a/sched/timer/timer_delete.c b/sched/timer/timer_delete.c index 0ee7ecd76ef..213b214b529 100644 --- a/sched/timer/timer_delete.c +++ b/sched/timer/timer_delete.c @@ -68,10 +68,10 @@ int timer_delete(timer_t timerid) if (ret < 0) { set_errno(-ret); - return ERROR; + ret = ERROR; } - return OK; + return ret; } #endif /* CONFIG_DISABLE_POSIX_TIMERS */ diff --git a/sched/timer/timer_getitimer.c b/sched/timer/timer_getitimer.c index b4d515d1a48..3e6ac3b7ae3 100644 --- a/sched/timer/timer_getitimer.c +++ b/sched/timer/timer_getitimer.c @@ -82,18 +82,20 @@ int getitimer(int which, FAR struct itimerval *value) if (which != ITIMER_REAL || !value) { set_errno(EINVAL); - return ERROR; + ret = ERROR; } - - if (rtcb->group->itimer) + else { - ret = timer_gettime(rtcb->group->itimer, &spec); - } + if (rtcb->group->itimer) + { + ret = timer_gettime(rtcb->group->itimer, &spec); + } - if (ret == OK) - { - TIMESPEC_TO_TIMEVAL(&value->it_value, &spec.it_value); - TIMESPEC_TO_TIMEVAL(&value->it_interval, &spec.it_interval); + if (ret == OK) + { + TIMESPEC_TO_TIMEVAL(&value->it_value, &spec.it_value); + TIMESPEC_TO_TIMEVAL(&value->it_interval, &spec.it_interval); + } } return ret; diff --git a/sched/timer/timer_getoverrun.c b/sched/timer/timer_getoverrun.c index 6fe13b20ac7..b7b77f1dff9 100644 --- a/sched/timer/timer_getoverrun.c +++ b/sched/timer/timer_getoverrun.c @@ -83,12 +83,15 @@ int timer_getoverrun(timer_t timerid) if (!timer) { set_errno(EINVAL); - return ERROR; + ret = ERROR; + } + else + { + ret = timer->pt_overrun; + ret = ret > DELAYTIMER_MAX ? DELAYTIMER_MAX : ret; } - ret = timer->pt_overrun; - - return ret > DELAYTIMER_MAX ? DELAYTIMER_MAX : ret; + return ret; } #endif /* CONFIG_DISABLE_POSIX_TIMERS */ diff --git a/sched/timer/timer_gettime.c b/sched/timer/timer_gettime.c index 322460cd7e4..280c656d13d 100644 --- a/sched/timer/timer_gettime.c +++ b/sched/timer/timer_gettime.c @@ -74,22 +74,26 @@ int timer_gettime(timer_t timerid, FAR struct itimerspec *value) { FAR struct posix_timer_s *timer = timer_gethandle(timerid); sclock_t ticks; + int ret = OK; if (!timer || !value) { set_errno(EINVAL); - return ERROR; + ret = ERROR; } + else + { + /* Get the number of ticks before the underlying watchdog expires */ - /* Get the number of ticks before the underlying watchdog expires */ + ticks = wd_gettime(&timer->pt_wdog); - ticks = wd_gettime(&timer->pt_wdog); + /* Convert that to a struct timespec and return it */ - /* Convert that to a struct timespec and return it */ + clock_ticks2time(&value->it_value, ticks); + clock_ticks2time(&value->it_interval, timer->pt_delay); + } - clock_ticks2time(&value->it_value, ticks); - clock_ticks2time(&value->it_interval, timer->pt_delay); - return OK; + return ret; } #endif /* CONFIG_DISABLE_POSIX_TIMERS */ diff --git a/sched/timer/timer_release.c b/sched/timer/timer_release.c index 9a132c403ce..3ab5354d375 100644 --- a/sched/timer/timer_release.c +++ b/sched/timer/timer_release.c @@ -105,35 +105,40 @@ static inline void timer_free(struct posix_timer_s *timer) int timer_release(FAR struct posix_timer_s *timer) { + int ret = OK; + /* Some sanity checks */ if (timer == NULL) { - return -EINVAL; + ret = -EINVAL; } /* Release one reference to timer. Don't delete the timer until the count * would decrement to zero. */ - if (timer->pt_crefs > 1) + else if (timer->pt_crefs > 1) { timer->pt_crefs--; - return 1; + ret = 1; } + else + { + /* Cancel the underlying watchdog instance */ - /* Cancel the underlying watchdog instance */ + wd_cancel(&timer->pt_wdog); - wd_cancel(&timer->pt_wdog); + /* Cancel any pending notification */ - /* Cancel any pending notification */ + nxsig_cancel_notification(&timer->pt_work); - nxsig_cancel_notification(&timer->pt_work); + /* Release the timer structure */ - /* Release the timer structure */ + timer_free(timer); + } - timer_free(timer); - return OK; + return ret; } #endif /* CONFIG_DISABLE_POSIX_TIMERS */ diff --git a/sched/timer/timer_setitimer.c b/sched/timer/timer_setitimer.c index a5e2d25a301..ac91b33ce54 100644 --- a/sched/timer/timer_setitimer.c +++ b/sched/timer/timer_setitimer.c @@ -97,33 +97,34 @@ int setitimer(int which, FAR const struct itimerval *value, if (which != ITIMER_REAL || !value) { set_errno(EINVAL); - return ERROR; + ret = ERROR; } - - rtcb = this_task(); - - nxrmutex_lock(&rtcb->group->tg_mutex); - - if (!rtcb->group->itimer) - { - ret = timer_create(CLOCK_REALTIME, NULL, &rtcb->group->itimer); - } - - nxrmutex_unlock(&rtcb->group->tg_mutex); - - if (ret != OK) - { - return ret; - } - - TIMEVAL_TO_TIMESPEC(&value->it_value, &spec.it_value); - TIMEVAL_TO_TIMESPEC(&value->it_interval, &spec.it_interval); - - ret = timer_settime(rtcb->group->itimer, 0, &spec, ovalue ? &ospec : NULL); - if (ret == OK && ovalue) + else { - TIMESPEC_TO_TIMEVAL(&ovalue->it_value, &ospec.it_value); - TIMESPEC_TO_TIMEVAL(&ovalue->it_interval, &ospec.it_interval); + rtcb = this_task(); + + nxrmutex_lock(&rtcb->group->tg_mutex); + + if (!rtcb->group->itimer) + { + ret = timer_create(CLOCK_REALTIME, NULL, &rtcb->group->itimer); + } + + nxrmutex_unlock(&rtcb->group->tg_mutex); + + if (ret == OK) + { + TIMEVAL_TO_TIMESPEC(&value->it_value, &spec.it_value); + TIMEVAL_TO_TIMESPEC(&value->it_interval, &spec.it_interval); + + ret = timer_settime(rtcb->group->itimer, 0, &spec, + ovalue ? &ospec : NULL); + if (ret == OK && ovalue) + { + TIMESPEC_TO_TIMEVAL(&ovalue->it_value, &ospec.it_value); + TIMESPEC_TO_TIMEVAL(&ovalue->it_interval, &ospec.it_interval); + } + } } return ret; diff --git a/sched/timer/timer_settime.c b/sched/timer/timer_settime.c index f69c3fa83ac..1fb67734f17 100644 --- a/sched/timer/timer_settime.c +++ b/sched/timer/timer_settime.c @@ -158,28 +158,26 @@ static void timer_timeout(wdparm_t itimer) { FAR struct posix_timer_s *timer = timer_gethandle((timer_t)itimer); - if (timer == NULL) + if (timer) { - return; - } - - /* Send the specified signal to the specified task. Increment the - * reference count on the timer first so that will not be deleted until - * after the signal handler returns. - */ + /* Send the specified signal to the specified task. Increment the + * reference count on the timer first so that will not be deleted until + * after the signal handler returns. + */ - timer->pt_crefs++; - timer_signotify(timer); + timer->pt_crefs++; + timer_signotify(timer); - /* Release the reference. timer_release will return nonzero if the timer - * was not deleted. - */ + /* Release the reference. timer_release will return nonzero if + * the timer was not deleted. + */ - if (timer_release(timer)) - { - /* If this is a repetitive timer, the restart the watchdog */ + if (timer_release(timer)) + { + /* If this is a repetitive timer, the restart the watchdog */ - timer_restart(timer, itimer); + timer_restart(timer, itimer); + } } } @@ -264,77 +262,81 @@ int timer_settime(timer_t timerid, int flags, if (!timer || !value) { set_errno(EINVAL); - return ERROR; + ret = -EINVAL; } - - if (ovalue) + else { - /* Get the number of ticks before the underlying watchdog expires */ + if (ovalue) + { + /* Get the number of ticks before the underlying watchdog expires */ - delay = wd_gettime(&timer->pt_wdog); + delay = wd_gettime(&timer->pt_wdog); - /* Convert that to a struct timespec and return it */ + /* Convert that to a struct timespec and return it */ - clock_ticks2time(&ovalue->it_value, delay); - clock_ticks2time(&ovalue->it_interval, timer->pt_delay); - } + clock_ticks2time(&ovalue->it_value, delay); + clock_ticks2time(&ovalue->it_interval, timer->pt_delay); + } - /* Disarm the timer (in case the timer was already armed when - * timer_settime() is called). - */ - - wd_cancel(&timer->pt_wdog); - - /* Cancel any pending notification */ - - nxsig_cancel_notification(&timer->pt_work); - - /* If the it_value member of value is zero, the timer will not be - * re-armed - */ - - if (value->it_value.tv_sec <= 0 && value->it_value.tv_nsec <= 0) - { - return OK; - } - - /* Setup up any repetitive timer */ + /* Disarm the timer (in case the timer was already armed when + * timer_settime() is called). + */ - if (value->it_interval.tv_sec > 0 || value->it_interval.tv_nsec > 0) - { - delay = clock_time2ticks(&value->it_interval); - timer->pt_delay = delay; - } - else - { - timer->pt_delay = 0; - } + wd_cancel(&timer->pt_wdog); - /* Check if abstime is selected */ + /* Cancel any pending notification */ - if ((flags & TIMER_ABSTIME) != 0) - { - /* Calculate a delay corresponding to the absolute time in 'value' */ + nxsig_cancel_notification(&timer->pt_work); - clock_abstime2ticks(timer->pt_clock, &value->it_value, &delay); - } - else - { - /* Calculate a delay assuming that 'value' holds the relative time - * to wait. We have internal knowledge that clock_time2ticks always - * returns success. + /* If the it_value member of value is zero, the timer will not be + * re-armed */ - delay = clock_time2ticks(&value->it_value); + if (value->it_value.tv_sec > 0 || value->it_value.tv_nsec > 0) + { + /* Setup up any repetitive timer */ + + if (value->it_interval.tv_sec > 0 || + value->it_interval.tv_nsec > 0) + { + delay = clock_time2ticks(&value->it_interval); + timer->pt_delay = delay; + } + else + { + timer->pt_delay = 0; + } + + /* Check if abstime is selected */ + + if ((flags & TIMER_ABSTIME) != 0) + { + /* Calculate a delay corresponding to the + * absolute time in 'value'. + */ + + clock_abstime2ticks(timer->pt_clock, &value->it_value, &delay); + } + else + { + /* Calculate a delay assuming that 'value' holds the + * relative time to wait. + * We have internal knowledge that clock_time2ticks always + * returns success. + */ + + delay = clock_time2ticks(&value->it_value); + } + + timer->pt_expected = clock_delay2abstick(delay); + + /* Then start the watchdog */ + + ret = wd_start_abstick(&timer->pt_wdog, timer->pt_expected, + timer_timeout, (wdparm_t)timer); + } } - timer->pt_expected = clock_delay2abstick(delay); - - /* Then start the watchdog */ - - ret = wd_start_abstick(&timer->pt_wdog, timer->pt_expected, - timer_timeout, (wdparm_t)timer); - if (ret < 0) { set_errno(-ret);
