From 39a5238c43b85706ebd7e2322d3b7d331e07718d Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sat, 31 Jan 2015 12:05:01 -0600 Subject: [PATCH] Pipes/FIFOs: Implement the unlink method. If the pipe/FIFO is unlinked, it will marked the pipe/FIFO as unliked. If/when all open references to the driver are closed, all of the driver resources will be freed. --- drivers/pipes/fifo.c | 21 +++++----- drivers/pipes/pipe.c | 35 ++++++++-------- drivers/pipes/pipe_common.c | 83 +++++++++++++++++++++++++++++++------ drivers/pipes/pipe_common.h | 20 +++++++-- 4 files changed, 116 insertions(+), 43 deletions(-) diff --git a/drivers/pipes/fifo.c b/drivers/pipes/fifo.c index 5fcc75b4ba..0c8f0d63f6 100644 --- a/drivers/pipes/fifo.c +++ b/drivers/pipes/fifo.c @@ -1,7 +1,7 @@ /**************************************************************************** * drivers/pipes/fifo.c * - * Copyright (C) 2008-2009, 2014 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2009, 2014-2015 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -73,10 +73,11 @@ static const struct file_operations fifo_fops = pipecommon_read, /* read */ pipecommon_write, /* write */ 0, /* seek */ - pipecommon_ioctl /* ioctl */ + pipecommon_ioctl, /* ioctl */ #ifndef CONFIG_DISABLE_POLL - , pipecommon_poll /* poll */ + pipecommon_poll, /* poll */ #endif + pipecommon_unlink /* unlink */ }; /**************************************************************************** @@ -92,15 +93,15 @@ static const struct file_operations fifo_fops = * * Description: * mkfifo() makes a FIFO device driver file with name 'pathname.' Unlike - * Linux, a NuttX FIFO is not a special file type but simply a device driver - * instance. 'mode' specifies the FIFO's permissions. + * Linux, a NuttX FIFO is not a special file type but simply a device + * driver instance. 'mode' specifies the FIFO's permissions. * * Once the FIFO has been created by mkfifo(), any thread can open it for - * reading or writing, in the same way as an ordinary file. However, it must - * have been opened from both reading and writing before input or output - * can be performed. This FIFO implementation will block all attempts to - * open a FIFO read-only until at least one thread has opened the FIFO for - * writing. + * reading or writing, in the same way as an ordinary file. However, it + * must have been opened from both reading and writing before input or + * output can be performed. This FIFO implementation will block all + * attempts to open a FIFO read-only until at least one thread has opened + * the FIFO for writing. * * If all threads that write to the FIFO have closed, subsequent calls to * read() on the FIFO will return 0 (end-of-file). diff --git a/drivers/pipes/pipe.c b/drivers/pipes/pipe.c index 6df5086e8c..df29e3ed79 100644 --- a/drivers/pipes/pipe.c +++ b/drivers/pipes/pipe.c @@ -1,7 +1,7 @@ /**************************************************************************** * drivers/pipes/pipe.c * - * Copyright (C) 2008-2009 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2009, 2015 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -82,10 +82,11 @@ static const struct file_operations pipe_fops = pipecommon_read, /* read */ pipecommon_write, /* write */ 0, /* seek */ - pipecommon_ioctl /* ioctl */ + pipecommon_ioctl, /* ioctl */ #ifndef CONFIG_DISABLE_POLL - , pipecommon_poll /* poll */ + pipecommon_poll, /* poll */ #endif + pipecommon_unlink /* unlink */ }; static sem_t g_pipesem = SEM_INITIALIZER(1); @@ -124,7 +125,9 @@ static inline int pipe_allocate(void) static inline void pipe_free(int pipeno) { - int ret = sem_wait(&g_pipesem); + int ret; + + ret = sem_wait(&g_pipesem); if (ret == 0) { g_pipeset &= ~(1 << pipeno); @@ -138,17 +141,11 @@ static inline void pipe_free(int pipeno) static int pipe_close(FAR struct file *filep) { - struct inode *inode = filep->f_inode; - struct pipe_dev_s *dev = inode->i_private; - int ret; + FAR struct inode *inode = filep->f_inode; + FAR struct pipe_dev_s *dev = inode->i_private; + int ret; - /* Some sanity checking */ -#if CONFIG_DEBUG - if (!dev) - { - return -EBADF; - } -#endif + DEBUGASSERT(dev); /* Perform common close operations */ @@ -171,8 +168,8 @@ static int pipe_close(FAR struct file *filep) * Name: pipe * * Description: - * pipe() creates a pair of file descriptors, pointing to a pipe inode, and - * places them in the array pointed to by 'fd'. fd[0] is for reading, + * pipe() creates a pair of file descriptors, pointing to a pipe inode, + * and places them in the array pointed to by 'fd'. fd[0] is for reading, * fd[1] is for writing. * * Inputs: @@ -187,7 +184,7 @@ static int pipe_close(FAR struct file *filep) int pipe(int fd[2]) { - struct pipe_dev_s *dev = NULL; + FAR struct pipe_dev_s *dev = NULL; char devname[16]; int pipeno; int err; @@ -272,15 +269,19 @@ int pipe(int fd[2]) errout_with_wrfd: close(fd[1]); + errout_with_driver: unregister_driver(devname); + errout_with_dev: if (dev) { pipecommon_freedev(dev); } + errout_with_pipe: pipe_free(pipeno); + errout: set_errno(err); return ERROR; diff --git a/drivers/pipes/pipe_common.c b/drivers/pipes/pipe_common.c index fe56f8d8ad..1d584ce601 100644 --- a/drivers/pipes/pipe_common.c +++ b/drivers/pipes/pipe_common.c @@ -172,10 +172,10 @@ FAR struct pipe_dev_s *pipecommon_allocdev(void) void pipecommon_freedev(FAR struct pipe_dev_s *dev) { - sem_destroy(&dev->d_bfsem); - sem_destroy(&dev->d_rdsem); - sem_destroy(&dev->d_wrsem); - kmm_free(dev); + sem_destroy(&dev->d_bfsem); + sem_destroy(&dev->d_rdsem); + sem_destroy(&dev->d_wrsem); + kmm_free(dev); } /**************************************************************************** @@ -203,9 +203,9 @@ int pipecommon_open(FAR struct file *filep) return -get_errno(); } - /* 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 this the first reference on the device, then allocate the buffer. + * In the case of policy 1, the buffer already be present when the pipe + * is first opened. */ if (dev->d_refs == 0 && dev->d_buffer == NULL) @@ -303,7 +303,7 @@ 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. + * still outstanding references to the pipe. */ /* Check if the decremented reference count would go to zero */ @@ -331,12 +331,12 @@ int pipecommon_close(FAR struct file *filep) } /* 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. + * last client closes the pipe policy 0, or when the buffer becomes empty. + * 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) + else if (PIPE_IS_POLICY_0(dev->d_flags) || dev->d_wrndx == dev->d_rdndx) { /* Policy 0 or the buffer is empty ... deallocate the buffer now. */ @@ -349,6 +349,16 @@ int pipecommon_close(FAR struct file *filep) dev->d_rdndx = 0; dev->d_refs = 0; dev->d_nwriters = 0; + + /* If, in addition, we have been unlinked, then also need to free the + * device structure as well to prevent a memory leak. + */ + + if (PIPE_IS_UNLINKED(dev->d_flags)) + { + pipecommon_freedev(dev); + return OK; + } } sem_post(&dev->d_bfsem); @@ -678,10 +688,57 @@ int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg) if (cmd == PIPEIOC_POLICY) { - dev->d_policy = (arg != 0); + if (arg != 0) + { + PIPE_POLICY_1(dev->d_flags); + } + else + { + PIPE_POLICY_0(dev->d_flags); + } + return OK; } return -ENOTTY; } + +/**************************************************************************** + * Name: pipecommon_unlink + ****************************************************************************/ + +int pipecommon_unlink(FAR void *priv) +{ + /* The value passed to pipcommon_unlink is the private data pointer from + * the inode that is being unlinked. If there are no open references to + * the driver, then we must free up all resources used by the driver now. + */ + + FAR struct pipe_dev_s *dev = (FAR struct pipe_dev_s *)priv; + + DEBUGASSERT(dev); + + /* Mark the pipe unlinked */ + + PIPE_UNLINK(dev->d_flags); + + /* Are the any open references to the driver? */ + + if (dev->d_refs == 0) + { + /* No.. free the buffer (if there is one) */ + + if (dev->d_buffer) + { + kmm_free(dev->d_buffer); + } + + /* And free the device structure. */ + + pipecommon_freedev(dev); + } + + return OK; +} + #endif /* CONFIG_DEV_PIPE_SIZE > 0 */ diff --git a/drivers/pipes/pipe_common.h b/drivers/pipes/pipe_common.h index 77e359dc80..b0889faac6 100644 --- a/drivers/pipes/pipe_common.h +++ b/drivers/pipes/pipe_common.h @@ -1,7 +1,7 @@ /**************************************************************************** * drivers/pipe/pipe_common.h * - * Copyright (C) 2008-2009 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2009, 2015 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -67,6 +67,20 @@ #define CONFIG_DEV_PIPE_MAXUSER 255 +/* d_flags values */ + +#define PIPE_FLAG_POLICY (1 << 0) /* Bit 0: Policy=Free buffer when empty */ +#define PIPE_FLAG_UNLINKED (1 << 1) /* Bit 1: The driver has been unlinked */ + +#define PIPE_POLICY_0(f) do { (f) &= ~PIPE_FLAG_POLICY; } while (0) +#define PIPE_POLICY_1(f) do { (f) |= PIPE_FLAG_POLICY; } while (0) +#define PIPE_IS_POLICY_0(f) (((f) & PIPE_FLAG_POLICY) == 0) +#define PIPE_IS_POLICY_1(f) (((f) & PIPE_FLAG_POLICY) != 0) + +#define PIPE_UNLINK(f) do { (f) |= PIPE_FLAG_UNLINKED; } while (0) +#define PIPE_IS_UNLINKED(f) (((f) & PIPE_FLAG_UNLINKED) != 0) + + /**************************************************************************** * Public Types ****************************************************************************/ @@ -96,7 +110,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_flags; /* See PIPE_FLAG_* definitions */ uint8_t *d_buffer; /* Buffer allocated when device opened */ /* The following is a list if poll structures of threads waiting for @@ -127,11 +141,11 @@ 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 int pipecommon_poll(FAR struct file *filep, FAR struct pollfd *fds, bool setup); #endif +int pipecommon_unlink(FAR void *priv); #undef EXTERN #ifdef __cplusplus