Skip to content

Commit

Permalink
Switch most calls to filepath.Walk to filepath.WalkDir
Browse files Browse the repository at this point in the history
It is faster then Walk, when you don't need to stat every
file and directory.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
  • Loading branch information
rhatdan committed Mar 28, 2022
1 parent d48cfc6 commit 5c036e5
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 58 deletions.
6 changes: 3 additions & 3 deletions .cirrus.yml
Expand Up @@ -117,7 +117,7 @@ lint_task:
env:
CIRRUS_WORKING_DIR: "/go/src/github.com/containers/storage"
container:
image: golang:1.15
image: golang:1.16
modules_cache:
fingerprint_script: cat go.sum
folder: $GOPATH/pkg/mod
Expand Down Expand Up @@ -154,7 +154,7 @@ meta_task:

vendor_task:
container:
image: golang:1.15
image: golang:1.16
modules_cache:
fingerprint_script: cat go.sum
folder: $GOPATH/pkg/mod
Expand All @@ -172,6 +172,6 @@ success_task:
- meta
- vendor
container:
image: golang:1.15
image: golang:1.16
clone_script: 'mkdir -p "$CIRRUS_WORKING_DIR"' # Source code not needed
script: /bin/true
6 changes: 4 additions & 2 deletions drivers/aufs/aufs.go
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

