Re: [PATCH 3/3] fs: fat: carve out fat_create_dir_entry()

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Tue, Jul 12, 2022 at 10:33:14PM +0000, Heinrich Schuchardt wrote:
> fat_mkdir() and file_fat_write_at() use identical code to create a new
> directory entry. Carve out a new function fat_create_dir_entry() to avoid
> this code duplication.

Why not merge your patch[1] here as you're going to newly introduce fat_create_dir_entry()?

[1] https://lists.denx.de/pipermail/u-boot/2022-July/488693.html

-Takahiro Akashi


> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  fs/fat/fat_write.c | 93 ++++++++++++++++++++--------------------------
>  1 file changed, 40 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 57522f96a8..a25b2283d4 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -1314,6 +1314,43 @@ static int normalize_longname(char *l_filename, const char *filename)
>  	return 0;
>  }
> 
> +/**
> + * fat_create_dir_entry() - create directory entry
> + *
> + * @itr:	directory iterator
> + * @basename:	name of file or directory to be created
> + * @size:	file size
> + * @attr:	file or directory attributes
> + * Return:	0 for success, -EIO on error
> + */
> +static int fat_create_dir_entry(fat_itr *itr, const char *basename,
> +				loff_t size, u8 attr)
> +{
> +	/* Create a new file */
> +	char shortname[SHORT_NAME_SIZE];
> +	int ndent;
> +	int ret;
> +
> +	/* Check if long name is needed */
> +	ndent = set_name(itr, basename, shortname);
> +	if (ndent < 0)
> +		return ndent;
> +	ret = fat_find_empty_dentries(itr, ndent);
> +	if (ret)
> +		return ret;
> +	if (ndent > 1) {
> +		/* Set long name entries */
> +		ret = fill_dir_slot(itr, basename, shortname);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Set short name entry */
> +	fill_dentry(itr->fsdata, itr->dent, shortname, 0, size, attr);
> +
> +	return 0;
> +}
> +
>  int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
>  		      loff_t size, loff_t *actwrite)
>  {
> @@ -1383,8 +1420,6 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
>  		retdent->size = cpu_to_le32(pos + size);
>  	} else {
>  		/* Create a new file */
> -		char shortname[SHORT_NAME_SIZE];
> -		int ndent;
> 
>  		if (pos) {
>  			/* No hole allowed */
> @@ -1392,25 +1427,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
>  			goto exit;
>  		}
> 
> -		/* Check if long name is needed */
> -		ndent = set_name(itr, basename, shortname);
> -		if (ndent < 0) {
> -			ret = ndent;
> -			goto exit;
> -		}
> -		ret = fat_find_empty_dentries(itr, ndent);
> -		if (ret)
> -			goto exit;
> -		if (ndent > 1) {
> -			/* Set long name entries */
> -			ret = fill_dir_slot(itr, basename, shortname);
> -			if (ret)
> -				goto exit;
> -		}
> -
> -		/* Set short name entry */
> -		fill_dentry(itr->fsdata, itr->dent, shortname, 0, size,
> -			    ATTR_ARCH);
> +		ret = fat_create_dir_entry(itr, basename, size, ATTR_ARCH);
> 
>  		retdent = itr->dent;
>  	}
> @@ -1693,38 +1710,8 @@ int fat_mkdir(const char *dirname)
>  		ret = -EEXIST;
>  		goto exit;
>  	} else {
> -		char shortname[SHORT_NAME_SIZE];
> -		int ndent;
> -
> -		if (itr->is_root) {
> -			/* root dir cannot have "." or ".." */
> -			if (!strcmp(l_dirname, ".") ||
> -			    !strcmp(l_dirname, "..")) {
> -				ret = -EINVAL;
> -				goto exit;
> -			}
> -		}
> -
> -		/* Check if long name is needed */
> -		ndent = set_name(itr, basename, shortname);
> -		if (ndent < 0) {
> -			ret = ndent;
> -			goto exit;
> -		}
> -		ret = fat_find_empty_dentries(itr, ndent);
> -		if (ret)
> -			goto exit;
> -		if (ndent > 1) {
> -			/* Set long name entries */
> -			ret = fill_dir_slot(itr, basename, shortname);
> -			if (ret)
> -				goto exit;
> -		}
> -
> -		/* Set attribute as archive for regular file */
> -		fill_dentry(itr->fsdata, itr->dent, shortname, 0, 0,
> -			    ATTR_DIR | ATTR_ARCH);
> -
> +		ret = fat_create_dir_entry(itr, basename, 0,
> +					   ATTR_DIR | ATTR_ARCH);
>  		retdent = itr->dent;
>  	}
> 
> --
> 2.30.2
>