From mboxrd@z Thu Jan  1 00:00:00 1970
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Mon, 1 Feb 2021 17:18:30 +0900
Subject: [PATCH 4/4] fs: fat: remove trailing periods from long name
In-Reply-To: <20210130230953.307517-5-xypron.glpk@gmx.de>
References: <20210130230953.307517-1-xypron.glpk@gmx.de>
 <20210130230953.307517-5-xypron.glpk@gmx.de>
Message-ID: <20210201081830.GA32785@laputa>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Sun, Jan 31, 2021 at 12:09:53AM +0100, Heinrich Schuchardt wrote:
> The FAT32 File System Specification [1] requires leading and trailing
> spaces as well as trailing periods of long names to be ignored.
> 
> This renders a test for '.' and '..' as file name superfluous.
> 
> But we must check that the resulting name has at least one character.
> 
> [1]
>     Microsoft Extensible Firmware Initiative
>     FAT32 File System Specification
>     Version 1.03, December 6, 2000
>     Microsoft Corporation
>     https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  fs/fat/fat_write.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 0f4786ef0f..1b0a0eda09 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -1237,12 +1237,32 @@ again:
>  		}
> 
>  		*last_slash_cont = '\0';
> -		*basename = last_slash_cont + 1;
> +		filename = last_slash_cont + 1;
>  	} else {
>  		*dirname = "/"; /* root by default */
> -		*basename = filename;
>  	}
> 
> +	/*
> +	 * The FAT32 File System Specification v1.03 requires leading and
> +	 * trailing spaces as well as trailing periods to be ignored.
> +	 */
> +	for (; *filename == ' '; ++filename)
> +		;
> +	/* Remove trailing periods and spaces */
> +	for (p = filename + strlen(filename) - 1; p >= filename; --p) {
> +		switch (*p) {
> +		case ' ':
> +		case '.':
> +			*p = 0;
> +			break;
> +		default:
> +			goto done;
> +		}
> +	}

Given the semantics of the functions, split_filename() and normalize_longname(),
the code you added above should be moved to normalize_longname().

> +done:
> +	*basename = filename;
> +
>  	return 0;
>  }
> 
> @@ -1260,10 +1280,7 @@ static int normalize_longname(char *l_filename, const char *filename)
>  {
>  	const char *p, illegal[] = "<>:\"/\\|?*";
> 
> -	if (!strcmp(filename, ".") || !strcmp(filename, ".."))
> -		return -1;

It would be better for the check above to remain here as "." and ".." are
legal directory names. (I even think it would be best to move it to
the caller, file_fat_write_at() or fat_mkdir().)

I think that the suggested sequence would be more intuitive for
better understanding of what Windows requirements say.

-Takahiro Akashi

> -	if (strlen(filename) >= VFAT_MAXLEN_BYTES)
> +	if (!*filename || strlen(filename) >= VFAT_MAXLEN_BYTES)
>  		return -1;
> 
>  	for (p = filename; *p; ++p) {
> --
> 2.29.2
>