From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Tue, 02 Feb 2021 07:52:22 +0100 Subject: [PATCH 4/4] fs: fat: remove trailing periods from long name In-Reply-To: <20210202063934.GA31880@laputa> References: <20210130230953.307517-1-xypron.glpk@gmx.de> <20210130230953.307517-5-xypron.glpk@gmx.de> <20210201081830.GA32785@laputa> <20210201235458.GA12814@laputa> <2CCF4B14-2F72-4548-A3E2-E863C9F0121D@gmx.de> <20210202063934.GA31880@laputa> Message-ID: <78E26052-DA1D-4FB8-B52F-84C3E8D13200@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am 2. Februar 2021 07:39:34 MEZ schrieb AKASHI Takahiro : >On Tue, Feb 02, 2021 at 07:05:53AM +0100, Heinrich Schuchardt wrote: >> Am 2. Februar 2021 00:54:58 MEZ schrieb AKASHI Takahiro >: >> >On Mon, Feb 01, 2021 at 01:34:59PM +0100, Heinrich Schuchardt wrote: >> >> 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 >> >> >> --- >> >> >> 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(). >> > >> >That is why I also suggested, "I even think it would be best to move >it >> >to >> >the caller, file_fat_write_at() or 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. >> > >> >Again, "." and ".." are legal directory names. >> >To reject a request of creating such names is a caller's job, >> >not split_filename()'s as its name suggests. >> > >> >> These filenames are illegal for all callers. > >The purpose of split_filename() is to separate a file name from >its directory path. As I said, excluding "." or ".." is out of this >function's scope, but the caller's semantics. > >> We should not duplicate code in each caller. > >I don't think so. >handling "." or ".." entry and removing leading/trailing spaces >are totally different. Trailing periods have to be removed here too. Putting special handling for '.' and '..' here *and* in each caller will increase code size without any gain in functionality. Best regards Heinrich > >-Takahiro Akashi > >> Best regards >> >> Heinrich >> >> >> >-Takahiro Akashi >> > >> >> 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 >> >> >> >> >> >>