From 60ba5a52610407154c0bb0f64bb95e026afa41a0 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 3 Feb 2017 13:22:33 -0600 Subject: [PATCH] Soft links: Fix logic and remove kludge of last commit. Rename inode_dereference() as inode_linktarget() and make global. --- fs/inode/fs_inodesearch.c | 99 ++++++++++++++++++++------------------- fs/inode/inode.h | 20 ++++++++ fs/vfs/fs_stat.c | 90 +++++++++++++++++++++++------------ include/sys/stat.h | 83 ++++++++++++++++++-------------- 4 files changed, 178 insertions(+), 114 deletions(-) diff --git a/fs/inode/fs_inodesearch.c b/fs/inode/fs_inodesearch.c index 5856e2f5ae..b3cd7f4c61 100644 --- a/fs/inode/fs_inodesearch.c +++ b/fs/inode/fs_inodesearch.c @@ -134,52 +134,6 @@ static int _inode_compare(FAR const char *fname, } } -/**************************************************************************** - * Name: _inode_dereference - * - * Description: - * If the inode is a soft link, then (1) get the name of the full path of - * the soft link, (2) recursively look-up the inode referenced by the soft - * link, and (3) return the inode referenced by the soft link. - * - * Assumptions: - * The caller holds the g_inode_sem semaphore - * - ****************************************************************************/ - -#ifdef CONFIG_PSEUDOFS_SOFTLINKS -static inline FAR struct inode * -_inode_dereference(FAR struct inode *node, FAR struct inode **peer, - FAR struct inode **parent, FAR const char **relpath) -{ - FAR const char *copy; - unsigned int count = 0; - - /* An infinite loop is avoided only by the loop count. - * - * REVISIT: The ELOOP error should be reported to the application in that - * case but there is no simple mechanism to do that. - */ - - while (node != NULL && INODE_IS_SOFTLINK(node)) - { - /* Careful: inode_search_nofollow overwrites the input string pointer */ - - copy = (FAR const char *)node->u.i_link; - - /* Now, look-up the inode associated with the target path */ - - node = inode_search_nofollow(©, peer, parent, relpath); - if (node == NULL && ++count > SYMLOOP_MAX) - { - return NULL; - } - } - - return node; -} -#endif - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -266,7 +220,7 @@ FAR struct inode *inode_search(FAR const char **path, FAR const char *nextname = inode_nextname(name); if (*nextname != '\0') { - newnode = _inode_dereference(node, NULL, &above, relpath); + newnode = inode_linktarget(node, NULL, &above, relpath); if (newnode == NULL) { /* Probably means that the node is a symbolic link, but @@ -340,7 +294,7 @@ FAR struct inode *inode_search(FAR const char **path, * continue searching with that inode instead. */ - newnode = _inode_dereference(node, NULL, NULL, relpath); + newnode = inode_linktarget(node, NULL, NULL, relpath); if (newnode == NULL) { /* Probably means that the node is a symbolic link, but @@ -438,7 +392,54 @@ FAR struct inode *inode_search(FAR const char **path, * return that inode instead. */ - return _inode_dereference(node, peer, parent, relpath); + return inode_linktarget(node, peer, parent, relpath); + } + + return node; +} +#endif + +/**************************************************************************** + * Name: inode_linktarget + * + * Description: + * If the inode is a soft link, then (1) get the name of the full path of + * the soft link, (2) recursively look-up the inode referenced by the soft + * link, and (3) return the inode referenced by the soft link. + * + * Assumptions: + * The caller holds the g_inode_sem semaphore + * + ****************************************************************************/ + +#ifdef CONFIG_PSEUDOFS_SOFTLINKS +FAR struct inode *inode_linktarget(FAR struct inode *node, + FAR struct inode **peer, + FAR struct inode **parent, + FAR const char **relpath) +{ + FAR const char *copy; + unsigned int count = 0; + + /* An infinite loop is avoided only by the loop count. + * + * REVISIT: The ELOOP error should be reported to the application in that + * case but there is no simple mechanism to do that. + */ + + while (node != NULL && INODE_IS_SOFTLINK(node)) + { + /* Careful: inode_search_nofollow overwrites the input string pointer */ + + copy = (FAR const char *)node->u.i_link; + + /* Now, look-up the inode associated with the target path */ + + node = inode_search_nofollow(©, peer, parent, relpath); + if (node == NULL && ++count > SYMLOOP_MAX) + { + return NULL; + } } return node; diff --git a/fs/inode/inode.h b/fs/inode/inode.h index 529642bfa7..040037b916 100644 --- a/fs/inode/inode.h +++ b/fs/inode/inode.h @@ -141,6 +141,26 @@ FAR struct inode *inode_search_nofollow(FAR const char **path, # define inode_search_nofollow(p,l,a,r) inode_search(p,l,a,r) #endif +/**************************************************************************** + * Name: inode_linktarget + * + * Description: + * If the inode is a soft link, then (1) get the name of the full path of + * the soft link, (2) recursively look-up the inode referenced by the soft + * link, and (3) return the inode referenced by the soft link. + * + * Assumptions: + * The caller holds the g_inode_sem semaphore + * + ****************************************************************************/ + +#ifdef CONFIG_PSEUDOFS_SOFTLINKS +FAR struct inode *inode_linktarget(FAR struct inode *node, + FAR struct inode **peer, + FAR struct inode **parent, + FAR const char **relpath); +#endif + /**************************************************************************** * Name: inode_free * diff --git a/fs/vfs/fs_stat.c b/fs/vfs/fs_stat.c index 5b818e2078..c1f2122df6 100644 --- a/fs/vfs/fs_stat.c +++ b/fs/vfs/fs_stat.c @@ -79,48 +79,84 @@ static inline int statpseudo(FAR struct inode *inode, FAR struct stat *buf) #if defined(CONFIG_FS_SHM) if (INODE_IS_SHM(inode)) */ { - buf->st_mode | S_IFSHM; + buf->st_mode = S_IFSHM; } else #endif { } } - else if (inode->u.i_ops) + else if (inode->u.i_ops != NULL) { - if (inode->u.i_ops->read) - { - buf->st_mode = S_IROTH | S_IRGRP | S_IRUSR; - } +#ifdef CONFIG_PSEUDOFS_SOFTLINKS + /* Handle softlinks differently. Just call stat() recursively on the + * target of the softlink. + * + * REVISIT: This has the possibility of an infinite loop! + */ - if (inode->u.i_ops->write) + if (INODE_IS_SOFTLINK(inode)) { - buf->st_mode |= S_IWOTH | S_IWGRP | S_IWUSR; - } + int ret; - if (INODE_IS_MOUNTPT(inode)) - { - buf->st_mode |= S_IFDIR; - } - else if (INODE_IS_BLOCK(inode)) - { - /* What is if also has child inodes? */ + /* stat() the target of the soft link. + * + * Note: We don't really handle links to links nicely. I suppose + * that we should repetitively deference the symbolic link inodes + * until we discover the true target link? + */ - buf->st_mode |= S_IFBLK; - } -#if 0 /* ifdef CONFIG_PSEUDOFS_SOFTLINKS See REVISIT in stat() */ - else if (INODE_IS_SOFTLINK(inode)) - { - /* Should not have child inodes. */ + ret = stat((FAR const char *)inode->u.i_link, buf); + + /* If stat() fails, then there is a probleml with target of the + * symbolic link, but not with the symbolic link itself. We should + * still report success, just with less information. + */ + + if (ret < 0) + { + memset(buf, 0, sizeof(struct stat)); + } + + /* And make sure that the caller knows that this really a symbolic link. */ buf->st_mode |= S_IFLNK; } + else #endif - else /* if (INODE_IS_DRIVER(inode)) */ { - /* What is it if it also has child inodes? */ + /* Determine read/write privileges based on the existence of read + * and write methods. + */ - buf->st_mode |= S_IFCHR; + if (inode->u.i_ops->read) + { + buf->st_mode = S_IROTH | S_IRGRP | S_IRUSR; + } + + if (inode->u.i_ops->write) + { + buf->st_mode |= S_IWOTH | S_IWGRP | S_IWUSR; + } + + /* Determine the type of the inode */ + + if (INODE_IS_MOUNTPT(inode)) + { + buf->st_mode |= S_IFDIR; + } + else if (INODE_IS_BLOCK(inode)) + { + /* What is if also has child inodes? */ + + buf->st_mode |= S_IFBLK; + } + else /* if (INODE_IS_DRIVER(inode)) */ + { + /* What is it if it also has child inodes? */ + + buf->st_mode |= S_IFCHR; + } } } else @@ -199,11 +235,7 @@ int stat(FAR const char *path, FAR struct stat *buf) /* Get an inode for this file */ -#if 0 /* REVISIT: inode_find_nofollow needed for symlinks but conflicts with other usage */ inode = inode_find_nofollow(path, &relpath); -#else - inode = inode_find(path, &relpath); -#endif if (!inode) { /* This name does not refer to a psudeo-inode and there is no diff --git a/include/sys/stat.h b/include/sys/stat.h index 3f2658b5f1..d5bc26e787 100644 --- a/include/sys/stat.h +++ b/include/sys/stat.h @@ -50,51 +50,62 @@ /* mode_t bit settings (most of these do not apply to Nuttx). This assumes * that the full size of a mode_t is 16-bits. (However, mode_t must be size * 'int' because it is promoted to size int when passed in varargs). + * + * LTTT ...U UUGG GOOO + * + * Bits 0-2: Permissions for others + * Bits 3-5: Group permissions + * Bits 6-8: Owner permissions + * Bits 9-11: Not used + * Bits 12-14: File type bits + * Bit 15: Symbolic link */ -#define S_IXOTH 0000001 /* Permissions for others: RWX */ -#define S_IWOTH 0000002 -#define S_IROTH 0000004 -#define S_IRWXO 0000007 +#define S_IXOTH (1 << 0) /* Bits 0-2: Permissions for others: RWX */ +#define S_IWOTH (1 << 1) +#define S_IROTH (1 << 2) +#define S_IRWXO (7 << 0) -#define S_IXGRP 0000010 /* Group permissions: RWX */ -#define S_IWGRP 0000020 -#define S_IRGRP 0000040 -#define S_IRWXG 0000070 +#define S_IXGRP (1 << 3) /* Bits 3-5: Group permissions: RWX */ +#define S_IWGRP (1 << 4) +#define S_IRGRP (1 << 5) +#define S_IRWXG (7 << 3) -#define S_IXUSR 0000100 /* Owner permissions: RWX */ -#define S_IWUSR 0000200 -#define S_IRUSR 0000400 -#define S_IRWXU 0000700 +#define S_IXUSR (1 << 6) /* Bits 6-8: Owner permissions: RWX */ +#define S_IWUSR (1 << 7) +#define S_IRUSR (1 << 8) +#define S_IRWXU (7 << 6) -#define S_ISVTX 0001000 /* "sticky" bit */ -#define S_ISGID 0002000 /* Set group ID bit */ -#define S_ISUID 0004000 /* Set UID bit */ +#define S_ISVTX 0 /* "Sticky" bit (not used) */ +#define S_ISGID 0 /* Set group ID bit (not used)*/ +#define S_ISUID 0 /* Set UID bit (not used) */ -#define S_IFIFO 0010000 /* File type bytes */ -#define S_IFCHR 0020000 -#define S_IFDIR 0040000 -#define S_IFBLK 0060000 -#define S_IFREG 0100000 -#define S_IFLNK 0120000 -#define S_IFSOCK 0140000 -#define S_IFMQ 0150000 -#define S_IFSEM 0160000 -#define S_IFSHM 0170000 -#define S_IFMT 0170000 +#define S_IFIFO 0 /* Bits 12-14: File type bits (not all used) */ +#define S_IFCHR (1 << 12) +#define S_IFDIR (2 << 12) +#define S_IFBLK (3 << 12) +#define S_IFREG (4 << 12) +#define S_IFSOCK 0 +#define S_IFMQ (5 << 12) +#define S_IFSEM (6 << 12) +#define S_IFSHM (7 << 12) +#define s_IFTGT (7 << 12) /* May be the target of a symbolic link */ + +#define S_IFLNK (1 << 15) /* Bit 15: Symbolic link */ +#define S_IFMT (15 << 15) /* Bits 12-15: Full file type */ /* File type macros that operate on an instance of mode_t */ -#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK) -#define S_ISREG(m) (((m) & S_IFMT) == S_IFREG) -#define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR) -#define S_ISCHR(m) (((m) & S_IFMT) == S_IFCHR) -#define S_ISBLK(m) (((m) & S_IFMT) == S_IFBLK) -#define S_ISFIFO(m) (((m) & S_IFMT) == S_IFIFO) -#define S_ISSOCK(m) (((m) & S_IFMT) == S_IFSOCK) -#define S_ISMQ(m) (((m) & S_IFMT) == S_IFMQ) -#define S_ISSSEM(m) (((m) & S_IFMT) == S_IFSEM) -#define S_ISSHM(m) (((m) & S_IFMT) == S_IFSHM) +#define S_ISLNK(m) (((m) & S_IFLNK) != 0) +#define S_ISFIFO(m) (0) +#define S_ISCHR(m) (((m) & s_IFTGT) == S_IFCHR) +#define S_ISDIR(m) (((m) & s_IFTGT) == S_IFDIR) +#define S_ISBLK(m) (((m) & s_IFTGT) == S_IFBLK) +#define S_ISREG(m) (((m) & s_IFTGT) == S_IFREG) +#define S_ISSOCK(m) (0) +#define S_ISMQ(m) (((m) & s_IFTGT) == S_IFMQ) +#define S_ISSEM(m) (((m) & s_IFTGT) == S_IFSEM) +#define S_ISSHM(m) (((m) & s_IFTGT) == S_IFSHM) /**************************************************************************** * Type Definitions