From 5dd78cfbf81d2f313c06af10cc82984dc6af28b6 Mon Sep 17 00:00:00 2001 From: patacongo Date: Mon, 28 Mar 2011 17:43:34 +0000 Subject: Fix an error in opendir() when a mountpoint is in the root directory. git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3432 42af7a65-404d-4744-a932-0658087f49c3 --- apps/namedapp/binfs.c | 7 +- nuttx/ChangeLog | 4 + nuttx/fs/fs_opendir.c | 208 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 145 insertions(+), 74 deletions(-) diff --git a/apps/namedapp/binfs.c b/apps/namedapp/binfs.c index 9d2e4a7ee..a9aed59d6 100644 --- a/apps/namedapp/binfs.c +++ b/apps/namedapp/binfs.c @@ -310,7 +310,7 @@ static int binfs_opendir(struct inode *mountpt, const char *relpath, struct binfs_state_s *bm; int ret; - fvdbg("relpath: '%s'\n", relpath); + fvdbg("relpath: \"%s\"\n", relpath ? relpath : "NULL"); /* Sanity checks */ @@ -352,8 +352,6 @@ static int binfs_readdir(struct inode *mountpt, struct fs_dirent_s *dir) unsigned int index; int ret; - fvdbg("Entry\n"); - /* Sanity checks */ DEBUGASSERT(mountpt != NULL && mountpt->i_private != NULL); @@ -372,13 +370,14 @@ static int binfs_readdir(struct inode *mountpt, struct fs_dirent_s *dir) * special error -ENOENT */ - fdbg("End of directory\n"); + fvdbg("Entry %d: End of directory\n", index); ret = -ENOENT; } else { /* Save the filename and file type */ + fvdbg("Entry %d: \"%s\"\n", index, namedapps[index].name); dir->fd_dir.d_type = DTYPE_FILE; strncpy(dir->fd_dir.d_name, namedapps[index].name, NAME_MAX+1); diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 636b7e246..ec0c827ea 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -1623,4 +1623,8 @@ of the nuttx source tree * apps/namedapp/binfs.c: Create a tiny filesystem that can be used to show the internal named apps under /bin. + * fs/fs_opendir.c: Correct an error that occurs when a file system is + mounted in the root directory. This was discovered while mounting + the named app's /bin directory. + diff --git a/nuttx/fs/fs_opendir.c b/nuttx/fs/fs_opendir.c index 8f287c050..0b164734b 100644 --- a/nuttx/fs/fs_opendir.c +++ b/nuttx/fs/fs_opendir.c @@ -55,6 +55,108 @@ * Private Functions ****************************************************************************/ +/**************************************************************************** + * Name: open_mountpoint + * + * Description: + * Handle the case where the inode to be opened is within a mountpoint. + * + * Inputs: + * inode -- the inode of the mountpoint to open + * relpath -- the relative path within the mountpoint to open + * dir -- the dirent structure to be initialized + * + * Return: + * On success, OK is returned; Otherwise, a positive errno is returned. + * + ****************************************************************************/ + +#ifndef CONFIG_DISABLE_MOUNTPOINT +static inline int open_mountpoint(FAR struct inode *inode, + FAR const char *relpath, + FAR struct fs_dirent_s *dir) +{ + int ret; + + /* The inode itself as the 'root' of mounted volume. The actually + * directory is at relpath into the* mounted filesystem. + * + * + * Verify that the mountpoint inode supports the opendir() method + */ + + if (!inode->u.i_mops || !inode->u.i_mops->opendir) + { + return ENOSYS; + } + + /* Take reference to the mountpoint inode (fd_root). Note that we do + * not use inode_addref() because we already hold the tree semaphore. + */ + + inode->i_crefs++; + + /* Perform the opendir() operation */ + + ret = inode->u.i_mops->opendir(inode, relpath, dir); + if (ret < 0) + { + /* We now need to back off our reference to the inode. We can't + * call inode_release() to do that unless we release the tree + * semaphore. The following should be safe because: (1) after the + * reference count was incremented above it should be >=1 so it should + * not decrement below zero, and (2) we hold the tree semaphore so no + * other thread should be able to change the reference count. + */ + + inode->i_crefs--; + DEBUGASSERT(inode->i_crefs >= 0); + + /* Negate the error value so that it can be used to set errno */ + + return -ret; + } + + return OK; +} +#endif + +/**************************************************************************** + * Name: open_pseudodir + * + * Description: + * Handle the case where the inode to be opened is within the top-level + * pseudo-file system. + * + * Inputs: + * inode -- the inode of the mountpoint to open + * dir -- the dirent structure to be initialized + * + * Return: + * On success, OK is returned; Otherwise, a positive errno is returned. + * + ****************************************************************************/ + +#ifndef CONFIG_DISABLE_MOUNTPOINT +static void open_pseudodir(FAR struct inode *inode, FAR struct fs_dirent_s *dir) +{ + /* We have a valid psuedo-filesystem node. Take two references on the + * inode -- one for the parent (fd_root) and one for the child (fd_next). + * Note that we do not call inode_addref because we are holding the tree + * semaphore and that would result in deadlock. + */ + + inode->i_crefs += 2; + dir->u.psuedo.fd_next = inode; /* This is the next node to use for readdir() */ + + /* Flag the inode as belonging to the psuedo-filesystem */ + +#ifndef CONFIG_DISABLE_MOUNTPOINT + DIRENT_SETPSUEDONODE(dir->fd_flags); +#endif +} +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -63,18 +165,16 @@ * Name: opendir * * Description: - * The opendir() function opens a directory stream - * corresponding to the directory name, and returns a - * pointer to the directory stream. The stream is - * positioned at the first entry in the directory. + * The opendir() function opens a directory stream corresponding to the + * directory name, and returns a pointer to the directory stream. The + * stream is positioned at the first entry in the directory. * * Inputs: * path -- the directory to open * * Return: - * The opendir() function returns a pointer to the - * directory stream. On error, NULL is returned, and - * errno is set appropriately. + * The opendir() function returns a pointer to the directory stream. On + * error, NULL is returned, and errno is set appropriately. * * EACCES - Permission denied. * EMFILE - Too many file descriptors in use by process. @@ -92,7 +192,7 @@ FAR DIR *opendir(FAR const char *path) FAR struct inode *inode = NULL; FAR struct fs_dirent_s *dir; FAR const char *relpath; - bool isroot = false; + bool bisroot = false; int ret; /* If we are given 'nothing' then we will interpret this as @@ -102,8 +202,9 @@ FAR DIR *opendir(FAR const char *path) inode_semtake(); if (!path || *path == 0 || strcmp(path, "/") == 0) { - inode = root_inode; - isroot = true; + inode = root_inode; + bisroot = true; + relpath = NULL; } else { @@ -151,85 +252,52 @@ FAR DIR *opendir(FAR const char *path) dir->fd_root = inode; /* Save the inode where we start */ dir->fd_position = 0; /* This is the position in the read stream */ - /* Is this a node in the psuedo filesystem? Or a mountpoint? */ - -#ifndef CONFIG_DISABLE_MOUNTPOINT - if (INODE_IS_MOUNTPT(inode)) - { - /* Yes, then return the inode itself as the 'root' of - * the directory. The actually directory is at relpath into the - * mounted filesystem. - */ + /* First, handle the special case of the root inode. This must be + * special-cased here because the root inode might ALSO be a mountpoint. + */ - /* The node is a file system mointpoint. Verify that the mountpoint - * supports the opendir() method + if (bisroot) + { + /* Whatever payload the root inode carries, the root inode is always + * a directory inode in the pseudo-file system */ - if (!inode->u.i_mops || !inode->u.i_mops->opendir) - { - ret = ENOSYS; - goto errout_with_direntry; - } - - /* Take reference to the mountpoint inode (fd_root). Note that we do - * not use inode_addref() because we already hold the tree semaphore. - */ + open_pseudodir(inode, dir); + } - inode->i_crefs++; + /* Is this a node in the psuedo filesystem? Or a mountpoint? If the node + * is the root (bisroot == TRUE), then this is a special case. + */ - /* Perform the opendir() operation */ +#ifndef CONFIG_DISABLE_MOUNTPOINT + else if (INODE_IS_MOUNTPT(inode)) + { + /* Yes, the node is a file system mointpoint. */ - ret = inode->u.i_mops->opendir(inode, relpath, dir); - if (ret < 0) + ret = open_mountpoint(inode, relpath, dir); + if (ret != OK) { - /* We now need to back off our reference to the inode. We can't - * call inode_release() to do that unless we release the tree - * semaphore. The following should be safe because: (1) after the - * reference count was incremented above it should be >=1 so it should - * not decrement below zero, and (2) we hold the tree semaphore so no - * other thread should be able to change the reference count. - */ - - inode->i_crefs--; - DEBUGASSERT(inode->i_crefs >= 0); - - /* Negate the error value so that it can be used to set errno */ - - ret = -ret; - goto errout_with_direntry; + goto errout_with_direntry; } } - else #endif + else { /* The node is part of the root psuedo file system. Does the inode have a child? * If so that the child would be the 'root' of a list of nodes under * the directory. */ - if (!isroot) + inode = inode->i_child; + if (!inode) { - inode = inode->i_child; - if (!inode) - { - ret = ENOTDIR; - goto errout_with_direntry; - } + ret = ENOTDIR; + goto errout_with_direntry; } - /* It looks we have a valid psuedo-filesystem node. Take two references - * on the inode -- one for the parent (fd_root) and one for the child (fd_next). - * Note that we do not call inode_addref because we are holding - * the tree semaphore and that would result in deadlock. - */ - - inode->i_crefs += 2; - dir->u.psuedo.fd_next = inode; /* This is the next node to use for readdir() */ + /* It looks we have a valid psuedo-filesystem directory node. */ - /* Flag the inode as belonging to the psuedo-filesystem */ -#ifndef CONFIG_DISABLE_MOUNTPOINT - DIRENT_SETPSUEDONODE(dir->fd_flags); -#endif + open_pseudodir(inode, dir); } inode_semgive(); @@ -242,7 +310,7 @@ errout_with_direntry: errout_with_semaphore: inode_semgive(); - *get_errno_ptr() = ret; + errno = ret; return NULL; } -- cgit v1.2.3