Re: [PATCH 2/2] fs: fat: Implement FAT file rename

On 12.12.24 23:06, Burak Gerz wrote:

Implement a simple FAT file rename.

Signed-off-by: Burak Gerz burak@gerz.io

Please, separate the change into two patches.

1) define fs_rename() 2) implement ffat_rename()

---

fs/fat/fat_write.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++ fs/fs.c | 30 ++++++++++++++ include/fat.h | 1 + include/fs.h | 2 + 4 files changed, 132 insertions(+)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index ea877ee917..acd0e22652 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1813,3 +1813,102 @@ exit: free(dotdent); return ret; }

+int file_fat_rename(const char *path, const char *new_filename)

%s/file_fat_rename/fat_rename/

+{

• char *dirname, *basename;
• char path_copy[VFAT_MAXLEN_BYTES];
• char new_path[VFAT_MAXLEN_BYTES];
• dir_entry *dentry_src, *dentry_dst;
• fsdata src_datablock = { .fatbuf = NULL };
• fsdata dst_datablock = { .fatbuf = NULL };
• fat_itr *src_itr = NULL;
• fat_itr *dst_itr = NULL;
• loff_t actwrite;
• int ret;
• void *buffer = &actwrite;
• if (strlen(new_filename) > VFAT_MAXLEN_BYTES)

• return -EINVAL;
  

• strcpy(path_copy, path);
• split_filename(path_copy, &dirname, &basename);
• strcpy(new_path, dirname);
• strcat(new_path, "/");
• strcat(new_path, new_filename);
• src_itr = malloc_cache_aligned(sizeof(*src_itr));

• if (!src_itr)
  

• 	goto exit;
  

• dst_itr = malloc_cache_aligned(sizeof(*dst_itr));

• if (!dst_itr)
  

• 	goto exit;
  

• ret = fat_itr_root(src_itr, &src_datablock);
• if (ret)

• goto exit;
  

• ret = fat_itr_root(dst_itr, &dst_datablock);
• if (ret)

• goto exit;
  

• ret = fat_itr_resolve(src_itr, dirname, TYPE_DIR);
• if (ret)

• goto exit;
  

• dentry_src = find_directory_entry(src_itr, (char *)basename);
• if (!dentry_src)

• goto exit;
  

• dst_datablock.fatbuf = NULL;
• fat_itr_root(dst_itr, &dst_datablock);
• ret = fat_itr_resolve(dst_itr, dirname, TYPE_DIR);
• if (ret)

• goto exit;
  

• dentry_dst = find_directory_entry(dst_itr, (char *)new_filename);
• if (dentry_dst)

• goto exit;
  

This duplicates code from fat_mkdir() and file_fat_write_at(). Please, carve out a common functions.

• ret = file_fat_write(new_path, buffer, 0, 0, &actwrite);

Before any update you must check that new_path does not exist.

• if (ret)

• goto exit;
  

• dst_datablock.fatbuf = NULL;
• fat_itr_root(dst_itr, &dst_datablock);
• ret = fat_itr_resolve(dst_itr, dirname, TYPE_DIR);
• if (ret)

• goto exit;
  

• dentry_dst = find_directory_entry(dst_itr, (char *)new_filename);
• if (!dentry_dst)

• goto exit;
  

• dentry_dst->starthi = dentry_src->starthi;
• dentry_dst->start = dentry_src->start;
• dentry_dst->size = dentry_src->size;
• ret = flush_dir(dst_itr);
• if (ret)

• return -1;
  

• src_datablock.fatbuf = NULL;
• ret = fat_itr_root(src_itr, &src_datablock);
• if (ret)

• goto exit;
  

• ret = fat_itr_resolve(src_itr, dirname, TYPE_DIR);
• if (ret)

• goto exit;
  

• dentry_src = find_directory_entry(src_itr, (char *)basename);
• if (!dentry_src)

• goto exit;
  

• dir_entry *dent = src_itr->dent;
• if (src_itr->clust != src_itr->dent_clust) {

• ret = fat_move_to_cluster(src_itr, src_itr->dent_clust);
  

• if (ret)
  

• 	goto exit;
  

• }
• src_itr->dent = src_itr->dent_start;
• src_itr->remaining = src_itr->dent_rem;
• dent = src_itr->dent_start;
• if ((dent->attr & ATTR_VFAT) == ATTR_VFAT &&

•    (dent->nameext.name[0] & LAST_LONG_ENTRY_MASK)) {
  

• ret = delete_long_name(src_itr);
  

• if (ret)
  

• 	goto exit;
  

• }
• delete_single_dentry(src_itr);

You are duplicating code from delete_dentry_long(). This can be avoided by moving the freeing of cluster blocks out of delete_dentry_long() and calling this function here.

• ret = flush_dir(src_itr);

+exit:

• free(src_itr);
• free(dst_itr);
• return ret;

+} diff --git a/fs/fs.c b/fs/fs.c index 21a23efd93..3024bd2cec 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -118,6 +118,11 @@ static inline int fs_ln_unsupported(const char *filename, const char *target) return -1; }

