fs: Simplify the symbol link process in inode_search

avoid the recursive call by moving the path catenation into _inode_search

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>

fs/inode/fs_inodesearch.c

@@ -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
+ *   If the inode is a soft link, then (1) recursively look-up the inode
- *   the soft link, (2) recursively look-up the inode referenced by the soft
+ *   referenced by the soft link, and (2) return the inode referenced by
- *   link, and (3) return the inode referenced by the soft link.
+ *   the soft link.
  *
  * Assumptions:
  *   The caller holds the g_inode_sem semaphore
@@ -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.
+  /* 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.
-   */
 
   save = desc->nofollow;
   while (INODE_IS_SOFTLINK(node))
@@ -206,7 +202,6 @@ static int _inode_linktarget(FAR struct inode *node,
 
       node = desc->node;
       DEBUGASSERT(node != NULL);
-      desc->linktgt = link;
     }
 
   desc->nofollow = save;
@@ -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)
+                   * (1) recursively look-up the inode referenced by the
-                   * recursively look-up the inode referenced by the soft
+                   * soft link, and (2) continue searching with that inode
-                   * link, and (3) continue searching with that inode
                    * instead.
                    */
 
@@ -365,11 +359,32 @@ static int _inode_search(FAR struct inode_search_s *desc)
                                */
 
                               node    = newnode;
-                              above   = NULL;
+                              above   = desc->parent;
-                              left    = NULL;
+                              left    = desc->peer;
-                              relpath = name;
-
                               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;
                             }
 
@@ -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);
 
@@ -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
@@ -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

fs/inode/inode.h

@@ -50,7 +50,6 @@
         (d)->peer     = NULL; \
         (d)->parent   = NULL; \
         (d)->relpath  = NULL; \
-        (d)->linktgt  = NULL; \
         (d)->buffer   = NULL; \
         (d)->nofollow = (n); \
       } \
@@ -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
@@ -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

fs/vfs/fs_symlink.c

@@ -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
+  /* Check that no inode exists at the 'path2' and that the path up to
-   * 'path1' does not lie on a mounted volume.
+   * 'path2' does not lie on a mounted volume.
    */
 
   SETUP_SEARCH(&desc, path2, false);