Unix domain/FIFOs: Fix a race condition between FIFO buffer operations and the opening and closing of FIFOs which necessary when the FIFOs are used to support Unix domain, datagram sockets. The default policy is the deallocate FIFO buffering when the last client closes the pipe. When when used for datagram communicatinos, packets left in the FIFO will be lost. Some like UDP read-ahead is needed: The buffered data in the FIFO needs to be retained until the reader gets a chance to re-open the FIFO. Added an ioctl (PIPEIOC_POLICY) to control the buffer policy. Default (0) is the legacy behavior; Unix domain datagram logic sets the alternative policy so that the packet data persists after the FIFO is closed.

9 files changed, 188 insertions, 82 deletions

diff --git a/apps/examples/udgram/udgram_client.c b/apps/examples/udgram/udgram_client.c
index cec238d05..f3ce4558f 100644
--- a/apps/examples/udgram/udgram_client.c
+++ b/apps/examples/udgram/udgram_client.c
@@ -93,7 +93,7 @@ int client_main(int argc, char *argv[])
   sockfd = socket(PF_LOCAL, SOCK_DGRAM, 0);
   if (sockfd < 0)
     {
-      printf("client socket failure %d\n", errno);
+      printf("client: ERROR socket failure %d\n", errno);
       return 1;
     }
 