+static inline int fs_rename_unsupported(const char *filename, const char *target) +{

• return -1;

+}

• static inline void fs_close_unsupported(void) { }

@@ -183,6 +188,7 @@ struct fstype_info { int (*unlink)(const char *filename); int (*mkdir)(const char *dirname); int (*ln)(const char *filename, const char *target);

• int (*rename)(const char *old_filename_path, const char *new_filename); };

  static struct fstype_info fstypes[] = {

@@ -201,6 +207,7 @@ static struct fstype_info fstypes[] = { .write = file_fat_write, .unlink = fat_unlink, .mkdir = fat_mkdir,

• .rename = file_fat_rename,
  

  #else .write = fs_write_unsupported, .unlink = fs_unlink_unsupported,

@@ -222,6 +229,7 @@ static struct fstype_info fstypes[] = { .probe = ext4fs_probe, .close = ext4fs_close, .ls = fs_ls_generic,

• .rename = fs_rename_unsupported,
  

  .exists = ext4fs_exists, .size = ext4fs_size, .read = ext4_read_file,

@@ -247,6 +255,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = true, .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close,

• .rename = fs_rename_unsupported,
  

  .ls = sandbox_fs_ls, .exists = sandbox_fs_exists, .size = sandbox_fs_size,

@@ -266,6 +275,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = true, .probe = smh_fs_set_blk_dev, .close = fs_close_unsupported,

• .rename = fs_rename_unsupported,
  

  .ls = fs_ls_unsupported, .exists = fs_exists_unsupported, .size = smh_fs_size,

@@ -284,6 +294,7 @@ static struct fstype_info fstypes[] = { .fstype = FS_TYPE_UBIFS, .name = "ubifs", .null_dev_desc_ok = true,

• .rename = fs_rename_unsupported,
  

  .probe = ubifs_set_blk_dev, .close = ubifs_close, .ls = ubifs_ls,

@@ -305,6 +316,7 @@ static struct fstype_info fstypes[] = { .fstype = FS_TYPE_BTRFS, .name = "btrfs", .null_dev_desc_ok = false,

• .rename = fs_rename_unsupported,
  

  .probe = btrfs_probe, .close = btrfs_close, .ls = btrfs_ls,

@@ -327,6 +339,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = false, .probe = sqfs_probe, .opendir = sqfs_opendir,

• .rename = fs_rename_unsupported,
  

  .readdir = sqfs_readdir, .ls = fs_ls_generic, .read = sqfs_read,

@@ -348,6 +361,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = false, .probe = erofs_probe, .opendir = erofs_opendir,

• .rename = fs_rename_unsupported,
  

  .readdir = erofs_readdir, .ls = fs_ls_generic, .read = erofs_read,

@@ -369,6 +383,7 @@ static struct fstype_info fstypes[] = { .probe = fs_probe_unsupported, .close = fs_close_unsupported, .ls = fs_ls_unsupported,

• .rename = fs_rename_unsupported,
  

  .exists = fs_exists_unsupported, .size = fs_size_unsupported, .read = fs_read_unsupported,

@@ -697,6 +712,21 @@ int fs_mkdir(const char *dirname) return ret; }

+int fs_rename(const char *path, const char *new_filename) +{

• struct fstype_info *info = fs_get_info(fs_type);
• int ret;
• ret = info->rename(path, new_filename);

Please, check if the name is equal before calling rename().

Best regards

Heinrich

• if (ret < 0) {

• log_err("** Unable to rename from %s to %s **\n", path, new_filename);
  

•    ret = -1;
  

• }
• fs_close();
• return ret;

+}

• int fs_ln(const char *fname, const char *target) { struct fstype_info *info = fs_get_info(fs_type);

diff --git a/include/fat.h b/include/fat.h index 3dce99a23c..1155e59c68 100644 --- a/include/fat.h +++ b/include/fat.h @@ -208,6 +208,7 @@ void fat_closedir(struct fs_dir_stream *dirs); int fat_unlink(const char *filename); int fat_mkdir(const char *dirname); void fat_close(void); +int file_fat_rename(const char *filename, const char *buffer); void *fat_next_cluster(fat_itr *itr, unsigned int *nbytes);

/** diff --git a/include/fs.h b/include/fs.h index 2474880385..c8d0eb41c9 100644 --- a/include/fs.h +++ b/include/fs.h @@ -165,6 +165,8 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len, int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, loff_t *actwrite);

+int fs_rename(const char *path, const char *filename);

• /*
  • Directory entry types, matches the subset of DT_x in posix readdir()
  • which apply to u-boot.