From 0aefa87d7f26b501203b531959e3bb075c441fcc Mon Sep 17 00:00:00 2001 From: ludelion5 <58593669+ludelion5@users.noreply.github.com> Date: Sun, 17 Nov 2024 16:47:11 +0100 Subject: [PATCH] implement remove and rename feature for fat32 (#233) * feat: implement remove for fat32, rename in same dir feature for fat32, add new unit-tests --- filesystem/fat32/directory.go | 57 +++++- filesystem/fat32/fat32.go | 130 +++++++++++- filesystem/fat32/fat32_test.go | 355 +++++++++++++++++++++++++++++++++ 3 files changed, 531 insertions(+), 11 deletions(-) diff --git a/filesystem/fat32/directory.go b/filesystem/fat32/directory.go index bd917b75..2996ae28 100644 --- a/filesystem/fat32/directory.go +++ b/filesystem/fat32/directory.go @@ -1,6 +1,7 @@ package fat32 import ( + "fmt" "time" ) @@ -41,7 +42,7 @@ func (d *Directory) entriesToBytes(bytesPerCluster int) ([]byte, error) { func (d *Directory) createEntry(name string, cluster uint32, dir bool) (*directoryEntry, error) { // is it a long filename or a short filename? var isLFN bool - // TODO: convertLfnSfn does not calculate if the short name conflicts and thus shoukld increment the last character + // TODO: convertLfnSfn does not calculate if the short name conflicts and thus should increment the last character // that should happen here, once we can look in the directory entry shortName, extension, isLFN, _ := convertLfnSfn(name) lfn := "" @@ -70,6 +71,60 @@ func (d *Directory) createEntry(name string, cluster uint32, dir bool) (*directo return &entry, nil } +// removeEntry removes an entry in the given directory +func (d *Directory) removeEntry(name string) error { + // TODO implement check for long/short filename after increment of sfn is correctly implemented + + removeEntryIndex := -1 + for i, entry := range d.entries { + if entry.filenameLong == name { // || entry.filenameShort == shortName do not compare SFN, since it is not incremented correctly + removeEntryIndex = i + } + } + + if removeEntryIndex == -1 { + return fmt.Errorf("cannot find entry for name %s", name) + } + + // remove the entry from the list + d.entries = append(d.entries[:removeEntryIndex], d.entries[removeEntryIndex+1:]...) + + return nil +} + +// renameEntry renames an entry in the given directory, and returns the handle to it +func (d *Directory) renameEntry(oldFileName, newFileName string) error { + // TODO implement check for long/short filename after increment of sfn is correctly implemented + + newEntries := make([]*directoryEntry, 0, len(d.entries)) + var isReplaced = false + for _, entry := range d.entries { + if entry.filenameLong == newFileName { + continue // skip adding already existing file, will be overwritten + } + if entry.filenameLong == oldFileName { // || entry.filenameShort == shortName do not compare SFN, since it is not incremented correctly + var lfn string + shortName, extension, isLFN, _ := convertLfnSfn(newFileName) + if isLFN { + lfn = newFileName + } + entry.filenameLong = lfn + entry.filenameShort = shortName + entry.fileExtension = extension + entry.modifyTime = time.Now() + isReplaced = true + } + newEntries = append(newEntries, entry) + } + if !isReplaced { + return fmt.Errorf("cannot find file entry for %s", oldFileName) + } + + d.entries = newEntries + + return nil +} + // createVolumeLabel create a volume label entry in the given directory, and return the handle to it func (d *Directory) createVolumeLabel(name string) (*directoryEntry, error) { // allocate a slot for the new filename in the existing directory diff --git a/filesystem/fat32/fat32.go b/filesystem/fat32/fat32.go index fffb6027..5c17bffa 100644 --- a/filesystem/fat32/fat32.go +++ b/filesystem/fat32/fat32.go @@ -637,18 +637,128 @@ func (fs *FileSystem) OpenFile(p string, flag int) (filesystem.File, error) { }, nil } +// removes the named file or (empty) directory. +func (fs *FileSystem) Remove(pathname string) error { + // get the path + dir := path.Dir(pathname) + filename := path.Base(pathname) + // if the dir == filename, then it is just / + if dir == filename { + return fmt.Errorf("cannot remove directory %s as file", pathname) + } + // get the directory entries + parentDir, entries, err := fs.readDirWithMkdir(dir, false) + if err != nil { + return fmt.Errorf("could not read directory entries for %s", dir) + } + // we now know that the directory exists, see if the file exists + var targetEntry *directoryEntry + for _, e := range entries { + shortName := e.filenameShort + if e.fileExtension != "" { + shortName += "." + e.fileExtension + } + if e.filenameLong != filename && shortName != filename { + continue + } + // cannot do anything with directories + if e.isSubdirectory { + content, err := fs.ReadDir(pathname) + if err != nil { + return fmt.Errorf("error while checking if file to delete is empty: %+v", err) + } + // '.' & '..' are always present in directory + if len(content) > 2 { + return fmt.Errorf("cannot remove non-empty directory %s", pathname) + } + } + // if we got this far, we have found the file + targetEntry = e + } + + // see if the file exists + // if the file does not exist, and is not opened for os.O_CREATE, return an error + if targetEntry == nil { + return fmt.Errorf("target file %s does not exist", pathname) + } + err = parentDir.removeEntry(filename) + if err != nil { + return fmt.Errorf("failed to remove file %s: %v", pathname, err) + } + + // we need to make sure that clusters are removed which may not be used anymore + _, err = fs.allocateSpace(uint64(parentDir.fileSize), parentDir.clusterLocation) + if err != nil { + return fmt.Errorf("failed to allocate clusters: %v", err) + } + + // write the directory entries to disk + err = fs.writeDirectoryEntries(parentDir) + if err != nil { + return fmt.Errorf("error writing directory file %s to disk: %v", pathname, err) + } + + return nil +} + // Rename renames (moves) oldpath to newpath. If newpath already exists and is not a directory, Rename replaces it. -// -//nolint:revive // parameters will be used eventually func (fs *FileSystem) Rename(oldpath, newpath string) error { - return filesystem.ErrNotImplemented -} + // get the path + dir := path.Dir(oldpath) + filename := path.Base(oldpath) -// removes the named file or (empty) directory. -// -//nolint:revive // parameters will be used eventually -func (fs *FileSystem) Remove(pathname string) error { - return filesystem.ErrNotImplemented + newDir := path.Dir(newpath) + newname := path.Base(newpath) + if dir != newDir { + return errors.New("can only rename files within the same directory") + } + + // if the dir == filename, then it is just / + if dir == filename { + return fmt.Errorf("cannot rename directory %s as file", oldpath) + } + // get the directory entries + parentDir, entries, err := fs.readDirWithMkdir(dir, false) + if err != nil { + return fmt.Errorf("could not read directory entries for %s", dir) + } + // we now know that the directory exists, see if the file exists + var targetEntry *directoryEntry + for _, e := range entries { + shortName := e.filenameShort + if e.fileExtension != "" { + shortName += "." + e.fileExtension + } + if e.filenameLong != filename && shortName != filename { + continue + } + // if we got this far, we have found the file + targetEntry = e + } + + // see if the file exists + // if the file does not exist, and is not opened for os.O_CREATE, return an error + if targetEntry == nil { + return fmt.Errorf("target file %s does not exist", oldpath) + } + err = parentDir.renameEntry(filename, newname) + if err != nil { + return fmt.Errorf("failed to rename file %s: %v", oldpath, err) + } + + // we need to make sure that clusters are removed which may not be used anymore + _, err = fs.allocateSpace(uint64(parentDir.fileSize), parentDir.clusterLocation) + if err != nil { + return fmt.Errorf("failed to allocate clusters: %v", err) + } + + // write the directory entries to disk + err = fs.writeDirectoryEntries(parentDir) + if err != nil { + return fmt.Errorf("error writing directory file %s to disk: %v", oldpath, err) + } + + return nil } // Label get the label of the filesystem from the secial file in the root directory. @@ -801,7 +911,7 @@ func (fs *FileSystem) mkSubdir(parent *Directory, name string) (*directoryEntry, } func (fs *FileSystem) writeDirectoryEntries(dir *Directory) error { - // we need to save the entries of theparent + // we need to save the entries of the parent b, err := dir.entriesToBytes(fs.bytesPerCluster) if err != nil { return fmt.Errorf("could not create a valid byte stream for a FAT32 Entries: %w", err) diff --git a/filesystem/fat32/fat32_test.go b/filesystem/fat32/fat32_test.go index 99033c04..70e822f8 100644 --- a/filesystem/fat32/fat32_test.go +++ b/filesystem/fat32/fat32_test.go @@ -1144,3 +1144,358 @@ func TestCreateFileTree(t *testing.T) { t.Errorf("Error making gigfile1 %s: %v", file, err) } } + +func Test_Rename(t *testing.T) { + workingPath := "/" + srcFile := "old.txt" + dstFile := "new.txt" + createFile := func(t *testing.T, fs *fat32.FileSystem, name, content string) { + t.Helper() + origFile, err := fs.OpenFile(filepath.Join(workingPath, name), os.O_CREATE|os.O_RDWR) + if err != nil { + t.Fatalf("Could not create file %s: %+v", name, err) + } + defer origFile.Close() + // write test file + _, err = origFile.Write([]byte(content)) + if err != nil { + t.Fatalf("Could not Write file %s, %+v", name, err) + } + } + readFile := func(t *testing.T, fs *fat32.FileSystem, name string) string { + t.Helper() + file, err := fs.OpenFile(filepath.Join(workingPath, name), os.O_RDONLY) + if err != nil { + t.Fatalf("file %s does not exist: %+v", name, err) + } + defer file.Close() + buf := &bytes.Buffer{} + _, err = io.Copy(buf, file) + if err != nil { + t.Fatalf("Could not read file %s: %+v", name, err) + } + return buf.String() + } + tests := []struct { + name string + hasError bool + pre func(t *testing.T, fs *fat32.FileSystem) + post func(t *testing.T, fs *fat32.FileSystem) + }{ + { + name: "simple renaming works without errors", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + createFile(t, fs, srcFile, "FooBar") + }, + post: func(_ *testing.T, fs *fat32.FileSystem) { + // check if original file is there -> should not be the case + origFile, err := fs.OpenFile(srcFile, os.O_RDONLY) + if err == nil { + defer origFile.Close() + t.Fatal("Original file is still there") + } + // check if new file is there -> should be the case + content := readFile(t, fs, dstFile) + if content != "FooBar" { + t.Fatalf("Content should be '%s', but is '%s'", "FooBar", content) + } + }, + }, + { + name: "destination file already exists and gets overwritten", + hasError: false, + pre: func(_ *testing.T, fs *fat32.FileSystem) { + createFile(t, fs, srcFile, "FooBar") + // create destination file + createFile(t, fs, dstFile, "This should be overwritten") + }, + post: func(_ *testing.T, fs *fat32.FileSystem) { + origFile, err := fs.OpenFile(filepath.Join(workingPath, srcFile), os.O_RDONLY) + if err == nil { + defer origFile.Close() + t.Fatal("Original file is still there") + } + // check if new file is there -> should be the case + content := readFile(t, fs, dstFile) + if content != "FooBar" { + t.Fatalf("Content should be '%s', but is '%s'", "FooBar", content) + } + }, + }, + { + name: "source file does not exist", + hasError: true, + pre: func(_ *testing.T, _ *fat32.FileSystem) { + // do not create orig file + }, + post: func(_ *testing.T, _ *fat32.FileSystem) { + + }, + }, + { + name: "renaming long file to short file", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + var s string + for i := 0; i < 255; i++ { + s += "a" + } + srcFile = s + createFile(t, fs, s, "orig") + }, + post: func(_ *testing.T, _ *fat32.FileSystem) { + srcFile = "old.txt" + }, + }, + { + name: "renaming short file to long file", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + var s string + for i := 0; i < 255; i++ { + s += "a" + } + dstFile = s + createFile(t, fs, srcFile, "orig") + }, + post: func(_ *testing.T, _ *fat32.FileSystem) { + dstFile = "new.txt" + }, + }, + { + name: "rename a non empty directory", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + err := fs.Mkdir(filepath.Join(workingPath, srcFile)) + if err != nil { + t.Fatalf("Could not create directory %s: %+v", srcFile, err) + } + // create file in directory which is going to be moved + createFile(t, fs, filepath.Join(srcFile, "test.txt"), "FooBar") + }, + post: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + _, err := fs.ReadDir(filepath.Join(workingPath, srcFile)) + if err == nil { + t.Fatalf("source directory does exist: %+v", err) + } + _, err = fs.ReadDir(filepath.Join(workingPath, dstFile)) + if err != nil { + t.Fatalf("destination directory does not exist: %+v", err) + } + content := readFile(t, fs, filepath.Join(dstFile, "test.txt")) + if content != "FooBar" { + t.Fatalf("Content should be '%s', but is '%s'", "FooBar", content) + } + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // get a mock filesystem image + f, err := tmpFat32(false, 0, 0) + if err != nil { + t.Fatal(err) + } + + if keepTmpFiles == "" { + defer os.Remove(f.Name()) + } else { + fmt.Println(f.Name()) + } + + fileInfo, err := f.Stat() + if err != nil { + t.Fatalf("error getting file info for tmpfile %s: %v", f.Name(), err) + } + + // create an empty filesystem + fs, err := fat32.Create(f, fileInfo.Size(), 0, 512, "go-diskfs") + if err != nil { + t.Fatalf("error creating fat32 filesystem: %v", err) + } + + test.pre(t, fs) + + err = fs.Rename(filepath.Join(workingPath, srcFile), filepath.Join(workingPath, dstFile)) + + if test.hasError { + if err == nil { + t.Fatal("No Error renaming file", err) + } + } else { + if err != nil { + t.Fatal("Error renaming file", err) + } + } + + test.post(t, fs) + }) + } +} + +func Test_Remove(t *testing.T) { + workingPath := "/" + fileToRemove := "fileToRemove.txt" + createFile := func(t *testing.T, fs *fat32.FileSystem, name, content string) { + t.Helper() + origFile, err := fs.OpenFile(filepath.Join(workingPath, name), os.O_CREATE|os.O_RDWR) + if err != nil { + t.Fatalf("Could not create file %s: %+v", name, err) + } + defer origFile.Close() + // write test file + _, err = origFile.Write([]byte(content)) + if err != nil { + t.Fatalf("Could not Write file %s, %+v", name, err) + } + } + tests := []struct { + name string + hasError bool + errorMsg string + pre func(t *testing.T, fs *fat32.FileSystem) + post func(t *testing.T, fs *fat32.FileSystem) + }{ + { + name: "simple remove works without", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + createFile(t, fs, fileToRemove, "FooBar") + }, + post: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + // check if original file is there -> should not be the case + origFile, err := fs.OpenFile(fileToRemove, os.O_RDONLY) + if err == nil { + defer origFile.Close() + t.Fatal("Original file is still there") + } + }, + }, + { + name: "file to remove does not exist", + hasError: true, + errorMsg: "target file /fileToRemove.txt does not exist", + pre: func(_ *testing.T, _ *fat32.FileSystem) { + // do not create any file + }, + post: func(_ *testing.T, _ *fat32.FileSystem) { + + }, + }, + { + name: "removing multiple files", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + var s string + for i := 0; i < 10240; i++ { + s += "this is a big file\n" + } + for i := 0; i < 50; i++ { + createFile(t, fs, fmt.Sprintf("file%d.txt", i), "small file") + } + createFile(t, fs, fileToRemove, s) + for i := 50; i < 100; i++ { + createFile(t, fs, fmt.Sprintf("file%d.txt", i), "small file") + } + }, + post: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + for i := 0; i < 100; i++ { + err := fs.Remove(fmt.Sprintf("/file%d.txt", i)) + if err != nil { + t.Fatalf("expected no error, but got %v", err) + } + } + }, + }, + { + name: "removing empty dir", + hasError: false, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + if err := fs.Mkdir(filepath.Join(workingPath, fileToRemove)); err != nil { + t.Fatalf("could not create test directory: %+v", err) + } + }, + post: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + _, err := fs.ReadDir(filepath.Join(workingPath, fileToRemove)) + if err == nil { + t.Fatalf("Expected that dir cannot be read, but is still there") + } + }, + }, + { + name: "cannot delete dir with content", + hasError: true, + pre: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + if err := fs.Mkdir(filepath.Join(workingPath, fileToRemove)); err != nil { + t.Fatalf("could not create test directory: %+v", err) + } + // file within dir to remove + createFile(t, fs, filepath.Join(workingPath, fileToRemove, "test"), "foo") + }, + post: func(t *testing.T, fs *fat32.FileSystem) { + t.Helper() + _, err := fs.ReadDir(filepath.Join(workingPath, fileToRemove)) + if err != nil { + t.Fatalf("Expected that dir can be read, but has error: %+v", err) + } + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // get a mock filesystem image + f, err := tmpFat32(false, 0, 0) + if err != nil { + t.Fatal(err) + } + + if keepTmpFiles == "" { + defer os.Remove(f.Name()) + } else { + fmt.Println(f.Name()) + } + + fileInfo, err := f.Stat() + if err != nil { + t.Fatalf("error getting file info for tmpfile %s: %v", f.Name(), err) + } + + // create an empty filesystem + fs, err := fat32.Create(f, fileInfo.Size(), 0, 512, "go-diskfs") + if err != nil { + t.Fatalf("error creating fat32 filesystem: %v", err) + } + + test.pre(t, fs) + + err = fs.Remove(filepath.Join(workingPath, fileToRemove)) + + if test.hasError { + if err == nil { + t.Fatal("No Error renaming file", err) + } else if !strings.Contains(err.Error(), test.errorMsg) { + t.Fatalf("Error does not contain expected msg: %s. Original error: %v", test.errorMsg, err) + } + } else { + if err != nil { + t.Fatal("Error removing file", err) + } + } + + test.post(t, fs) + }) + } +}