From 47b47e0bb7383db128618253b3e8ef81d925f0af Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Sat, 15 Oct 2022 14:55:16 +0800 Subject: [PATCH] fs/vfs: Remove the redundancy file name comparison in mountptrename Signed-off-by: Xiang Xiao --- fs/vfs/fs_rename.c | 156 +++++++++++++++++++++------------------------ 1 file changed, 74 insertions(+), 82 deletions(-) diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c index f722258212..91c09230ed 100644 --- a/fs/vfs/fs_rename.c +++ b/fs/vfs/fs_rename.c @@ -310,7 +310,17 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode, goto errout_with_newinode; } - /* Does a directory entry already exist at the 'rewrelpath'? And is it + /* If oldrelpath and newrelpath are the same, then this is an attempt + * to move the directory entry onto itself. Let's not but say we did. + */ + + if (strcmp(oldrelpath, newrelpath) == 0) + { + ret = OK; + goto errout_with_newinode; /* Same name, this is not an error case. */ + } + + /* Does a directory entry already exist at the 'newrelpath'? And is it * not the same directory entry that we are moving? * * If the directory entry at the newrelpath is a regular file, then that @@ -318,117 +328,99 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode, * * If the directory entry at the target is a directory, then the source * file should be moved "under" the directory, i.e., if newrelpath is a - * directory, then rename(b,a) should use move the olrelpath should be + * directory, then rename(b,a) should use move the oldrelpath should be * moved as if rename(b,a/basename(b)) had been called. */ - if (oldinode->u.i_mops->stat != NULL && - strcmp(oldrelpath, newrelpath) != 0) + if (oldinode->u.i_mops->stat != NULL) { struct stat buf; next_subdir: - /* Something exists for this directory entry. Do nothing in the - * degenerate case where a directory or file is being moved to - * itself. - */ - - if (strcmp(oldrelpath, newrelpath) != 0) + ret = oldinode->u.i_mops->stat(oldinode, newrelpath, &buf); + if (ret >= 0) { - ret = oldinode->u.i_mops->stat(oldinode, newrelpath, &buf); - if (ret >= 0) + /* Is the directory entry a directory? */ + + if (S_ISDIR(buf.st_mode)) { - /* Is the directory entry a directory? */ + FAR char *subdirname; - if (S_ISDIR(buf.st_mode)) + /* Free memory may be allocated in previous loop */ + + if (subdir != NULL) { - FAR char *subdirname; + kmm_free(subdir); + subdir = NULL; + } - /* Free memory may be allocated in previous loop */ + /* Yes.. In this case, the target of the rename must be a + * subdirectory of newinode, not the newinode itself. For + * example: mv b a/ must move b to a/b. + */ - if (subdir != NULL) - { - kmm_free(subdir); - subdir = NULL; - } + subdirname = basename((FAR char *)oldrelpath); - /* Yes.. In this case, the target of the rename must be a - * subdirectory of newinode, not the newinode itself. For - * example: mv b a/ must move b to a/b. - */ + /* Special case the root directory */ - subdirname = basename((FAR char *)oldrelpath); - - /* Special case the root directory */ - - if (*newrelpath == '\0') - { - newrelpath = subdirname; - } - else - { - asprintf(&subdir, "%s/%s", newrelpath, - subdirname); - if (subdir == NULL) - { - ret = -ENOMEM; - goto errout_with_newinode; - } - - newrelpath = subdir; - } - - /* This can be a recursive, another directory may already - * exist at the newrelpath. In that case, we need to - * do this all over again. A nasty goto is used because - * I am lazy. - */ - - goto next_subdir; + if (*newrelpath == '\0') + { + newrelpath = subdirname; } else { - /* No.. newrelpath must refer to a regular file. Make sure - * that the file at the olrelpath actually exists before - * performing any further actions with newrelpath - */ - - ret = oldinode->u.i_mops->stat(oldinode, oldrelpath, &buf); - if (ret < 0) + asprintf(&subdir, "%s/%s", newrelpath, + subdirname); + if (subdir == NULL) { + ret = -ENOMEM; goto errout_with_newinode; } - if (oldinode->u.i_mops->unlink) - { - /* Attempt to remove the file before doing the rename. - * - * NOTE that errors are not handled here. If we failed - * to remove the file, then the file system 'rename' - * method should check that. - */ + newrelpath = subdir; + } - oldinode->u.i_mops->unlink(oldinode, newrelpath); - } + /* This can be a recursive, another directory may already + * exist at the newrelpath. In that case, we need to + * do this all over again. A nasty goto is used because + * I am lazy. + */ + + goto next_subdir; + } + else + { + /* No.. newrelpath must refer to a regular file. Make sure + * that the file at the oldrelpath actually exists before + * performing any further actions with newrelpath + */ + + ret = oldinode->u.i_mops->stat(oldinode, oldrelpath, &buf); + if (ret < 0) + { + goto errout_with_newinode; + } + + if (oldinode->u.i_mops->unlink) + { + /* Attempt to remove the file before doing the rename. + * + * NOTE that errors are not handled here. If we failed + * to remove the file, then the file system 'rename' + * method should check that. + */ + + oldinode->u.i_mops->unlink(oldinode, newrelpath); } } } } - /* Just declare success of the oldrepath and the newrelpath point to - * the same directory entry. That directory entry should have been - * stat'ed above to assure that it exists. + /* Perform the rename operation using the relative paths at the common + * mountpoint. */ - ret = OK; - if (strcmp(oldrelpath, newrelpath) != 0) - { - /* Perform the rename operation using the relative paths at the common - * mountpoint. - */ - - ret = oldinode->u.i_mops->rename(oldinode, oldrelpath, newrelpath); - } + ret = oldinode->u.i_mops->rename(oldinode, oldrelpath, newrelpath); errout_with_newinode: inode_release(newinode);