From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 2 Feb 2021 15:39:34 +0900 Subject: [PATCH 4/4] fs: fat: remove trailing periods from long name In-Reply-To: <2CCF4B14-2F72-4548-A3E2-E863C9F0121D@gmx.de> 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> Message-ID: <20210202063934.GA31880@laputa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. -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 > >> >> > >> >