From bd7d3a92f5b7836ebe6d7298f0ef0090f5afa484 Mon Sep 17 00:00:00 2001
From: Gregory Nutt <gnutt@nuttx.org>
Date: Sat, 11 Feb 2017 11:18:30 -0600
Subject: [PATCH] Add logic to VFS rename: If target of rename exists and is a
directory, then the source file should be moved 'under' the target directory.
POSIX also requires that if the target is a file, then that old file must be
deleted.
---
fs/fat/fs_fat32.c | 5 +-
fs/smartfs/smartfs_smart.c | 90 +++-------------------------
fs/vfs/fs_rename.c | 120 +++++++++++++++++++++++++++++++------
3 files changed, 116 insertions(+), 99 deletions(-)
diff --git a/fs/fat/fs_fat32.c b/fs/fat/fs_fat32.c
index 774db3c276..4ddf1e5f8e 100644
--- a/fs/fat/fs_fat32.c
+++ b/fs/fat/fs_fat32.c
@@ -2419,7 +2419,10 @@ int fat_rename(FAR struct inode *mountpt, FAR const char *oldrelpath,
{
if (ret == OK)
{
- /* It is an error if the object at newrelpath already exists */
+ /* It is an error if the directory entry at newrelpath already
+ * exists. The necessary steps to avoid this case should have
+ * been handled by higher level logic in the VFS.
+ */
ret = -EEXIST;
}
diff --git a/fs/smartfs/smartfs_smart.c b/fs/smartfs/smartfs_smart.c
index c269031d27..64ab2eeb1f 100644
--- a/fs/smartfs/smartfs_smart.c
+++ b/fs/smartfs/smartfs_smart.c
@@ -1890,9 +1890,6 @@ int smartfs_rename(struct inode *mountpt, const char *oldrelpath,
const char *newfilename;
mode_t mode;
uint16_t type;
- uint16_t sector;
- uint16_t offset;
- uint16_t entrysize;
struct smartfs_entry_header_s *direntry;
struct smart_read_write_s readwrite;
@@ -1911,94 +1908,25 @@ int smartfs_rename(struct inode *mountpt, const char *oldrelpath,
oldentry.name = NULL;
newentry.name = NULL;
ret = smartfs_finddirentry(fs, &oldentry, oldrelpath, &oldparentdirsector,
- &oldfilename);
+ &oldfilename);
if (ret < 0)
{
goto errout_with_semaphore;
}
- /* Search for the new entry and validate it DOESN'T exist, unless we
- * are copying to a directory and keeping the same filename, such as:
- *
- * mv /mntpoint/somefile.txt /mntpoint/somedir
- *
- * in which case, we need to validate the /mntpoint/somedir/somefile.txt
- * doens't exsit.
- */
+ /* Search for the new entry and validate it DOESN'T exist. */
ret = smartfs_finddirentry(fs, &newentry, newrelpath, &newparentdirsector,
- &newfilename);
+ &newfilename);
if (ret == OK)
{
- /* Test if it's a file. If it is, then it's an error */
-
- if ((newentry.flags & SMARTFS_DIRENT_TYPE) == SMARTFS_DIRENT_TYPE_FILE)
- {
- ret = -EEXIST;
- goto errout_with_semaphore;
- }
-
- /* Nope, it's a directory. Now search the directory for oldfilename */
-
- sector = newentry.firstsector;
- while (sector != SMARTFS_ERASEDSTATE_16BIT)
- {
- /* Read the next sector of diretory entries */
-
- readwrite.logsector = sector;
- readwrite.offset = 0;
- readwrite.count = fs->fs_llformat.availbytes;
- readwrite.buffer = (uint8_t *) fs->fs_rwbuffer;
- ret = FS_IOCTL(fs, BIOC_READSECT, (unsigned long) &readwrite);
- if (ret < 0)
- {
- ferr("ERROR: Error %d reading sector %d data\n",
- ret, oldentry.dsector);
- goto errout_with_semaphore;
- }
-
- /* Search for newfilename in this sector */
-
- offset = sizeof(struct smartfs_chain_header_s);
- entrysize = sizeof(struct smartfs_entry_header_s) + fs->fs_llformat.namesize;
- while (offset + entrysize < fs->fs_llformat.availbytes)
- {
- /* Test the next entry */
-
- direntry = (struct smartfs_entry_header_s *) &fs->fs_rwbuffer[offset];
- if (((direntry->flags & SMARTFS_DIRENT_EMPTY) ==
- (SMARTFS_ERASEDSTATE_16BIT & SMARTFS_DIRENT_EMPTY)) ||
- ((direntry->flags & SMARTFS_DIRENT_ACTIVE) !=
- (SMARTFS_ERASEDSTATE_16BIT & SMARTFS_DIRENT_ACTIVE)))
- {
- /* This entry isn't valid, skip it */
-
- offset += entrysize;
- continue;
- }
-
- /* Test if this entry matches newfilename */
-
- if (strcmp(newfilename, direntry->name) == 0)
- {
- /* Uh-oh, looks like the entry already exists */
-
- ret = -EEXIST;
- goto errout_with_semaphore;
- }
-
- offset += entrysize;
- }
-
- /* Chain to next sector */
-
- sector = SMARTFS_NEXTSECTOR(((struct smartfs_chain_header_s *)fs->fs_rwbuffer));
- }
-
- /* Okay, we will create newfilename in the newentry directory */
+ /* It is an error if the directory entry at newrelpath already
+ * exists. The necessary steps to avoid this case should have been
+ * handled by higher level logic in the VFS.
+ */
- newparentdirsector = newentry.firstsector;
- newfilename = oldfilename;
+ ret = -EEXIST;
+ goto errout_with_semaphore;
}
/* Test if the new parent directory is valid */
diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c
index b29b0deb14..162220d86d 100644
--- a/fs/vfs/fs_rename.c
+++ b/fs/vfs/fs_rename.c
@@ -39,6 +39,7 @@
#include <nuttx/config.h>
+#include <sys/stat.h>
#include <stdbool.h>
#include <stdio.h>
#include <libgen.h>
@@ -95,6 +96,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
* first, provided that it is not a directory.
*/
+next_subdir:
SETUP_SEARCH(&newdesc, newpath, true);
ret = inode_find(&newdesc);
if (ret >= 0)
@@ -121,6 +123,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
if (newinode->i_child != NULL)
{
FAR char *subdirname;
+ FAR char *tmp;
/* Yes.. In this case, the target of the rename must be a
* subdirectory of newinode, not the newinode itself. For
@@ -128,7 +131,16 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
*/
subdirname = basename((FAR char *)oldpath);
+ tmp = subdir;
+ subdir = NULL;
+
(void)asprintf(&subdir, "%s/%s", newpath, subdirname);
+
+ if (tmp != NULL)
+ {
+ kmm_free(tmp);
+ }
+
if (subdir == NULL)
{
ret = -ENOMEM;
@@ -137,10 +149,12 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
newpath = subdir;
- /* REVISIT: This can be a recursive case, another inode may
- * already exist at oldpth/subdirname. In that case, we need
- * to do this all over again.
+ /* This can be a recursive case, another inode may already exist
+ * at oldpth/subdirname. In that case, we need to do this all
+ * over again. A nasty goto is used because I am lazy.
*/
+
+ goto next_subdir;
}
else
{
@@ -252,10 +266,19 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode,
{
struct inode_search_s newdesc;
FAR struct inode *newinode;
+ FAR const char *newrelpath;
+ FAR char *subdir = NULL;
int ret;
DEBUGASSERT(oldinode->u.i_mops);
+ /* If the file system does not support the rename() method, then bail now. */
+
+ if (oldinode->u.i_mops->rename == NULL)
+ {
+ return -ENOSYS;
+ }
+
/* Get an inode for the new relpath -- it should lie on the same
* mountpoint
*/
@@ -272,7 +295,8 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode,
/* Get the search results */
- newinode = newdesc.node;
+ newinode = newdesc.node;
+ newrelpath = newdesc.relpath;
DEBUGASSERT(newinode != NULL);
/* Verify that the two paths lie on the same mountpoint inode */
@@ -283,27 +307,84 @@ static int mountptrename(FAR const char *oldpath, FAR struct inode *oldinode,
goto errout_with_newinode;
}
- /* Perform the rename operation using the relative paths at the common
- * mountpoint.
+ /* Does a directory entry already exist at the 'rewrelpath'?
+ *
+ * If the directy entry at the newrelpath is a regular file, then that
+ * file should remove removed first.
+ *
+ * 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
+ * moved as if rename(b,a/basename(b)) had been called.
*/
- if (oldinode->u.i_mops->rename)
+ if (oldinode->u.i_mops->stat != NULL)
{
- ret = oldinode->u.i_mops->rename(oldinode, oldrelpath, newdesc.relpath);
- if (ret < 0)
+ struct stat buf;
+
+next_subdir:
+ ret = oldinode->u.i_mops->stat(oldinode, newrelpath, &buf);
+ if (ret >= 0)
{
- goto errout_with_newinode;
+ /* Something exists for this directory entry. Is it a directory? */
+
+ if (S_ISDIR(buf.st_mode))
+ {
+ FAR char *subdirname;
+ FAR char *tmp;
+
+ /* 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.
+ */
+
+ subdirname = basename((FAR char *)oldrelpath);
+ tmp = subdir;
+ subdir = NULL;
+
+ (void)asprintf(&subdir, "%s/%s", newrelpath, subdirname);
+
+ if (tmp != NULL)
+ {
+ kmm_free(tmp);
+ }
+
+ if (subdir == NULL)
+ {
+ ret = -ENOMEM;
+ goto errout_with_newinode;
+ }
+
+ newrelpath = subdir;
+
+ /* This can be a recursive case, another inode may already
+ * exist at oldpth/subdirname. In that case, we need to
+ * do this all over again. A nasty goto is used because
+ * I am lazy.
+ */
+
+ goto next_subdir;
+ }
+ else if (oldinode->u.i_mops->unlink)
+ {
+ /* No.. newrelpath must refer to a regular file. 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.
+ */
+
+ (void)oldinode->u.i_mops->unlink(oldinode, newrelpath);
+ }
}
}
- else
- {
- ret = -ENOSYS;
- goto errout_with_newinode;
- }
- /* Successfully renamed */
+ /* Perform the rename operation using the relative paths at the common
+ * mountpoint.
+ */
- ret = OK;
+ ret = oldinode->u.i_mops->rename(oldinode, oldrelpath, newrelpath);
errout_with_newinode:
inode_release(newinode);
@@ -311,6 +392,11 @@ errout_with_newinode:
errout_with_newsearch:
RELEASE_SEARCH(&newdesc);
+ if (subdir != NULL)
+ {
+ kmm_free(subdir);
+ }
+
return ret;
}
#endif /* CONFIG_DISABLE_MOUNTPOINT */
--
GitLab