[PATCH 4/4] fs: fat: remove trailing periods from long name

[Heinrich Schuchardt]

On 01.02.21 09:18, AKASHI Takahiro wrote:
> 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 at 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().

normalize_longname(l_filename, filename) converts the argument filename
to a lowercase string l_filename. The parameter filename remains
unchanged. But it is the value of filename that is used to create the
new directory entry in file_fat_write_at() and fat_mkdir().

So moving the change to normalize_longname() will not lead to the
intended behavior.

Removing leading and trailing blanks fits well into the task of
split_filename to identify the actual file name.

Best regards

Heinrich

>
>> +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
>>