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

Heinrich Schuchardt xypron.glpk at gmx.de
Fri Jan 3 23:59:48 CET 2025


On 12.12.24 23:06, Burak Gerz wrote:
> Implement a simple FAT file rename.
>
> Signed-off-by: Burak Gerz <burak at 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.



More information about the U-Boot mailing list