Skip to content

Commit

Permalink
fs: Simplify the symbol link process in inode_search
Browse files Browse the repository at this point in the history 
avoid the recursive call by moving the path catenation into _inode_search

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
  • Loading branch information
@xiaoxiang781216 @liuguo09
xiaoxiang781216 authored and liuguo09 committed Sep 22, 2020
1 parent f4794f0 commit 43f0070
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 95 deletions.
121 changes: 31 additions & 90 deletions fs/inode/fs_inodesearch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ static int _inode_compare(FAR const char *fname, FAR struct inode *node)
* 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.
* If the inode is a soft link, then (1) recursively look-up the inode
* referenced by the soft link, and (2) return the inode referenced by
* the soft link.
*
* Assumptions:
* The caller holds the g_inode_sem semaphore
Expand All @@ -170,11 +170,7 @@ static int _inode_linktarget(FAR struct inode *node,

DEBUGASSERT(desc != NULL && node != NULL);

/* 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.
*/
/* An infinite loop is avoided only by the loop count. */

save = desc->nofollow;
while (INODE_IS_SOFTLINK(node))
Expand Down Expand Up @@ -206,7 +202,6 @@ static int _inode_linktarget(FAR struct inode *node,

node = desc->node;
DEBUGASSERT(node != NULL);
desc->linktgt = link;
}

desc->nofollow = save;
Expand Down Expand Up @@ -328,9 +323,8 @@ static int _inode_search(FAR struct inode_search_s *desc)
int status;

/* If this intermediate inode in the 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) continue searching with that inode
* (1) recursively look-up the inode referenced by the
* soft link, and (2) continue searching with that inode
* instead.
*/

Expand Down Expand Up @@ -365,11 +359,32 @@ static int _inode_search(FAR struct inode_search_s *desc)
*/

node = newnode;
above = NULL;
left = NULL;
relpath = name;

above = desc->parent;
left = desc->peer;
ret = OK;

if (*desc->relpath != '\0')
{
char *buffer = NULL;

asprintf(&buffer,
"%s/%s", desc->relpath, name);
if (buffer != NULL)
{
kmm_free(desc->buffer);
desc->buffer = buffer;
relpath = buffer;
}
else
{
ret = -ENOMEM;
}
}
else
{
relpath = name;
}

break;
}

Expand Down Expand Up @@ -453,9 +468,6 @@ int inode_search(FAR struct inode_search_s *desc)
*/

DEBUGASSERT(desc != NULL);
#ifdef CONFIG_PSEUDOFS_SOFTLINKS
desc->linktgt = NULL;
#endif

ret = _inode_search(desc);

Expand All @@ -473,12 +485,6 @@ int inode_search(FAR struct inode_search_s *desc)

if (!desc->nofollow && INODE_IS_SOFTLINK(node))
{
/* Save some things we need that will be clobbered by the call to
* _inode_linktgt().
*/

FAR const char *relpath = desc->relpath; /* Will always be "" here */

/* The terminating inode is a valid soft link: Return the inode,
* corresponding to link target. _inode_linktarget() will follow
* a link (or a series of links to links) and will return the
Expand All @@ -494,71 +500,6 @@ int inode_search(FAR struct inode_search_s *desc)

return ret;
}

/* The dereferenced node might be a mountpoint */

node = desc->node;
DEBUGASSERT(node != NULL && desc->linktgt != NULL);

if (INODE_IS_MOUNTPT(node))
{
/* Yes... set up for the MOUNTPOINT logic below. */

desc->relpath = relpath;
}
}

/* Handle a special case. This special occurs with either (1)
* inode_search() terminates early because it encountered a MOUNTPOINT
* at an intermediate node in the path, or (2) inode_search()
* terminates because it reached the terminal node and 'nofollow' is
* false and the above logic converted the symbolic link to a
* MOUNTPOINT.
*
* We can detect the special cases because desc->linktgt will be
* non-NULL.
*/

if (desc->linktgt != NULL && INODE_IS_MOUNTPT(node))
{
FAR char *buffer;

/* There would be no problem in this case if the link was to
* either to the root directory of the MOUNTPOINT or to a
* regular file within the mounted volume. However, there
* is a problem if the symbolic link is to a directory within
* the mounted volume. In that case, the 'relpath' will be
* relative to the symbolic link and not to the MOUNTPOINT.
*
* We will handle the worst case by creating the full path
* excluding the symbolic link and performing the look-up
* again.
*/

if (desc->relpath != NULL && *desc->relpath != '\0')
{
asprintf(&buffer, "%s/%s",
desc->linktgt, desc->relpath);
}
else
{
buffer = strdup(desc->linktgt);
}

if (buffer == NULL)
{
ret = -ENOMEM;
}
else
{
/* Reset the search description and perform the search again. */

RELEASE_SEARCH(desc);
SETUP_SEARCH(desc, buffer, false);
desc->buffer = buffer;

ret = _inode_search(desc);
}
}
}
#endif
Expand Down
3 changes: 0 additions & 3 deletions fs/inode/inode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
(d)->peer = NULL; \
(d)->parent = NULL; \
(d)->relpath = NULL; \
(d)->linktgt = NULL; \
(d)->buffer = NULL; \
(d)->nofollow = (n); \
} \
Expand Down Expand Up @@ -97,7 +96,6 @@
* relpath - INPUT: (not used)
* OUTPUT: If the returned inode is a mountpoint, this is the
* relative path from the mountpoint.
* linktgt - INPUT: (not used)
* OUTPUT: If a symobolic link into a mounted file system is
* detected while traversing the path, then the link
* will be converted to a mountpoint inode if the
Expand All @@ -122,7 +120,6 @@ struct inode_search_s
FAR struct inode *parent; /* Node "above" the found inode */
FAR const char *relpath; /* Relative path into the mountpoint */
#ifdef CONFIG_PSEUDOFS_SOFTLINKS
FAR const char *linktgt; /* Target of symbolic link if linked to a directory */
FAR char *buffer; /* Path expansion buffer */
bool nofollow; /* true: Don't follow terminal soft link */
#endif
Expand Down
4 changes: 2 additions & 2 deletions fs/vfs/fs_symlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ int symlink(FAR const char *path1, FAR const char *path2)
goto errout;
}

/* Check that no inode exists at the 'path1' and that the path up to
* 'path1' does not lie on a mounted volume.
/* Check that no inode exists at the 'path2' and that the path up to
* 'path2' does not lie on a mounted volume.
*/

SETUP_SEARCH(&desc, path2, false);
Expand Down

0 comments on commit 43f0070

Please sign in to comment.