diff --git a/ChangeLog b/ChangeLog index 39d4fa60390acf387e5a47a398f2646a76ba7dfe..be4610ec0628c77be439a80f27d0d1749a327731 100644 --- a/ChangeLog +++ b/ChangeLog @@ -383,6 +383,7 @@ * Added a test for redirection of stdio through pipes * Fixed error in dup and dup2: Must call open/close methods in fs/driver so that driver can correctly maintain open reference counts. + * Same issue on closing file descriptors in exit() * Fixed in error in stdio flush logic. Needed ssize_t vs size_t for error check. diff --git a/Documentation/NuttX.html b/Documentation/NuttX.html index e7a60182c5ed465b8c644774c97a2570ac6c238c..e75d27e3026cf66678204194d3a5f1fc9e3e4688 100644 --- a/Documentation/NuttX.html +++ b/Documentation/NuttX.html @@ -1032,6 +1032,7 @@ nuttx-0.3.12 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> * Added a test for redirection of stdio through pipes * Fixed error in dup and dup2: Must call open/close methods in fs/driver so that driver can correctly maintain open reference counts. + * Same issue on closing file descriptors in exit() * Fixed in error in stdio flush logic. Needed ssize_t vs size_t for error check. diff --git a/TODO b/TODO index bc91875500e5f73baf88466bd1cd9e6bfaf738e3..66399c13f73593d486df3bb17f33fbb7d7fecf75 100644 --- a/TODO +++ b/TODO @@ -242,12 +242,6 @@ o File system / Generic drivers (fs/, drivers/) when there are not open references to the pipe/FIFO. Priority: Medium - Desccripton: dup and dup2 need to call into driver. Most drivers - will maintain internal open counts. dup and dup2 logically - increase the open count but do not interact with the driver - Status: Open - Priority: Medium - o Pascal Add-On (pcode/) ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/fs/fs_close.c b/fs/fs_close.c index 736788a2e21b110f386f418720db0f6c6065f61c..6a152540d6a2053b01b5fee605abce3303b71768 100644 --- a/fs/fs_close.c +++ b/fs/fs_close.c @@ -82,7 +82,7 @@ int close(int fd) int err; #if CONFIG_NFILE_DESCRIPTORS > 0 FAR struct filelist *list; - FAR struct inode *inode; + int ret; /* Did we get a valid file descriptor? */ @@ -116,8 +116,7 @@ int close(int fd) /* If the file was properly opened, there should be an inode assigned */ - inode = list->fl_files[fd].f_inode; - if (!inode) + if (!list->fl_files[fd].f_inode) { err = EBADF; goto errout; @@ -133,30 +132,18 @@ int close(int fd) * vtable. */ - if (inode->u.i_ops && inode->u.i_ops->close) + ret = files_close(&list->fl_files[fd]); + if (ret < 0) { - /* Perform the close operation (by the driver) */ + /* An error occurred while closing the driver */ - int ret = inode->u.i_ops->close(&list->fl_files[fd]); - if (ret < 0) - { - /* An error occurred while closing the driver */ - - err = -ret; - goto errout; - } + err = -ret; + goto errout; } /* Release the file descriptor */ files_release(fd); - - /* Decrement the reference count on the inode. This may remove the inode and - * eliminate the name from the namespace - */ - - inode_release(inode); - return OK; #endif diff --git a/fs/fs_files.c b/fs/fs_files.c index 6f4941dcea92333d5b22ccdab63828c89db28107..30b3caca9dd72285739509a5dc9ba2fbb14d36a7 100644 --- a/fs/fs_files.c +++ b/fs/fs_files.c @@ -67,6 +67,9 @@ * Private Functions ****************************************************************************/ +/**************************************************************************** + * Name: _files_semtake + ****************************************************************************/ static void _files_semtake(FAR struct filelist *list) { /* Take the semaphore (perhaps waiting) */ @@ -81,22 +84,32 @@ static void _files_semtake(FAR struct filelist *list) } } +/**************************************************************************** + * Name: _files_semgive + ****************************************************************************/ #define _files_semgive(list) sem_post(&list->fl_sem) /**************************************************************************** - * Pulblic Functions + * Public Functions ****************************************************************************/ -/* This is called from the FS initialization logic to configure - * the files. - */ - +/**************************************************************************** + * Name: files_initialize + * + * Description: + * This is called from the FS initialization logic to configure the files. + * + ****************************************************************************/ void files_initialize(void) { } -/* Allocate a list of files for a new task */ - +/**************************************************************************** + * Name: files_alloclist + * + * Description: Allocate a list of files for a new task + * + ****************************************************************************/ FAR struct filelist *files_alloclist(void) { FAR struct filelist *list; @@ -114,8 +127,12 @@ FAR struct filelist *files_alloclist(void) return list; } -/* Increase the reference count on a file list */ - +/**************************************************************************** + * Name: files_addreflist + * + * Description: Increase the reference count on a file list + * + ****************************************************************************/ int files_addreflist(FAR struct filelist *list) { if (list) @@ -135,8 +152,44 @@ int files_addreflist(FAR struct filelist *list) return OK; } -/* Release a reference to the file list */ +/**************************************************************************** + * Name: files_close + * + * Description: Close an inode (if open) + * + ****************************************************************************/ +int files_close(FAR struct file *filep) +{ + struct inode *inode = filep->f_inode; + int ret = OK; + /* Check if the struct file is open (i.e., assigned an inode) */ + + if (inode) + { + /* Close the file, driver, or mountpoint. */ + + if (inode->u.i_ops && inode->u.i_ops->close) + { + /* Perform the close operation */ + + ret = inode->u.i_ops->close(filep); + } + + /* And release the inode */ + + inode_release(inode); + filep->f_inode = NULL; + } + return ret; +} + +/**************************************************************************** + * Name: files_releaselist + * + * Description: Release a reference to the file list + * + ****************************************************************************/ int files_releaselist(FAR struct filelist *list) { int crefs; @@ -168,26 +221,25 @@ int files_releaselist(FAR struct filelist *list) for (i = 0; i < CONFIG_NFILE_DESCRIPTORS; i++) { - FAR struct inode *inode = list->fl_files[i].f_inode; - if (inode) - { - inode_release(inode); - } + (void)files_close(&list->fl_files[i]); } - /* Destroy the semaphore and release the filelist */ + /* Destroy the semaphore and release the filelist */ - (void)sem_destroy(&list->fl_sem); - sched_free(list); + (void)sem_destroy(&list->fl_sem); + sched_free(list); } } return OK; } -/* Assign an inode to a specific files structure. this is the - * heart of dup2. - */ - +/**************************************************************************** + * Name: files_dup + * + * Description: + * Assign an inode to a specific files structure. This is the heart of dup2. + * + ****************************************************************************/ int files_dup(FAR struct file *filep1, FAR struct file *filep2) { FAR struct filelist *list; @@ -222,27 +274,12 @@ int files_dup(FAR struct file *filep1, FAR struct file *filep2) * close the file and release the inode. */ - inode = filep2->f_inode; - if (inode) + ret = files_close(filep2); + if (ret < 0) { - /* Close the file, driver, or mountpoint. */ + /* An error occurred while closing the driver */ - if (inode->u.i_ops && inode->u.i_ops->close) - { - /* Perform the close operation */ - - ret = inode->u.i_ops->close(filep2); - if (ret < 0) - { - /* An error occurred while closing the driver */ - - goto errout_with_ret; - } - } - - /* Release the inode */ - - inode_release(filep2->f_inode); + goto errout_with_ret; } /* Increment the reference count on the contained inode */ @@ -304,11 +341,14 @@ errout: return ERROR; } -/* Allocate a struct files instance and associate it with an - * inode instance. Returns the file descriptor == index into - * the files array. - */ - +/**************************************************************************** + * Name: files_allocate + * + * Description: + * Allocate a struct files instance and associate it with an inode instance. + * Returns the file descriptor == index into the files array. + * + ****************************************************************************/ int files_allocate(FAR struct inode *inode, int oflags, off_t pos) { FAR struct filelist *list; @@ -334,6 +374,9 @@ int files_allocate(FAR struct inode *inode, int oflags, off_t pos) return ERROR; } +/**************************************************************************** + * Name: files_release + ****************************************************************************/ void files_release(int filedes) { FAR struct filelist *list; diff --git a/fs/fs_internal.h b/fs/fs_internal.h index b20843d2f8adad91950b0bc974189fda08ecff2a..936761d4a8ebf9eb6117558f62f652c4dbb68da3 100644 --- a/fs/fs_internal.h +++ b/fs/fs_internal.h @@ -210,6 +210,7 @@ EXTERN void inode_release(FAR struct inode *inode); EXTERN void weak_function files_initialize(void); EXTERN int files_allocate(FAR struct inode *inode, int oflags, off_t pos); EXTERN void files_release(int filedes); +EXTERN int files_close(struct file *filep); #undef EXTERN #if defined(__cplusplus)