@@ -127,13 +127,13 @@ int client_main(int argc, char *argv[])
 
       if (nbytes < 0)
         {
-          printf("client: %d. sendto failed: %d\n", offset, errno);
+          printf("client: %d. ERROR sendto failed: %d\n", offset, errno);
           close(sockfd);
           return 1;
         }
       else if (nbytes != SENDSIZE)
         {
-          printf("client: %d. Bad send length: %d Expected: %d\n",
+          printf("client: %d. ERROR Bad send length: %d Expected: %d\n",
                  offset, nbytes, SENDSIZE);
           close(sockfd);
           return 1;
diff --git a/apps/examples/udgram/udgram_server.c b/apps/examples/udgram/udgram_server.c
index f09904881..5a114e50f 100644
--- a/apps/examples/udgram/udgram_server.c
+++ b/apps/examples/udgram/udgram_server.c
@@ -122,7 +122,7 @@ int server_main(int argc, char *argv[])
 
   if (bind(sockfd, (struct sockaddr*)&server, addrlen) < 0)
     {
-      printf("server: bind failure: %d\n", errno);
+      printf("server: ERROR bind failure: %d\n", errno);
       return 1;
     }
 
@@ -137,7 +137,7 @@ int server_main(int argc, char *argv[])
 
       if (nbytes < 0)
         {
-          printf("server: %d. recv failed: %d\n", offset, errno);
+          printf("server: %d. ERROR recv failed: %d\n", offset, errno);
           close(sockfd);
           return 1;
         }
@@ -178,7 +178,7 @@ int server_main(int argc, char *argv[])
 
       if (nbytes != SENDSIZE)
         {
-          printf("server: %d. recv size incorrect: %d vs %d\n",
+          printf("server: %d. ERROR recv size incorrect: %d vs %d\n",
                  offset, nbytes, SENDSIZE);
           close(sockfd);
           return 1;
@@ -186,13 +186,13 @@ int server_main(int argc, char *argv[])
 
       if (offset < inbuf[0])
         {
-          printf("server: %d. %d packets lost, resetting offset\n",
+          printf("server: %d. ERROR %d packets lost, resetting offset\n",
                  offset, inbuf[0] - offset);
           offset = inbuf[0];
         }
       else if (offset > inbuf[0])
         {
-          printf("server: %d. Bad offset in buffer: %d\n",
+          printf("server: %d. ERROR Bad offset in buffer: %d\n",
                  offset, inbuf[0]);
           close(sockfd);
           return 1;
@@ -200,7 +200,7 @@ int server_main(int argc, char *argv[])
 
       if (!check_buffer(inbuf))
         {
-          printf("server: %d. Bad buffer contents\n", offset);
+          printf("server: %d. ERROR Bad buffer contents\n", offset);
           close(sockfd);
           return 1;
         }
diff --git a/nuttx/drivers/pipes/fifo.c b/nuttx/drivers/pipes/fifo.c
index 57873ca99..5fcc75b4b 100644
--- a/nuttx/drivers/pipes/fifo.c
+++ b/nuttx/drivers/pipes/fifo.c
@@ -73,7 +73,7 @@ static const struct file_operations fifo_fops =
   pipecommon_read,  /* read */
   pipecommon_write, /* write */
   0,                /* seek */
-  0                 /* ioctl */
+  pipecommon_ioctl  /* ioctl */
 #ifndef CONFIG_DISABLE_POLL
   , pipecommon_poll /* poll */
 #endif
diff --git a/nuttx/drivers/pipes/pipe.c b/nuttx/drivers/pipes/pipe.c
index 60764c510..6df5086e8 100644
--- a/nuttx/drivers/pipes/pipe.c
+++ b/nuttx/drivers/pipes/pipe.c
@@ -82,7 +82,7 @@ static const struct file_operations pipe_fops =
   pipecommon_read,   /* read */
   pipecommon_write,  /* write */
   0,                 /* seek */
-  0                  /* ioctl */
+  pipecommon_ioctl   /* ioctl */
 #ifndef CONFIG_DISABLE_POLL
   , pipecommon_poll  /* poll */
 #endif
diff --git a/nuttx/drivers/pipes/pipe_common.c b/nuttx/drivers/pipes/pipe_common.c
index 2acaeab97..fe56f8d8a 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, 2011 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2008-2009, 2011, 2015 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41,6 +41,7 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/ioctl.h>
 #include <stdint.h>
 #include <stdbool.h>
 #include <stdlib.h>
@@ -188,13 +189,7 @@ int pipecommon_open(FAR struct file *filep)
   int                sval;
   int                ret;
 
-  /* Some sanity checking */
-#if CONFIG_DEBUG
-  if (!dev)
-    {
-       return -EBADF;
-    }
-#endif
+  DEBUGASSERT(dev);
 
   /* 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.
@@ -208,9 +203,12 @@ int pipecommon_open(FAR struct file *filep)
       return -get_errno();
     }
 
-  /* If this the first reference on the device, then allocate the buffer */
+  /* If this the first reference on the device, then allocate the buffer.  In the
+   * case of d_policy == 1, the buffer already be present when the pipe is
+   * first opened.
+   */
 
-  if (dev->d_refs == 0)
+  if (dev->d_refs == 0 && dev->d_buffer == NULL)
     {
       dev->d_buffer = (uint8_t*)kmm_malloc(CONFIG_DEV_PIPE_SIZE);
       if (!dev->d_buffer)
@@ -243,11 +241,17 @@ int pipecommon_open(FAR struct file *filep)
         }
     }
 
-  /* If opened for read-only, then wait for at least one writer on the pipe */
+  /* If opened for read-only, then wait for either (1) at least one writer
+   * on the pipe (policy == 0), or (2) until there is buffered data to be
+   * read (policy == 1).
+   */
 
   sched_lock();
   (void)sem_post(&dev->d_bfsem);
-  if ((filep->f_oflags & O_RDWR) == O_RDONLY && dev->d_nwriters < 1)
+
+  if ((filep->f_oflags & O_RDWR) == O_RDONLY &&  /* Read-only */
+      dev->d_nwriters < 1 &&                     /* No writers on the pipe */
+      dev->d_wrndx == dev->d_rdndx)              /* Buffer is empty */
     {
       /* 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
@@ -289,13 +293,7 @@ int pipecommon_close(FAR struct file *filep)
   struct pipe_dev_s *dev   = inode->i_private;
   int                sval;
 
-  /* Some sanity checking */
-#if CONFIG_DEBUG
-  if (!dev)
-    {
-       return -EBADF;
-    }
-#endif
+  DEBUGASSERT(dev && dev->d_refs > 0);
 
   /* Make sure that we have exclusive access to the device structure.
    * NOTE: close() is supposed to return EINTR if interrupted, however
@@ -304,15 +302,17 @@ int pipecommon_close(FAR struct file *filep)
 
   pipecommon_semtake(&dev->d_bfsem);
 
+  /* Decrement the number of references on the pipe.  Check if there are
+   * still oustanding references to the pipe.
+   */
+
   /* Check if the decremented reference count would go to zero */
 
-  if (dev->d_refs > 1)
+  if (--dev->d_refs > 0)
     {
-      /* No.. then just decrement the reference count */
-
-      dev->d_refs--;
-
-      /* If opened for writing, decrement the count of writers on on the pipe instance */
+      /* No more references.. If opened for writing, decrement the count of
+       * writers on the pipe instance.
+       */
 
       if ((filep->f_oflags & O_WROK) != 0)
         {
@@ -329,9 +329,16 @@ int pipecommon_close(FAR struct file *filep)
             }
         }
     }
-  else
+
+  /* What is the buffer management policy?  Do we free the buffe when the
+   * last client closes the pipe (d_policy == 0), or when the buffer becomes
+   * empty (d_policy).  In the latter case, the buffer data will remain
+   * valid and can be obtained when the pipe is re-opened.
+   */
+
+  else if (dev->d_policy == 0 || dev->d_wrndx == dev->d_rdndx)
     {
-      /* Yes... deallocate the buffer */
+      /* Policy 0 or the buffer is empty ... deallocate the buffer now. */
 
       kmm_free(dev->d_buffer);
       dev->d_buffer = NULL;
@@ -363,13 +370,7 @@ ssize_t pipecommon_read(FAR struct file *filep, FAR char *buffer, size_t len)
   int                sval;
   int                ret;
 
-  /* Some sanity checking */
-#if CONFIG_DEBUG
-  if (!dev)
-    {
-      return -ENODEV;
-    }
-#endif
+  DEBUGASSERT(dev);
 
   /* Make sure that we have exclusive access to the device structure */
 
@@ -453,15 +454,7 @@ ssize_t pipecommon_write(FAR struct file *filep, FAR const char *buffer, size_t
   int                nxtwrndx;
   int                sval;
 
-  /* Some sanity checking */
-
-#if CONFIG_DEBUG
-  if (!dev)
-    {
-      return -ENODEV;
-    }
-#endif
-
+  DEBUGASSERT(dev);
   pipe_dumpbuffer("To PIPE:", (uint8_t*)buffer, len);
 
   /* At present, this method cannot be called from interrupt handlers.  That is
@@ -581,14 +574,7 @@ int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
   int                    ret      = OK;
   int                    i;
 
-  /* Some sanity checking */
-
-#if CONFIG_DEBUG
-  if (!dev || !fds)
-    {
-      return -ENODEV;
-    }
-#endif
+  DEBUGASSERT(dev && fds);
 
   /* Are we setting up the poll?  Or tearing it down? */
 
@@ -679,4 +665,23 @@ errout:
 }
 #endif
 
+/****************************************************************************
+ * Name: pipecommon_ioctl
+ ****************************************************************************/
+
+int  pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+{
+  FAR struct inode      *inode    = filep->f_inode;
+  FAR struct pipe_dev_s *dev      = inode->i_private;
+
+  /* Only one command supported */
+
+  if (cmd == PIPEIOC_POLICY)
+    {
+      dev->d_policy = (arg != 0);
+      return OK;
+    }
+
+  return -ENOTTY;
+}
 #endif /* CONFIG_DEV_PIPE_SIZE > 0 */
diff --git a/nuttx/drivers/pipes/pipe_common.h b/nuttx/drivers/pipes/pipe_common.h
index c22fb981e..77e359dc8 100644
--- a/nuttx/drivers/pipes/pipe_common.h
+++ b/nuttx/drivers/pipes/pipe_common.h
@@ -96,6 +96,7 @@ struct pipe_dev_s
   uint8_t    d_refs;        /* References counts on pipe (limited to 255) */
   uint8_t    d_nwriters;    /* Number of reference counts for write access */
   uint8_t    d_pipeno;      /* Pipe minor number */
+  bool       d_policy;      /* Buffer policy: 0=free on close; 1=free on empty */
   uint8_t   *d_buffer;      /* Buffer allocated when device opened */
 
   /* The following is a list if poll structures of threads waiting for
@@ -119,14 +120,16 @@ extern "C" {
 #  define EXTERN extern
 #endif
 
-EXTERN FAR struct pipe_dev_s *pipecommon_allocdev(void);
-EXTERN void    pipecommon_freedev(FAR struct pipe_dev_s *dev);
-EXTERN int     pipecommon_open(FAR struct file *filep);
-EXTERN int     pipecommon_close(FAR struct file *filep);
-EXTERN ssize_t pipecommon_read(FAR struct file *, FAR char *, size_t);
-EXTERN ssize_t pipecommon_write(FAR struct file *, FAR const char *, size_t);
+FAR struct pipe_dev_s *pipecommon_allocdev(void);
+void    pipecommon_freedev(FAR struct pipe_dev_s *dev);
+int     pipecommon_open(FAR struct file *filep);
+int     pipecommon_close(FAR struct file *filep);
+ssize_t pipecommon_read(FAR struct file *, FAR char *, size_t);
+ssize_t pipecommon_write(FAR struct file *, FAR const char *, size_t);
+int     pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+
 #ifndef CONFIG_DISABLE_POLL
-EXTERN int     pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
+int     pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds,
                                bool setup);
 #endif
 
diff --git a/nuttx/include/nuttx/fs/ioctl.h b/nuttx/include/nuttx/fs/ioctl.h
index fc904495f..8f0835755 100644
--- a/nuttx/include/nuttx/fs/ioctl.h
+++ b/nuttx/include/nuttx/fs/ioctl.h
@@ -73,6 +73,7 @@
 #define _TCIOCBASE      (0x1400) /* Timer ioctl commands */
 #define _DJOYBASE       (0x1500) /* Discrete joystick ioctl commands */
 #define _AJOYBASE       (0x1600) /* Analog joystick ioctl commands */
+#define _PIPEBASE       (0x1700) /* FIFO/pipe ioctl commands */
 
 /* Macros used to manage ioctl commands */
 
@@ -316,6 +317,18 @@
 #define _AJOYIOCVALID(c)   (_IOC_TYPE(c)==_AJOYBASE)
 #define _AJOYIOC(nr)       _IOC(_AJOYBASE,nr)
 
+/* FIFOs and pipe driver ioctl definitions **********************************/
+
+#define _PIPEIOCVALID(c)   (_IOC_TYPE(c)==_PIPEBASE)
+#define _PIPEIOC(nr)       _IOC(_PIPEBASE,nr)
+
+#define PIPEIOC_POLICY     _PIPEIOC(0x0001)  /* Set buffer policy
+                                              * IN: unsigned long integer
+                                              *     0=free on last close
+                                              *       (default)
+                                              *     1=fre when empty
+                                              * OUT: None */
+
 /****************************************************************************
  * Public Type Definitions
  ****************************************************************************/
diff --git a/nuttx/net/local/local_fifo.c b/nuttx/net/local/local_fifo.c
index bd9b141d9..071fcdfb2 100644
--- a/nuttx/net/local/local_fifo.c
+++ b/nuttx/net/local/local_fifo.c
@@ -41,6 +41,8 @@
 #if defined(CONFIG_NET) && defined(CONFIG_NET_LOCAL)
 
 #include <sys/stat.h>
+#include <sys/ioctl.h>
+
 #include <stdbool.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -49,6 +51,7 @@
 #include <errno.h>
 #include <assert.h>
 
+
 #include "local/local.h"
 
 /****************************************************************************
@@ -185,8 +188,6 @@ static int local_create_fifo(FAR const char *path)
 
 static int local_release_fifo(FAR const char *path)
 {
-  int ret;
-
   /* Unlink the client-to-server FIFO if it exists. */
 
   if (local_fifo_exists(path))
@@ -197,6 +198,8 @@ static int local_release_fifo(FAR const char *path)
        */
 #warning Missing logic
 #if 0
+      int ret;
+
       ret = unlink(path);
       if (ret < 0)
         {
@@ -222,8 +225,7 @@ static int local_release_fifo(FAR const char *path)
  *
  ****************************************************************************/
 
-static inline int local_rx_open(FAR struct local_conn_s *conn,
-                                FAR const char *path)
+static int local_rx_open(FAR struct local_conn_s *conn, FAR const char *path)
 {
   conn->lc_infd = open(path, O_RDONLY);
   if (conn->lc_infd < 0)
@@ -256,8 +258,7 @@ static inline int local_rx_open(FAR struct local_conn_s *conn,
  *
  ****************************************************************************/
 
-static inline int local_tx_open(FAR struct local_conn_s *conn,
-                                FAR const char *path)
+static int local_tx_open(FAR struct local_conn_s *conn, FAR const char *path)
 {
   conn->lc_outfd = open(path, O_WRONLY);
   if (conn->lc_outfd < 0)
@@ -283,6 +284,36 @@ static inline int local_tx_open(FAR struct local_conn_s *conn,
 }
 
 /****************************************************************************
+ * Name: local_set_policy
+ *
+ * Description:
+ *   Set the FIFO buffer policy:
+ *
+ *     0=Free FIFO resources when the last reference is closed
+ *     1=Free FIFO resources when the buffer is empty.
+ *
+ ****************************************************************************/
+
+static int local_set_policy(int fd, unsigned long policy)
+{
+  int ret;
+
+  /* Set the buffer policy */
+
+  ret = ioctl(fd, PIPEIOC_POLICY, policy);
+  if (ret < 0)
+    {
+      int errcode = get_errno();
+      DEBUGASSERT(errcode > 0);
+
+      ndbg("ERROR: Failed to set FIFO buffer policty: %d\n", errcode);
+      return -errcode;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
  * Public Functions
  ****************************************************************************/
 
@@ -390,6 +421,7 @@ int local_release_halfduplex(FAR struct local_conn_s *conn)
 int local_open_client_rx(FAR struct local_conn_s *client)
 {
   char path[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the server-to-client path name */
 
@@ -397,7 +429,15 @@ int local_open_client_rx(FAR struct local_conn_s *client)
 
   /* Then open the file for read-only access */
 
-  return local_rx_open(client, path);
+  ret = local_rx_open(client, path);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the last reference is closed */
+
+      ret = local_set_policy(client->lc_infd, 0);
+    }
+
+  return ret;
 }
 
 /****************************************************************************
@@ -411,6 +451,7 @@ int local_open_client_rx(FAR struct local_conn_s *client)
 int local_open_client_tx(FAR struct local_conn_s *client)
 {
   char path[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the client-to-server path name */
 
@@ -418,7 +459,15 @@ int local_open_client_tx(FAR struct local_conn_s *client)
 
   /* Then open the file for write-only access */
 
-  return local_tx_open(client, path);
+  ret = local_tx_open(client, path);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the last reference is closed */
+
+      ret = local_set_policy(client->lc_outfd, 0);
+    }
+
+  return ret;
 }
 
 /****************************************************************************
@@ -432,6 +481,7 @@ int local_open_client_tx(FAR struct local_conn_s *client)
 int local_open_server_rx(FAR struct local_conn_s *server)
 {
   char path[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the client-to-server path name */
 
@@ -439,7 +489,15 @@ int local_open_server_rx(FAR struct local_conn_s *server)
 
   /* Then open the file for write-only access */
 
-  return local_rx_open(server, path);
+  ret = local_rx_open(server, path);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the last reference is closed */
+
+      ret = local_set_policy(server->lc_infd, 0);
+    }
+
+  return ret;
 }
 
 /****************************************************************************
@@ -453,6 +511,7 @@ int local_open_server_rx(FAR struct local_conn_s *server)
 int local_open_server_tx(FAR struct local_conn_s *server)
 {
   char path[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the server-to-client path name */
 
@@ -460,7 +519,15 @@ int local_open_server_tx(FAR struct local_conn_s *server)
 
   /* Then open the file for read-only access */
 
-  return local_tx_open(server, path);
+  ret = local_tx_open(server, path);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the last reference is closed */
+
+      ret = local_set_policy(server->lc_outfd, 0);
+    }
+
+  return ret;
 }
 
 /****************************************************************************
@@ -474,6 +541,7 @@ int local_open_server_tx(FAR struct local_conn_s *server)
 int local_open_receiver(FAR struct local_conn_s *conn)
 {
   char path[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the server-to-client path name */
 
@@ -481,7 +549,15 @@ int local_open_receiver(FAR struct local_conn_s *conn)
 
   /* Then open the file for read-only access */
 
-  return local_rx_open(conn, path);
+  ret = local_rx_open(conn, path);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the buffer is empty. */
+
+      ret = local_set_policy(conn->lc_infd, 1);
+    }
+
+  return ret;
 }
 
 /****************************************************************************
@@ -495,6 +571,7 @@ int local_open_receiver(FAR struct local_conn_s *conn)
 int local_open_sender(FAR struct local_conn_s *conn, FAR const char *path)
 {
   char fullpath[LOCAL_FULLPATH_LEN];
+  int ret;
 
   /* Get the server-to-client path name */
 
@@ -502,7 +579,15 @@ int local_open_sender(FAR struct local_conn_s *conn, FAR const char *path)
 
   /* Then open the file for read-only access */
 
-  return local_tx_open(conn, fullpath);
+  ret = local_tx_open(conn, fullpath);
+  if (ret == OK)
+    {
+      /* Policy: Free FIFO resources when the buffer is empty. */
+
+      ret = local_set_policy(conn->lc_outfd, 1);
+    }
+
+  return ret;
 }
 
 #endif /* CONFIG_NET && CONFIG_NET_LOCAL */
diff --git a/nuttx/net/local/local_recvfrom.c b/nuttx/net/local/local_recvfrom.c
index 97109ea1a..b3346b7f7 100644
--- a/nuttx/net/local/local_recvfrom.c
+++ b/nuttx/net/local/local_recvfrom.c
@@ -337,7 +337,7 @@ psock_dgram_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len,
 
           /* Adjust the number of bytes remaining to be read from the packet */
 
-          DEBUGASSERT(tmplen <= remain);
+          DEBUGASSERT(tmplen <= remaining);
           remaining -= tmplen;
         }
       while (remaining > 0);