diff --git a/libs/libc/semaphore/sem_init.c b/libs/libc/semaphore/sem_init.c index f4f276b6fa..fdcd8d9f5e 100644 --- a/libs/libc/semaphore/sem_init.c +++ b/libs/libc/semaphore/sem_init.c @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -63,35 +64,28 @@ int nxsem_init(FAR sem_t *sem, int pshared, unsigned int value) { UNUSED(pshared); - /* Verify that a semaphore was provided and the count is within the valid - * range. - */ + DEBUGASSERT(sem != NULL && value <= SEM_VALUE_MAX); - if (sem != NULL && value <= SEM_VALUE_MAX) - { - /* Initialize the semaphore count */ + /* Initialize the semaphore count */ - sem->semcount = (int16_t)value; + sem->semcount = (int16_t)value; - /* Initialize semaphore wait list */ + /* Initialize semaphore wait list */ - dq_init(&sem->waitlist); + dq_init(&sem->waitlist); - /* Initialize to support priority inheritance */ + /* Initialize to support priority inheritance */ #ifdef CONFIG_PRIORITY_INHERITANCE - sem->flags = 0; + sem->flags = 0; # if CONFIG_SEM_PREALLOCHOLDERS > 0 - sem->hhead = NULL; + sem->hhead = NULL; # else - INITIALIZE_SEMHOLDER(&sem->holder[0]); - INITIALIZE_SEMHOLDER(&sem->holder[1]); + INITIALIZE_SEMHOLDER(&sem->holder[0]); + INITIALIZE_SEMHOLDER(&sem->holder[1]); # endif #endif - return OK; - } - - return -EINVAL; + return OK; } /**************************************************************************** @@ -122,6 +116,16 @@ int sem_init(FAR sem_t *sem, int pshared, unsigned int value) { int ret; + /* Verify that a semaphore was provided and the count is within the valid + * range. + */ + + if (sem == NULL || value > SEM_VALUE_MAX) + { + set_errno(EINVAL); + return ERROR; + } + ret = nxsem_init(sem, pshared, value); if (ret < 0) { diff --git a/sched/semaphore/sem_clockwait.c b/sched/semaphore/sem_clockwait.c index 1ea1d4fc86..77f8b53d0e 100644 --- a/sched/semaphore/sem_clockwait.c +++ b/sched/semaphore/sem_clockwait.c @@ -97,19 +97,9 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid, int status; int ret = ERROR; + DEBUGASSERT(sem != NULL && abstime != NULL); DEBUGASSERT(up_interrupt_context() == false); - /* Verify the input parameters and, in case of an error, set - * errno appropriately. - */ - -#ifdef CONFIG_DEBUG_FEATURES - if (!abstime || !sem) - { - return -EINVAL; - } -#endif - /* We will disable interrupts until we have completed the semaphore * wait. We need to do this (as opposed to just disabling pre-emption) * because there could be interrupt handlers that are asynchronously @@ -134,11 +124,13 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid, * with a valid timeout. */ +#ifdef CONFIG_DEBUG_FEATURES if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) { ret = -EINVAL; goto out; } +#endif /* Convert the timespec to clock ticks. We must have interrupts * disabled here so that this time stays valid until the wait begins. @@ -268,6 +260,16 @@ int sem_clockwait(FAR sem_t *sem, clockid_t clockid, { int ret; + /* Verify the input parameters and, in case of an error, set + * errno appropriately. + */ + + if (sem == NULL || abstime == NULL) + { + set_errno(EINVAL); + return ERROR; + } + /* sem_timedwait() is a cancellation point */ enter_cancellation_point(); diff --git a/sched/semaphore/sem_destroy.c b/sched/semaphore/sem_destroy.c index a5a33f7c18..883a659a52 100644 --- a/sched/semaphore/sem_destroy.c +++ b/sched/semaphore/sem_destroy.c @@ -56,34 +56,29 @@ * ****************************************************************************/ -int nxsem_destroy (FAR sem_t *sem) +int nxsem_destroy(FAR sem_t *sem) { - /* Assure a valid semaphore is specified */ + DEBUGASSERT(sem != NULL); - if (sem != NULL) + /* There is really no particular action that we need + * take to destroy a semaphore. We will just reset + * the count to some reasonable value (0) and release + * ownership. + * + * Check if other threads are waiting on the semaphore. + * In this case, the behavior is undefined. We will: + * leave the count unchanged but still return OK. + */ + + if (sem->semcount >= 0) { - /* There is really no particular action that we need - * take to destroy a semaphore. We will just reset - * the count to some reasonable value (1) and release - * ownership. - * - * Check if other threads are waiting on the semaphore. - * In this case, the behavior is undefined. We will: - * leave the count unchanged but still return OK. - */ - - if (sem->semcount >= 0) - { - sem->semcount = 1; - } - - /* Release holders of the semaphore */ - - nxsem_destroyholder(sem); - return OK; + sem->semcount = 1; } - return -EINVAL; + /* Release holders of the semaphore */ + + nxsem_destroyholder(sem); + return OK; } /**************************************************************************** @@ -114,6 +109,14 @@ int sem_destroy (FAR sem_t *sem) { int ret; + /* Assure a valid semaphore is specified */ + + if (sem == NULL) + { + set_errno(EINVAL); + return ERROR; + } + ret = nxsem_destroy(sem); if (ret < 0) { diff --git a/sched/semaphore/sem_post.c b/sched/semaphore/sem_post.c index 851c5f2f7f..475bc9d121 100644 --- a/sched/semaphore/sem_post.c +++ b/sched/semaphore/sem_post.c @@ -72,121 +72,112 @@ int nxsem_post(FAR sem_t *sem) FAR struct tcb_s *stcb = NULL; irqstate_t flags; int16_t sem_count; - int ret = -EINVAL; - /* Make sure we were supplied with a valid semaphore. */ + DEBUGASSERT(sem != NULL); - if (sem != NULL) - { - /* The following operations must be performed with interrupts - * disabled because sem_post() may be called from an interrupt - * handler. - */ + /* The following operations must be performed with interrupts + * disabled because sem_post() may be called from an interrupt + * handler. + */ - flags = enter_critical_section(); + flags = enter_critical_section(); - sem_count = sem->semcount; + sem_count = sem->semcount; - /* Check the maximum allowable value */ + /* Check the maximum allowable value */ - if (sem_count >= SEM_VALUE_MAX) - { - leave_critical_section(flags); - return -EOVERFLOW; - } + DEBUGASSERT(sem_count < SEM_VALUE_MAX); - /* Perform the semaphore unlock operation, releasing this task as a - * holder then also incrementing the count on the semaphore. - * - * NOTE: When semaphores are used for signaling purposes, the holder - * of the semaphore may not be this thread! In this case, - * nxsem_release_holder() will do nothing. - * - * In the case of a mutex this could be simply resolved since there is - * only one holder but for the case of counting semaphores, there may - * be many holders and if the holder is not this thread, then it is - * not possible to know which thread/holder should be released. - * - * For this reason, it is recommended that priority inheritance be - * disabled via nxsem_set_protocol(SEM_PRIO_NONE) when the semaphore is - * initialized if the semaphore is to used for signaling purposes. - */ + /* Perform the semaphore unlock operation, releasing this task as a + * holder then also incrementing the count on the semaphore. + * + * NOTE: When semaphores are used for signaling purposes, the holder + * of the semaphore may not be this thread! In this case, + * nxsem_release_holder() will do nothing. + * + * In the case of a mutex this could be simply resolved since there is + * only one holder but for the case of counting semaphores, there may + * be many holders and if the holder is not this thread, then it is + * not possible to know which thread/holder should be released. + * + * For this reason, it is recommended that priority inheritance be + * disabled via nxsem_set_protocol(SEM_PRIO_NONE) when the semaphore is + * initialized if the semaphore is to used for signaling purposes. + */ - nxsem_release_holder(sem); - sem_count++; - sem->semcount = sem_count; + nxsem_release_holder(sem); + sem_count++; + sem->semcount = sem_count; #ifdef CONFIG_PRIORITY_INHERITANCE - /* Don't let any unblocked tasks run until we complete any priority - * restoration steps. Interrupts are disabled, but we do not want - * the head of the ready-to-run list to be modified yet. - * - * NOTE: If this sched_lock is called from an interrupt handler, it - * will do nothing. - */ + /* Don't let any unblocked tasks run until we complete any priority + * restoration steps. Interrupts are disabled, but we do not want + * the head of the ready-to-run list to be modified yet. + * + * NOTE: If this sched_lock is called from an interrupt handler, it + * will do nothing. + */ - sched_lock(); + sched_lock(); #endif - /* If the result of semaphore unlock is non-positive, then - * there must be some task waiting for the semaphore. + /* If the result of semaphore unlock is non-positive, then + * there must be some task waiting for the semaphore. + */ + + if (sem_count <= 0) + { + /* Check if there are any tasks in the waiting for semaphore + * task list that are waiting for this semaphore. This is a + * prioritized list so the first one we encounter is the one + * that we want. */ - if (sem_count <= 0) + stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(sem)); + + if (stcb != NULL) { - /* Check if there are any tasks in the waiting for semaphore - * task list that are waiting for this semaphore. This is a - * prioritized list so the first one we encounter is the one - * that we want. + /* The task will be the new holder of the semaphore when + * it is awakened. */ - stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(sem)); + nxsem_add_holder_tcb(stcb, sem); - if (stcb != NULL) + /* Stop the watchdog timer */ + + if (WDOG_ISACTIVE(&stcb->waitdog)) { - /* The task will be the new holder of the semaphore when - * it is awakened. - */ - - nxsem_add_holder_tcb(stcb, sem); - - /* Stop the watchdog timer */ - - if (WDOG_ISACTIVE(&stcb->waitdog)) - { - wd_cancel(&stcb->waitdog); - } - - /* Restart the waiting task. */ - - up_unblock_task(stcb); + wd_cancel(&stcb->waitdog); } -#if 0 /* REVISIT: This can fire on IOB throttle semaphore */ - else - { - /* This should not happen. */ - DEBUGPANIC(); - } -#endif + /* Restart the waiting task. */ + + up_unblock_task(stcb); } +#if 0 /* REVISIT: This can fire on IOB throttle semaphore */ + else + { + /* This should not happen. */ - /* Check if we need to drop the priority of any threads holding - * this semaphore. The priority could have been boosted while they - * held the semaphore. - */ - -#ifdef CONFIG_PRIORITY_INHERITANCE - nxsem_restore_baseprio(stcb, sem); - sched_unlock(); + DEBUGPANIC(); + } #endif - ret = OK; - - /* Interrupts may now be enabled. */ - - leave_critical_section(flags); } - return ret; + /* Check if we need to drop the priority of any threads holding + * this semaphore. The priority could have been boosted while they + * held the semaphore. + */ + +#ifdef CONFIG_PRIORITY_INHERITANCE + nxsem_restore_baseprio(stcb, sem); + sched_unlock(); +#endif + + /* Interrupts may now be enabled. */ + + leave_critical_section(flags); + + return OK; } /**************************************************************************** @@ -222,6 +213,14 @@ int sem_post(FAR sem_t *sem) { int ret; + /* Make sure we were supplied with a valid semaphore. */ + + if (sem == NULL) + { + set_errno(EINVAL); + return ERROR; + } + ret = nxsem_post(sem); if (ret < 0) { diff --git a/sched/semaphore/sem_trywait.c b/sched/semaphore/sem_trywait.c index 6fe483c51d..e67038a0c2 100644 --- a/sched/semaphore/sem_trywait.c +++ b/sched/semaphore/sem_trywait.c @@ -75,41 +75,33 @@ int nxsem_trywait(FAR sem_t *sem) DEBUGASSERT(sem != NULL && up_interrupt_context() == false); DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask()); - if (sem != NULL) + /* The following operations must be performed with interrupts disabled + * because sem_post() may be called from an interrupt handler. + */ + + flags = enter_critical_section(); + + /* If the semaphore is available, give it to the requesting task */ + + if (sem->semcount > 0) { - /* The following operations must be performed with interrupts disabled - * because sem_post() may be called from an interrupt handler. - */ + /* It is, let the task take the semaphore */ - flags = enter_critical_section(); - - /* If the semaphore is available, give it to the requesting task */ - - if (sem->semcount > 0) - { - /* It is, let the task take the semaphore */ - - sem->semcount--; - nxsem_add_holder(sem); - rtcb->waitobj = NULL; - ret = OK; - } - else - { - /* Semaphore is not available */ - - ret = -EAGAIN; - } - - /* Interrupts may now be enabled. */ - - leave_critical_section(flags); + sem->semcount--; + nxsem_add_holder(sem); + rtcb->waitobj = NULL; + ret = OK; } else { - ret = -EINVAL; + /* Semaphore is not available */ + + ret = -EAGAIN; } + /* Interrupts may now be enabled. */ + + leave_critical_section(flags); return ret; } @@ -138,6 +130,12 @@ int sem_trywait(FAR sem_t *sem) { int ret; + if (sem == NULL) + { + set_errno(EINVAL); + return ERROR; + } + /* Let nxsem_trywait do the real work */ ret = nxsem_trywait(sem); diff --git a/sched/semaphore/sem_wait.c b/sched/semaphore/sem_wait.c index 33c8b3d4b6..67e1722469 100644 --- a/sched/semaphore/sem_wait.c +++ b/sched/semaphore/sem_wait.c @@ -72,7 +72,7 @@ int nxsem_wait(FAR sem_t *sem) { FAR struct tcb_s *rtcb = this_task(); irqstate_t flags; - int ret = -EINVAL; + int ret; /* This API should not be called from interrupt handlers & idleloop */ @@ -88,108 +88,105 @@ int nxsem_wait(FAR sem_t *sem) /* Make sure we were supplied with a valid semaphore. */ - if (sem != NULL) + /* Check if the lock is available */ + + if (sem->semcount > 0) { - /* Check if the lock is available */ + /* It is, let the task take the semaphore. */ - if (sem->semcount > 0) - { - /* It is, let the task take the semaphore. */ + sem->semcount--; + nxsem_add_holder(sem); + rtcb->waitobj = NULL; + ret = OK; + } - sem->semcount--; - nxsem_add_holder(sem); - rtcb->waitobj = NULL; - ret = OK; - } + /* The semaphore is NOT available, We will have to block the + * current thread of execution. + */ - /* The semaphore is NOT available, We will have to block the - * current thread of execution. + else + { + /* First, verify that the task is not already waiting on a + * semaphore */ - else - { - /* First, verify that the task is not already waiting on a - * semaphore - */ + DEBUGASSERT(rtcb->waitobj == NULL); - DEBUGASSERT(rtcb->waitobj == NULL); + /* Handle the POSIX semaphore (but don't set the owner yet) */ - /* Handle the POSIX semaphore (but don't set the owner yet) */ + sem->semcount--; - sem->semcount--; + /* Save the waited on semaphore in the TCB */ - /* Save the waited on semaphore in the TCB */ + rtcb->waitobj = sem; - rtcb->waitobj = sem; - - /* If priority inheritance is enabled, then check the priority of - * the holder of the semaphore. - */ + /* If priority inheritance is enabled, then check the priority of + * the holder of the semaphore. + */ #ifdef CONFIG_PRIORITY_INHERITANCE - /* Disable context switching. The following operations must be - * atomic with regard to the scheduler. - */ + /* Disable context switching. The following operations must be + * atomic with regard to the scheduler. + */ - sched_lock(); + sched_lock(); - /* Boost the priority of any threads holding a count on the - * semaphore. - */ + /* Boost the priority of any threads holding a count on the + * semaphore. + */ - nxsem_boost_priority(sem); + nxsem_boost_priority(sem); #endif - /* Set the errno value to zero (preserving the original errno) - * value). We reuse the per-thread errno to pass information - * between sem_waitirq() and this functions. - */ + /* Set the errno value to zero (preserving the original errno) + * value). We reuse the per-thread errno to pass information + * between sem_waitirq() and this functions. + */ - rtcb->errcode = OK; + rtcb->errcode = OK; - /* Add the TCB to the prioritized semaphore wait queue, after - * checking this is not the idle task - descheduling that - * isn't going to end well. - */ + /* Add the TCB to the prioritized semaphore wait queue, after + * checking this is not the idle task - descheduling that + * isn't going to end well. + */ - DEBUGASSERT(!is_idle_task(rtcb)); - up_block_task(rtcb, TSTATE_WAIT_SEM); + DEBUGASSERT(!is_idle_task(rtcb)); + up_block_task(rtcb, TSTATE_WAIT_SEM); - /* When we resume at this point, either (1) the semaphore has been - * assigned to this thread of execution, or (2) the semaphore wait - * has been interrupted by a signal or a timeout. We can detect - * these latter cases be examining the per-thread errno value. - * - * In the event that the semaphore wait was interrupted by a - * signal or a timeout, certain semaphore clean-up operations have - * already been performed (see sem_waitirq.c). Specifically: - * - * - nxsem_canceled() was called to restore the priority of all - * threads that hold a reference to the semaphore, - * - The semaphore count was decremented, and - * - tcb->waitobj was nullifed. - * - * It is necessary to do these things in sem_waitirq.c because a - * long time may elapse between the time that the signal was issued - * and this thread is awakened and this leaves a door open to - * several race conditions. - */ + /* When we resume at this point, either (1) the semaphore has been + * assigned to this thread of execution, or (2) the semaphore wait + * has been interrupted by a signal or a timeout. We can detect + * these latter cases be examining the per-thread errno value. + * + * In the event that the semaphore wait was interrupted by a + * signal or a timeout, certain semaphore clean-up operations have + * already been performed (see sem_waitirq.c). Specifically: + * + * - nxsem_canceled() was called to restore the priority of all + * threads that hold a reference to the semaphore, + * - The semaphore count was decremented, and + * - tcb->waitobj was nullifed. + * + * It is necessary to do these things in sem_waitirq.c because a + * long time may elapse between the time that the signal was issued + * and this thread is awakened and this leaves a door open to + * several race conditions. + */ - /* Check if an error occurred while we were sleeping. Expected - * errors include EINTR meaning that we were awakened by a signal - * or ETIMEDOUT meaning that the timer expired for the case of - * sem_timedwait(). - * - * If we were not awakened by a signal or a timeout, then - * nxsem_add_holder() was called by logic in sem_wait() fore this - * thread was restarted. - */ + /* Check if an error occurred while we were sleeping. Expected + * errors include EINTR meaning that we were awakened by a signal + * or ETIMEDOUT meaning that the timer expired for the case of + * sem_timedwait(). + * + * If we were not awakened by a signal or a timeout, then + * nxsem_add_holder() was called by logic in sem_wait() fore this + * thread was restarted. + */ - ret = rtcb->errcode != OK ? -rtcb->errcode : OK; + ret = rtcb->errcode != OK ? -rtcb->errcode : OK; #ifdef CONFIG_PRIORITY_INHERITANCE - sched_unlock(); + sched_unlock(); #endif - } } leave_critical_section(flags); @@ -254,6 +251,12 @@ int sem_wait(FAR sem_t *sem) int errcode; int ret; + if (sem == NULL) + { + set_errno(EINVAL); + return ERROR; + } + /* sem_wait() is a cancellation point */ if (enter_cancellation_point())