Fix pipe/fifo open logic: semaphore wait in open() must abort if a signal is received

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3327 42af7a65-404d-4744-a932-0658087f49c3

5 files changed, 80 insertions, 45 deletions

diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog
index c9be40c5d..23520da45 100644
--- a/nuttx/ChangeLog
+++ b/nuttx/ChangeLog
@@ -1499,3 +1499,8 @@
 
 	    nsh> cat test.txt
 	    This is a test
+
+	* drvers/pipes/pipe_common.c:  Driver open method eas not returning an EINTR
+	  error when it received a signal.  Instead, it just re-started the wait.  This
+	  makes it impossible to kill a background pipe operation from NSH.
+
diff --git a/nuttx/Documentation/NuttX.html b/nuttx/Documentation/NuttX.html
index 15ee60dae..44fc71a9e 100644
--- a/nuttx/Documentation/NuttX.html
+++ b/nuttx/Documentation/NuttX.html
@@ -8,7 +8,7 @@
   <tr align="center" bgcolor="#e4e4e4">
     <td>
       <h1><big><font color="#3c34ec"><i>NuttX RTOS</i></font></big></h1>
-      <p>Last Updated: February 28, 2011</p>
+      <p>Last Updated: March 1, 2011</p>
     </td>
   </tr>
 </table>
@@ -2102,6 +2102,10 @@ nuttx-5.19 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
         nsh> cat test.txt
         This is a test
 
+    * drvers/pipes/pipe_common.c:  Driver open method eas not returning an EINTR
+      error when it received a signal.  Instead, it just re-started the wait.  This
+      makes it impossible to kill a background pipe operation from NSH.
+
 pascal-2.1 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 
 buildroot-1.10 2011-xx-xx <spudmonkey@racsa.co.cr>
diff --git a/nuttx/drivers/pipes/pipe_common.c b/nuttx/drivers/pipes/pipe_common.c
index e22678688..a6af41fe1 100644
--- a/nuttx/drivers/pipes/pipe_common.c
+++ b/nuttx/drivers/pipes/pipe_common.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * drivers/pipes/pipe_common.c
  *
- *   Copyright (C) 2008-2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2008-2009, 2011 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -184,6 +184,7 @@ int pipecommon_open(FAR struct file *filep)
   struct inode      *inode = filep->f_inode;
   struct pipe_dev_s *dev   = inode->i_private;
   int                sval;
+  int                ret;
  
   /* Some sanity checking */
 #if CONFIG_DEBUG
@@ -192,66 +193,87 @@ int pipecommon_open(FAR struct file *filep)
        return -EBADF;
     }
 #endif
-  /* Make sure that we have exclusive access to the device structure */
+  /* Make sure that we have exclusive access to the device structure.  The
+   * sem_wait() call should fail only if we are awakened by a signal.
+   */
 
-  if (sem_wait(&dev->d_bfsem) == 0)
+  ret = sem_wait(&dev->d_bfsem);
+  if (ret != OK)
     {
-      /* If this the first reference on the device, then allocate the buffer */
+      fdbg("sem_wait failed: %d\n", errno);
+      DEBUGASSERT(errno > 0);
+      return -errno;
+    }
 
-      if (dev->d_refs == 0)
+  /* If this the first reference on the device, then allocate the buffer */
+
+  if (dev->d_refs == 0)
+    {
+      dev->d_buffer = (uint8_t*)malloc(CONFIG_DEV_PIPE_SIZE);
+      if (!dev->d_buffer)
         {
-          dev->d_buffer = (uint8_t*)malloc(CONFIG_DEV_PIPE_SIZE);
-          if (!dev->d_buffer)
-            {
-              (void)sem_post(&dev->d_bfsem);
-              return -ENOMEM;
-            }
+          (void)sem_post(&dev->d_bfsem);
+          return -ENOMEM;
         }
+    }
 
-      /* Increment the reference count on the pipe instance */
+  /* Increment the reference count on the pipe instance */
 
-      dev->d_refs++;
+  dev->d_refs++;
 
-      /* If opened for writing, increment the count of writers on on the pipe instance */
+  /* If opened for writing, increment the count of writers on on the pipe instance */
 
