Am 2. Februar 2021 00:54:58 MEZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
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 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;
*basename = filename;
- /*
* The FAT32 File System Specification v1.03 requires leadingand
* 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.
We should not duplicate code in each caller.
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