On 2/7/25 13:45, Daniel Venzin wrote:
This implementation currently does not support
moving files between directories.
Why would we make such a restriction?
Signed-off-by: Daniel Venzin <daniel.ven...@mt.com>
---
Changes in v3:
- Abort the rename if the destination is a path instead of a file
Changes in v2:
- Separate the rename implementation from changes in the filesystem layer
- Eliminate code duplication with delete_dentry_long(..)
- Ensure that the source file exists, but not the destination file
- Free the fatbuf if it has been reinitialized with fat_itr_root
- Delete orphaned FAT entry after copying file metadata
fs/fat/fat_write.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
fs/fs.c | 2 +-
include/fat.h | 1 +
3 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index c78b80e0b7a..6e4ff7c57d4 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1821,3 +1821,128 @@ exit:
free(dotdent);
return ret;
}
+
+int file_fat_rename(const char *path, const char *new_fname)
Below you are expecting that new_fname is a path not only a file name.
%s/new_fname/new_path/
+{
+ fsdata datablock = { .fatbuf = NULL, };
+ fat_itr *itr = NULL;
+
+ dir_entry *dentry;
+
+ char new_path[VFAT_MAXLEN_BYTES];
+ char *new_fname_copy = NULL, *new_fname_dirname, *new_fname_basename;
+ char *path_copy = NULL, *path_dirname, *path_basename;
+
+ loff_t actwrite;
+
+ __u16 starthi, start;
+ __u32 size, entry;
+
+ int ret = -1;
+
+ new_fname_copy = strdup(new_fname);
+ if (!new_fname_copy)
+ goto exit;
+
+ split_filename(new_fname_copy, &new_fname_dirname, &new_fname_basename);
+
+ if (strlen(new_fname_dirname) > 1 ||
+ (strcmp(new_fname_dirname, "/") != 0 &&
+ strcmp(new_fname_dirname, ".") != 0)) {
No clue, why you expect "/" or "." at the beginning of a path.
In U-Boot file paths are always relative to the root directory.
Shouldn't fat_itr_resolve() take care of checking the validity of the path?
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ path_copy = strdup(path);
+ if (!path_copy)
+ goto exit;
+
+ split_filename(path_copy, &path_dirname, &path_basename);
+ snprintf(new_path, sizeof(new_path), "%s/%s", path_dirname,
new_fname_basename);
You already determined new_fname_dirname. You must not ignore it.
You should delete above statement.
+
+ if (!fat_exists(path)) {
+ ret = -ENOENT;
+ goto exit;
+ }
+
+ if (fat_exists(new_path)) {
+ ret = -EEXIST;
+ goto exit;
+ }
+
+ ret = file_fat_write(new_path, &actwrite, 0, 0, &actwrite);
+ if (ret)
+ goto exit;
+
+ itr = malloc_cache_aligned(sizeof(*itr));
+ if (!itr) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ ret = fat_itr_root(itr, &datablock);
+ if (ret)
+ goto exit;
+
+ ret = fat_itr_resolve(itr, path, TYPE_FILE);
Why don't we allow renaming of directories?
TYPE_FILE | TYPE_DIR
+ if (ret)
+ goto exit;
+
+ dentry = itr->dent;
+
+ starthi = dentry->starthi;
+ start = dentry->start;
+ size = dentry->size;
+
+ free(datablock.fatbuf);
+ datablock.fatbuf = NULL;
+
+ ret = fat_itr_root(itr, &datablock);
+ if (ret)
+ goto exit;
+
+ ret = fat_itr_resolve(itr, new_path, TYPE_FILE);
This would not detect if new_path points to an existing or non-existing
directory.
Check that new_fname_dirname exists and then use the same itr to check
that new_fname_dirname/new_basename does not exist.
This avoids iterating once here and once in fat_rename().
+ if (ret)
+ goto exit;
We don't want to overwrite existing files or directories. Don't we
expect -ENOENT or -ENOTDIR here?
+
+ dentry = itr->dent;
+
+ fsdata *mydata = itr->fsdata;
+
+ entry = START(dentry);
+
+ dentry->starthi = starthi;
+ dentry->start = start;
+ dentry->size = size;
+
+ ret = flush_dir(itr);
+ if (ret)
+ goto exit;
+
+ clear_fatent(mydata, entry);
+ if (flush_dirty_fat_buffer(mydata) < 0) {
+ printf("Error: flush fat buffer\n");
+ ret = -EIO;
+ goto exit;
+ }
+
+ free(datablock.fatbuf);
+ datablock.fatbuf = NULL;
+
+ ret = fat_itr_root(itr, &datablock);
+ if (ret)
+ goto exit;
+
+ ret = fat_itr_resolve(itr, path, TYPE_FILE);
+ if (ret)
+ goto exit;
Why don't you use separate variables for the old and the new itr value?
Then you would not have to resolve the old path twice.
Best regards
Heinrich
+
+ ret = delete_dentry_long(itr, FATENT_CLEAR_SKIP);> +
+exit:
+ free(new_fname_copy);
+ free(path_copy);
+ free(datablock.fatbuf);
+ free(itr);
+ return ret;
+}
diff --git a/fs/fs.c b/fs/fs.c
index b746d05ebcd..3576d5c6644 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -208,7 +208,7 @@ static struct fstype_info fstypes[] = {
.write = file_fat_write,
.unlink = fat_unlink,
.mkdir = fat_mkdir,
- .rename = fs_rename_unsupported,
+ .rename = file_fat_rename,
#else
.write = fs_write_unsupported,
.unlink = fs_unlink_unsupported,
diff --git a/include/fat.h b/include/fat.h
index 3dce99a23cf..415ebd7b7c8 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -207,6 +207,7 @@ int fat_readdir(struct fs_dir_stream *dirs, struct
fs_dirent **dentp);
void fat_closedir(struct fs_dir_stream *dirs);
int fat_unlink(const char *filename);
int fat_mkdir(const char *dirname);
+int file_fat_rename(const char *path, const char *new_filename);
void fat_close(void);
void *fat_next_cluster(fat_itr *itr, unsigned int *nbytes);