-      if ((filep->f_oflags & O_WROK) != 0)
-        {
-          dev->d_nwriters++;
+  if ((filep->f_oflags & O_WROK) != 0)
+    {
+      dev->d_nwriters++;
 
-          /* If this this is the first writer, then the read semaphore indicates the
-           * number of readers waiting for the first writer.  Wake them all up.
-           */
+      /* If this this is the first writer, then the read semaphore indicates the
+       * number of readers waiting for the first writer.  Wake them all up.
+       */
 
-          if (dev->d_nwriters == 1)
+      if (dev->d_nwriters == 1)
+        {
+          while (sem_getvalue(&dev->d_rdsem, &sval) == 0 && sval < 0)
             {
-              while (sem_getvalue(&dev->d_rdsem, &sval) == 0 && sval < 0)
-                {
-                  sem_post(&dev->d_rdsem);
-                }
+              sem_post(&dev->d_rdsem);
             }
         }
+    }
 
-      /* If opened for read-only, then wait for at least one writer on the pipe */
+  /* If opened for read-only, then wait for at least one writer on the pipe */
 
-      sched_lock();
-      (void)sem_post(&dev->d_bfsem);
-      if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
+  sched_lock();
+  (void)sem_post(&dev->d_bfsem);
+  if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
+    {
+      /* NOTE: d_rdsem is normally used when the read logic waits for more
+       * data to be written.  But until the first writer has opened the
+       * pipe, the meaning is different: it is used prevent O_RDONLY open
+       * calls from returning until there is at least one writer on the pipe.
+       * This is required both by spec and also because it prevents
+       * subsequent read() calls from returning end-of-file because there is
+       * no writer on the pipe.
+       */
+
+      ret = sem_wait(&dev->d_rdsem);
+      if (ret != OK)
         {
-          /* NOTE: d_rdsem is normally used when the read logic waits for more
-           * data to be written.  But until the first writer has opened the
-           * pipe, the meaning is different: it is used prevent O_RDONLY open
-           * calls from returning until there is at least one writer on the pipe.
-           * This is required both by spec and also because it prevents
-           * subsequent read() calls from returning end-of-file because there is
-           * no writer on the pipe.
+          /* The sem_wait() call should fail only if we are awakened by
+           * a signal.
            */
 
-          pipecommon_semtake(&dev->d_rdsem);
+          fdbg("sem_wait failed: %d\n", errno);
+          DEBUGASSERT(errno > 0);
+          ret = -errno;
+
+          /* Immediately close the pipe that we just opened */
+
+          (void)pipecommon_close(filep);
         }
-      sched_unlock();
-      return OK;
-  }
-  return ERROR;
+    }
+
+  sched_unlock();
+  return ret;
 }
 
 /****************************************************************************
diff --git a/nuttx/examples/nsh/nsh_main.c b/nuttx/examples/nsh/nsh_main.c
index aea823be8..9981ad073 100644
--- a/nuttx/examples/nsh/nsh_main.c
+++ b/nuttx/examples/nsh/nsh_main.c
@@ -360,7 +360,7 @@ const char g_fmtcmdfailed[]      = "nsh: %s: %s failed: %d\n";
 const char g_fmtcmdoutofmemory[] = "nsh: %s: out of memory\n";
 const char g_fmtinternalerror[]  = "nsh: %s: Internal error\n";
 #ifndef CONFIG_DISABLE_SIGNALS
-const char g_fmtsignalrecvd[]    = "nsh: %s: Signal received\n";
+const char g_fmtsignalrecvd[]    = "nsh: %s: Interrupted by signal\n";
 #endif
 
 /****************************************************************************
diff --git a/nuttx/fs/fs_open.c b/nuttx/fs/fs_open.c
index 76b941f95..467eef858 100644
--- a/nuttx/fs/fs_open.c
+++ b/nuttx/fs/fs_open.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * fs_open.c
  *
- *   Copyright (C) 2007, 2008, 2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2009, 2011 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -69,6 +69,10 @@ int inode_checkflags(FAR struct inode *inode, int oflags)
     }
 }
 
+/****************************************************************************
+ * Name: open
+ ****************************************************************************/
+
 int open(const char *path, int oflags, ...)
 {
   struct filelist  *list;
@@ -181,7 +185,7 @@ int open(const char *path, int oflags, ...)
  errout_with_inode:
   inode_release(inode);
  errout:
-  *get_errno_ptr() = ret;
+  errno = ret;
   return ERROR;
 }