You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/11/10 14:36:28 UTC

[incubator-nuttx] branch master updated (a522a8eeaf -> a9c647d418)

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


    from a522a8eeaf tools/Win.mk
     new a4b378a583 mqueue: use to DEBUGASSERT to cover some param check
     new a9c647d418 semaphore: move param check to sem_xx level

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 libs/libc/semaphore/sem_init.c  |  40 +++++-----
 sched/mqueue/mq_send.c          |   4 +-
 sched/semaphore/sem_clockwait.c |  24 +++---
 sched/semaphore/sem_destroy.c   |  51 ++++++------
 sched/semaphore/sem_post.c      | 169 ++++++++++++++++++++--------------------
 sched/semaphore/sem_trywait.c   |  50 ++++++------
 sched/semaphore/sem_wait.c      | 169 ++++++++++++++++++++--------------------
 7 files changed, 258 insertions(+), 249 deletions(-)


[incubator-nuttx] 01/02: mqueue: use to DEBUGASSERT to cover some param check

Posted by xi...@apache.org.
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/incubator-nuttx.git

commit a4b378a583027d7493b45b25f6554514cb0d609e
Author: ligd <li...@xiaomi.com>
AuthorDate: Fri Sep 9 18:00:06 2022 +0800

    mqueue: use to DEBUGASSERT to cover some param check
    
    for the speed improve
    
    Signed-off-by: ligd <li...@xiaomi.com>
---
 sched/mqueue/mq_send.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sched/mqueue/mq_send.c b/sched/mqueue/mq_send.c
index 60eb932fc1..fca28a581b 100644
--- a/sched/mqueue/mq_send.c
+++ b/sched/mqueue/mq_send.c
@@ -117,6 +117,7 @@ int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
       /* Now allocate the message. */
 
       mqmsg = nxmq_alloc_msg();
+      DEBUGASSERT(mqmsg != NULL);
 
       /* Check if the message was successfully allocated */
 
@@ -129,8 +130,7 @@ int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
        * to be exceeded in that case.
        */
 
-      ret = (mqmsg == NULL) ? -ENOMEM :
-            nxmq_do_send(msgq, mqmsg, msg, msglen, prio);
+      ret = nxmq_do_send(msgq, mqmsg, msg, msglen, prio);
     }
 
   leave_critical_section(flags);


[incubator-nuttx] 02/02: semaphore: move param check to sem_xx level

Posted by xi...@apache.org.
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/incubator-nuttx.git

commit a9c647d418abceb744f42a7a3600bc256b871474
Author: ligd <li...@xiaomi.com>
AuthorDate: Fri Oct 21 00:00:08 2022 +0800

    semaphore: move param check to sem_xx level
    
    for the speed improve
    
    Signed-off-by: ligd <li...@xiaomi.com>
---
 libs/libc/semaphore/sem_init.c  |  40 +++++-----
 sched/semaphore/sem_clockwait.c |  24 +++---
 sched/semaphore/sem_destroy.c   |  51 ++++++------
 sched/semaphore/sem_post.c      | 169 ++++++++++++++++++++--------------------
 sched/semaphore/sem_trywait.c   |  50 ++++++------
 sched/semaphore/sem_wait.c      | 169 ++++++++++++++++++++--------------------
 6 files changed, 256 insertions(+), 247 deletions(-)

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 <nuttx/config.h>
 
 #include <sys/types.h>
+#include <assert.h>
 #include <limits.h>
 #include <errno.h>
 
@@ -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 */
-
-  if (sem != NULL)
+  DEBUGASSERT(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.
-       */
-
-      sched_lock();
+  /* 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();
 #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));
-
-          if (stcb != NULL)
-            {
-              /* The task will be the new holder of the semaphore when
-               * it is awakened.
-               */
-
-              nxsem_add_holder_tcb(stcb, sem);
+          nxsem_add_holder_tcb(stcb, sem);
 
-              /* Stop the watchdog timer */
+          /* Stop the watchdog timer */
 
-              if (WDOG_ISACTIVE(&stcb->waitdog))
-                {
-                  wd_cancel(&stcb->waitdog);
-                }
+          if (WDOG_ISACTIVE(&stcb->waitdog))
+            {
+              wd_cancel(&stcb->waitdog);
+            }
 
-              /* Restart the waiting task. */
+          /* Restart the waiting task. */
 
-              up_unblock_task(stcb);
-            }
+          up_unblock_task(stcb);
+        }
 #if 0 /* REVISIT:  This can fire on IOB throttle semaphore */
-          else
-            {
-              /* This should not happen. */
+      else
+        {
+          /* This should not happen. */
 
-              DEBUGPANIC();
-            }
-#endif
+          DEBUGPANIC();
         }
+#endif
+    }
 
-      /* 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.
-       */
+  /* 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();
+  nxsem_restore_baseprio(stcb, sem);
+  sched_unlock();
 #endif
-      ret = OK;
 
-      /* Interrupts may now be enabled. */
+  /* Interrupts may now be enabled. */
 
-      leave_critical_section(flags);
-    }
+  leave_critical_section(flags);
 
-  return ret;
+  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 */
+  /* The following operations must be performed with interrupts disabled
+   * because sem_post() may be called from an interrupt handler.
+   */
 
-      if (sem->semcount > 0)
-        {
-          /* It is, let the task take the semaphore */
+  flags = enter_critical_section();
 
-          sem->semcount--;
-          nxsem_add_holder(sem);
-          rtcb->waitobj = NULL;
-          ret = OK;
-        }
-      else
-        {
-          /* Semaphore is not available */
+  /* If the semaphore is available, give it to the requesting task */
 
-          ret = -EAGAIN;
-        }
-
-      /* Interrupts may now be enabled. */
+  if (sem->semcount > 0)
+    {
+      /* It is, let the task take the semaphore */
 
-      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.
-           */
-
-          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.
-           */
-
-          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.
-           */
-
-          /* 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;
+      /* 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;
+
+      /* 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);
+
+      /* 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.
+       */
+
+      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())