From 79b882e901348b8b51120c63203efb1ebc715acb Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Mon, 17 Feb 2020 09:22:32 +0100 Subject: [PATCH] Merge duplicated readdir functionality internal/archiver.readdir and internal/fs.ReadDir were unused. internal/fs.ReadDirNames and internal/archiver.readdirnames were doing nearly the same thing, except one sorted its output and opened with fs.O_NOFOLLOW. Both were only used in internal/archiver. --- internal/archiver/archiver.go | 55 +++++++---------------------------- internal/archiver/scanner.go | 4 ++- internal/archiver/tree.go | 2 +- internal/fs/fs_helpers.go | 45 ---------------------------- 4 files changed, 15 insertions(+), 91 deletions(-) delete mode 100644 internal/fs/fs_helpers.go diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 222dd1507..5a3fec108 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -216,10 +216,11 @@ func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo return FutureTree{}, err } - names, err := readdirnames(arch.FS, dir) + names, err := readdirnames(arch.FS, dir, fs.O_NOFOLLOW) if err != nil { return FutureTree{}, err } + sort.Strings(names) nodes := make([]FutureNode, 0, len(names)) @@ -628,43 +629,9 @@ func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree, return tree, nil } -type fileInfoSlice []os.FileInfo - -func (fi fileInfoSlice) Len() int { - return len(fi) -} - -func (fi fileInfoSlice) Swap(i, j int) { - fi[i], fi[j] = fi[j], fi[i] -} - -func (fi fileInfoSlice) Less(i, j int) bool { - return fi[i].Name() < fi[j].Name() -} - -func readdir(filesystem fs.FS, dir string) ([]os.FileInfo, error) { - f, err := filesystem.OpenFile(dir, fs.O_RDONLY|fs.O_NOFOLLOW, 0) - if err != nil { - return nil, errors.Wrap(err, "Open") - } - - entries, err := f.Readdir(-1) - if err != nil { - _ = f.Close() - return nil, errors.Wrapf(err, "Readdir %v failed", dir) - } - - err = f.Close() - if err != nil { - return nil, err - } - - sort.Sort(fileInfoSlice(entries)) - return entries, nil -} - -func readdirnames(filesystem fs.FS, dir string) ([]string, error) { - f, err := filesystem.OpenFile(dir, fs.O_RDONLY|fs.O_NOFOLLOW, 0) +// flags are passed to fs.OpenFile. O_RDONLY is implied. +func readdirnames(filesystem fs.FS, dir string, flags int) ([]string, error) { + f, err := filesystem.OpenFile(dir, fs.O_RDONLY|flags, 0) if err != nil { return nil, errors.Wrap(err, "Open") } @@ -680,32 +647,32 @@ func readdirnames(filesystem fs.FS, dir string) ([]string, error) { return nil, err } - sort.Sort(sort.StringSlice(entries)) return entries, nil } // resolveRelativeTargets replaces targets that only contain relative // directories ("." or "../../") with the contents of the directory. Each // element of target is processed with fs.Clean(). -func resolveRelativeTargets(fs fs.FS, targets []string) ([]string, error) { +func resolveRelativeTargets(filesys fs.FS, targets []string) ([]string, error) { debug.Log("targets before resolving: %v", targets) result := make([]string, 0, len(targets)) for _, target := range targets { - target = fs.Clean(target) - pc, _ := pathComponents(fs, target, false) + target = filesys.Clean(target) + pc, _ := pathComponents(filesys, target, false) if len(pc) > 0 { result = append(result, target) continue } debug.Log("replacing %q with readdir(%q)", target, target) - entries, err := readdirnames(fs, target) + entries, err := readdirnames(filesys, target, fs.O_NOFOLLOW) if err != nil { return nil, err } + sort.Strings(entries) for _, name := range entries { - result = append(result, fs.Join(target, name)) + result = append(result, filesys.Join(target, name)) } } diff --git a/internal/archiver/scanner.go b/internal/archiver/scanner.go index bd789893c..71634015b 100644 --- a/internal/archiver/scanner.go +++ b/internal/archiver/scanner.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "sort" "github.com/restic/restic/internal/fs" ) @@ -86,10 +87,11 @@ func (s *Scanner) scan(ctx context.Context, stats ScanStats, target string) (Sca stats.Files++ stats.Bytes += uint64(fi.Size()) case fi.Mode().IsDir(): - names, err := readdirnames(s.FS, target) + names, err := readdirnames(s.FS, target, fs.O_NOFOLLOW) if err != nil { return stats, s.Error(target, fi, err) } + sort.Strings(names) for _, name := range names { stats, err = s.scan(ctx, stats, filepath.Join(target, name)) diff --git a/internal/archiver/tree.go b/internal/archiver/tree.go index 0c8a21539..04c0a8e33 100644 --- a/internal/archiver/tree.go +++ b/internal/archiver/tree.go @@ -214,7 +214,7 @@ func unrollTree(f fs.FS, t *Tree) error { // nodes, add the contents of Path to the nodes. if t.Path != "" && len(t.Nodes) > 0 { debug.Log("resolve path %v", t.Path) - entries, err := fs.ReadDirNames(f, t.Path) + entries, err := readdirnames(f, t.Path, 0) if err != nil { return err } diff --git a/internal/fs/fs_helpers.go b/internal/fs/fs_helpers.go deleted file mode 100644 index 6b269f763..000000000 --- a/internal/fs/fs_helpers.go +++ /dev/null @@ -1,45 +0,0 @@ -package fs - -import "os" - -// ReadDir reads the directory named by dirname within fs and returns a list of -// directory entries. -func ReadDir(fs FS, dirname string) ([]os.FileInfo, error) { - f, err := fs.Open(dirname) - if err != nil { - return nil, err - } - - entries, err := f.Readdir(-1) - if err != nil { - return nil, err - } - - err = f.Close() - if err != nil { - return nil, err - } - - return entries, nil -} - -// ReadDirNames reads the directory named by dirname within fs and returns a -// list of entry names. -func ReadDirNames(fs FS, dirname string) ([]string, error) { - f, err := fs.Open(dirname) - if err != nil { - return nil, err - } - - entries, err := f.Readdirnames(-1) - if err != nil { - return nil, err - } - - err = f.Close() - if err != nil { - return nil, err - } - - return entries, nil -}