/*
Expand Down Expand Up @@ -26,6 +27,7 @@ import (
"bufio"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -649,11 +651,11 @@ func (a *Driver) mounted(mountpoint string) (bool, error) {
// Cleanup aufs and unmount all mountpoints
func (a *Driver) Cleanup() error {
var dirs []string
if err := filepath.Walk(a.mntPath(), func(path string, info os.FileInfo, err error) error {
if err := filepath.WalkDir(a.mntPath(), func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
if !d.IsDir() {
return nil
}
dirs = append(dirs, path)
Expand Down
10 changes: 6 additions & 4 deletions drivers/btrfs/btrfs.go
@@ -1,3 +1,4 @@
//go:build linux && cgo
// +build linux,cgo

package btrfs
Expand All @@ -16,6 +17,7 @@ import "C"

import (
"fmt"
"io/fs"
"io/ioutil"
"math"
"os"
Expand Down Expand Up @@ -256,7 +258,7 @@ func subvolDelete(dirpath, name string, quotaEnabled bool) error {
var args C.struct_btrfs_ioctl_vol_args

// walk the btrfs subvolumes
walkSubvolumes := func(p string, f os.FileInfo, err error) error {
walkSubvolumes := func(p string, d fs.DirEntry, err error) error {
if err != nil {
if os.IsNotExist(err) && p != fullPath {
// missing most likely because the path was a subvolume that got removed in the previous iteration
Expand All @@ -267,20 +269,20 @@ func subvolDelete(dirpath, name string, quotaEnabled bool) error {
}
// we want to check children only so skip itself
// it will be removed after the filepath walk anyways
if f.IsDir() && p != fullPath {
if d.IsDir() && p != fullPath {
sv, err := isSubvolume(p)
if err != nil {
return fmt.Errorf("Failed to test if %s is a btrfs subvolume: %v", p, err)
}
if sv {
if err := subvolDelete(path.Dir(p), f.Name(), quotaEnabled); err != nil {
if err := subvolDelete(path.Dir(p), d.Name(), quotaEnabled); err != nil {
return fmt.Errorf("Failed to destroy btrfs child subvolume (%s) of parent (%s): %v", p, dirpath, err)
}
}
}
return nil
}
if err := filepath.Walk(path.Join(dirpath, name), walkSubvolumes); err != nil {
if err := filepath.WalkDir(path.Join(dirpath, name), walkSubvolumes); err != nil {
return fmt.Errorf("Recursively walking subvolumes for %s failed: %v", dirpath, err)
}

Expand Down
1 change: 1 addition & 0 deletions drivers/copy/copy_test.go
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

package copy
Expand Down
29 changes: 13 additions & 16 deletions drivers/devmapper/deviceset.go
@@ -1,3 +1,4 @@
//go:build linux && cgo
// +build linux,cgo

package devmapper
Expand All @@ -6,6 +7,7 @@ import (
"bufio"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -419,40 +421,35 @@ func (devices *DeviceSet) constructDeviceIDMap() {
}
}

func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) error {
func (devices *DeviceSet) deviceFileWalkFunction(path string, name string) error {

// Skip some of the meta files which are not device files.
if strings.HasSuffix(finfo.Name(), ".migrated") {
if strings.HasSuffix(name, ".migrated") {
logrus.Debugf("devmapper: Skipping file %s", path)
return nil
}

if strings.HasPrefix(finfo.Name(), ".") {
if strings.HasPrefix(name, ".") {
logrus.Debugf("devmapper: Skipping file %s", path)
return nil
}

if finfo.Name() == deviceSetMetaFile {
if name == deviceSetMetaFile {
logrus.Debugf("devmapper: Skipping file %s", path)
return nil
}

if finfo.Name() == transactionMetaFile {
if name == transactionMetaFile {
logrus.Debugf("devmapper: Skipping file %s", path)
return nil
}

logrus.Debugf("devmapper: Loading data for file %s", path)

hash := finfo.Name()
if hash == base {
hash = ""
}

// Include deleted devices also as cleanup delete device logic
// will go through it and see if there are any deleted devices.
if _, err := devices.lookupDevice(hash); err != nil {
return fmt.Errorf("devmapper: Error looking up device %s:%v", hash, err)
if _, err := devices.lookupDevice(name); err != nil {
return fmt.Errorf("devmapper: Error looking up device %s:%v", name, err)
}

return nil
Expand All @@ -462,21 +459,21 @@ func (devices *DeviceSet) loadDeviceFilesOnStart() error {
logrus.Debug("devmapper: loadDeviceFilesOnStart()")
defer logrus.Debug("devmapper: loadDeviceFilesOnStart() END")

var scan = func(path string, info os.FileInfo, err error) error {
var scan = func(path string, d fs.DirEntry, err error) error {
if err != nil {
logrus.Debugf("devmapper: Can't walk the file %s", path)
return nil
}

// Skip any directories
if info.IsDir() {
if d.IsDir() {
return nil
}

return devices.deviceFileWalkFunction(path, info)
return devices.deviceFileWalkFunction(path, d.Name())
}

return filepath.Walk(devices.metadataDir(), scan)
return filepath.WalkDir(devices.metadataDir(), scan)
}

// Should be called with devices.Lock() held.
Expand Down
6 changes: 4 additions & 2 deletions drivers/overlay/check_115.go
@@ -1,8 +1,10 @@
//go:build !go1.16
// +build !go1.16

package overlay

import (
"io/fs"
"os"
"path/filepath"
"strings"
Expand All @@ -12,7 +14,7 @@ import (
)

func scanForMountProgramIndicators(home string) (detected bool, err error) {
err = filepath.Walk(home, func(path string, info os.FileInfo, err error) error {
err = filepath.WalkDir(home, func(path string, d fs.DirEntry, err error) error {
if detected {
return filepath.SkipDir
}
Expand All @@ -24,7 +26,7 @@ func scanForMountProgramIndicators(home string) (detected bool, err error) {
detected = true
return filepath.SkipDir
}
if info.IsDir() {
if d.IsDir() {
xattrs, err := system.Llistxattr(path)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion go.mod
@@ -1,4 +1,4 @@
go 1.14
go 1.16

module github.com/containers/storage

Expand Down
7 changes: 4 additions & 3 deletions pkg/archive/archive.go
Expand Up @@ -7,6 +7,7 @@ import (
"compress/bzip2"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -863,14 +864,14 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
rebaseName := options.RebaseNames[include]

walkRoot := getWalkRoot(srcPath, include)
filepath.Walk(walkRoot, func(filePath string, f os.FileInfo, err error) error {
filepath.WalkDir(walkRoot, func(filePath string, d fs.DirEntry, err error) error {
if err != nil {
logrus.Errorf("Tar: Can't stat file %s to tar: %s", srcPath, err)
return nil
}

relFilePath, err := filepath.Rel(srcPath, filePath)
if err != nil || (!options.IncludeSourceDir && relFilePath == "." && f.IsDir()) {
if err != nil || (!options.IncludeSourceDir && relFilePath == "." && d.IsDir()) {
// Error getting relative path OR we are looking
// at the source directory path. Skip in both situations.
return nil
Expand Down Expand Up @@ -903,7 +904,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
// dir. If so then we can't skip this dir.

// Its not a dir then so we can just return/skip.
if !f.IsDir() {
if !d.IsDir() {
return nil
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/archive/changes_other.go
@@ -1,9 +1,11 @@
//go:build !linux
// +build !linux

package archive

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -41,7 +43,7 @@ func collectFileInfoForChanges(oldDir, newDir string, oldIDMap, newIDMap *idtool
func collectFileInfo(sourceDir string, idMappings *idtools.IDMappings) (*FileInfo, error) {
root := newRootFileInfo(idMappings)

err := filepath.Walk(sourceDir, func(path string, f os.FileInfo, err error) error {
err := filepath.WalkDir(sourceDir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/archive/copy_unix_test.go
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

// TODO Windows: Some of these tests may be salvageable and portable to Windows.
Expand All @@ -10,6 +11,7 @@ import (
"encoding/hex"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -102,24 +104,22 @@ func dirContentsEqual(t *testing.T, newDir, oldDir string) (err error) {
}

func logDirContents(t *testing.T, dirPath string) {
logWalkedPaths := filepath.WalkFunc(func(path string, info os.FileInfo, err error) error {
t.Logf("logging directory contents: %q", dirPath)

err := filepath.WalkDir(dirPath, func(path string, d fs.DirEntry, err error) error {
if err != nil {
t.Errorf("stat error for path %q: %s", path, err)
return nil
}

if info.IsDir() {
if d.IsDir() {
path = joinTrailingSep(path)
}

t.Logf("\t%s", path)

return nil
})

t.Logf("logging directory contents: %q", dirPath)

err := filepath.Walk(dirPath, logWalkedPaths)
require.NoError(t, err)
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/archive/diff.go
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -134,7 +135,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64,
if err != nil {
return 0, err
}
err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
if os.IsNotExist(err) {
err = nil // parent was deleted
Expand Down
5 changes: 3 additions & 2 deletions pkg/archive/diff_test.go
Expand Up @@ -3,6 +3,7 @@ package archive
import (
"archive/tar"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -362,7 +363,7 @@ func makeTestLayer(paths []string) (rc io.ReadCloser, err error) {

func readDirContents(root string) ([]string, error) {
var files []string
err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
Expand All @@ -373,7 +374,7 @@ func readDirContents(root string) ([]string, error) {
if err != nil {
return err
}
if info.IsDir() {
if d.IsDir() {
rel = rel + "/"
}
files = append(files, rel)
Expand Down
5 changes: 3 additions & 2 deletions pkg/archive/utils_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -140,8 +141,8 @@ func testBreakout(untarFn string, tmpdir string, headers []*tar.Header) error {
// Since victim/hello was generated with time.Now(), it is safe to assume
// that any file whose content matches exactly victim/hello, managed somehow
// to access victim/hello.
return filepath.Walk(dest, func(path string, info os.FileInfo, err error) error {
if info.IsDir() {
return filepath.WalkDir(dest, func(path string, d fs.DirEntry, err error) error {
if d.IsDir() {
if err != nil {
// skip directory if error
return filepath.SkipDir
Expand Down

0 comments on commit 5c036e5

Please sign in to comment.