diff options
author | Baokun Lee <nototon@gmail.com> | 2019-03-01 18:04:14 +0800 |
---|---|---|
committer | Brad Fitzpatrick <bradfitz@golang.org> | 2019-03-15 16:32:03 +0000 |
commit | ff048033e4304898245d843e79ed1a0897006c6d (patch) | |
tree | d02ecd7c23a44233389fc5ac72f597d67afc0539 | |
parent | c1d8d9d8be8d78e87ddcfb938c6b6f74e305882f (diff) | |
download | go-ff048033e4304898245d843e79ed1a0897006c6d.tar.gz go-ff048033e4304898245d843e79ed1a0897006c6d.zip |
[release-branch.go1.12] os: consistently return PathError from RemoveAll
Fixes #30859
Updates #30491
Change-Id: If4070e5d39d8649643d7e90f6f3eb499642e25ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/164720
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
(cherry picked from commit d039e12b54a73796caa913994597a9f3a73e8e87)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167739
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Baokun Lee <nototon@gmail.com>
-rw-r--r-- | src/os/path.go | 1 | ||||
-rw-r--r-- | src/os/path_unix.go | 2 | ||||
-rw-r--r-- | src/os/removeall_at.go | 38 | ||||
-rw-r--r-- | src/os/removeall_test.go | 15 |
4 files changed, 39 insertions, 17 deletions
diff --git a/src/os/path.go b/src/os/path.go index 104b7ceaf7..ba43ea3525 100644 --- a/src/os/path.go +++ b/src/os/path.go @@ -62,6 +62,7 @@ func MkdirAll(path string, perm FileMode) error { // It removes everything it can but returns the first error // it encounters. If the path does not exist, RemoveAll // returns nil (no error). +// If there is an error, it will be of type *PathError. func RemoveAll(path string) error { return removeAll(path) } diff --git a/src/os/path_unix.go b/src/os/path_unix.go index be373a50a9..a08ddaf6db 100644 --- a/src/os/path_unix.go +++ b/src/os/path_unix.go @@ -51,7 +51,7 @@ func splitPath(path string) (string, string) { // Remove leading directory path for i--; i >= 0; i-- { if path[i] == '/' { - dirname = path[:i+1] + dirname = path[:i] basename = path[i+1:] break } diff --git a/src/os/removeall_at.go b/src/os/removeall_at.go index 94232cf556..330963b354 100644 --- a/src/os/removeall_at.go +++ b/src/os/removeall_at.go @@ -46,13 +46,20 @@ func removeAll(path string) error { } defer parent.Close() - return removeAllFrom(parent, base) + if err := removeAllFrom(parent, base); err != nil { + if pathErr, ok := err.(*PathError); ok { + pathErr.Path = parentDir + string(PathSeparator) + pathErr.Path + err = pathErr + } + return err + } + return nil } -func removeAllFrom(parent *File, path string) error { +func removeAllFrom(parent *File, base string) error { parentFd := int(parent.Fd()) // Simple case: if Unlink (aka remove) works, we're done. - err := unix.Unlinkat(parentFd, path, 0) + err := unix.Unlinkat(parentFd, base, 0) if err == nil || IsNotExist(err) { return nil } @@ -64,21 +71,21 @@ func removeAllFrom(parent *File, path string) error { // whose contents need to be removed. // Otherwise just return the error. if err != syscall.EISDIR && err != syscall.EPERM && err != syscall.EACCES { - return err + return &PathError{"unlinkat", base, err} } // Is this a directory we need to recurse into? var statInfo syscall.Stat_t - statErr := unix.Fstatat(parentFd, path, &statInfo, unix.AT_SYMLINK_NOFOLLOW) + statErr := unix.Fstatat(parentFd, base, &statInfo, unix.AT_SYMLINK_NOFOLLOW) if statErr != nil { if IsNotExist(statErr) { return nil } - return statErr + return &PathError{"fstatat", base, statErr} } if statInfo.Mode&syscall.S_IFMT != syscall.S_IFDIR { - // Not a directory; return the error from the Remove. - return err + // Not a directory; return the error from the unix.Unlinkat. + return &PathError{"unlinkat", base, err} } // Remove the directory's entries. @@ -87,12 +94,12 @@ func removeAllFrom(parent *File, path string) error { const request = 1024 // Open the directory to recurse into - file, err := openFdAt(parentFd, path) + file, err := openFdAt(parentFd, base) if err != nil { if IsNotExist(err) { return nil } - recurseErr = err + recurseErr = &PathError{"openfdat", base, err} break } @@ -103,12 +110,15 @@ func removeAllFrom(parent *File, path string) error { if IsNotExist(readErr) { return nil } - return readErr + return &PathError{"readdirnames", base, readErr} } for _, name := range names { err := removeAllFrom(file, name) if err != nil { + if pathErr, ok := err.(*PathError); ok { + pathErr.Path = base + string(PathSeparator) + pathErr.Path + } recurseErr = err } } @@ -127,7 +137,7 @@ func removeAllFrom(parent *File, path string) error { } // Remove the directory itself. - unlinkError := unix.Unlinkat(parentFd, path, unix.AT_REMOVEDIR) + unlinkError := unix.Unlinkat(parentFd, base, unix.AT_REMOVEDIR) if unlinkError == nil || IsNotExist(unlinkError) { return nil } @@ -135,7 +145,7 @@ func removeAllFrom(parent *File, path string) error { if recurseErr != nil { return recurseErr } - return unlinkError + return &PathError{"unlinkat", base, unlinkError} } // openFdAt opens path relative to the directory in fd. @@ -157,7 +167,7 @@ func openFdAt(dirfd int, name string) (*File, error) { continue } - return nil, &PathError{"openat", name, e} + return nil, e } if !supportsCloseOnExec { diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go index 21371d8776..945a38e8e0 100644 --- a/src/os/removeall_test.go +++ b/src/os/removeall_test.go @@ -294,7 +294,7 @@ func TestRemoveReadOnlyDir(t *testing.T) { } // Issue #29983. -func TestRemoveAllButReadOnly(t *testing.T) { +func TestRemoveAllButReadOnlyAndPathError(t *testing.T) { switch runtime.GOOS { case "nacl", "js", "windows": t.Skipf("skipping test on %s", runtime.GOOS) @@ -355,10 +355,21 @@ func TestRemoveAllButReadOnly(t *testing.T) { defer Chmod(d, 0777) } - if err := RemoveAll(tempDir); err == nil { + err = RemoveAll(tempDir) + if err == nil { t.Fatal("RemoveAll succeeded unexpectedly") } + // The error should be of type *PathError. + // see issue 30491 for details. + if pathErr, ok := err.(*PathError); ok { + if g, w := pathErr.Path, filepath.Join(tempDir, "b", "y"); g != w { + t.Errorf("got %q, expected pathErr.path %q", g, w) + } + } else { + t.Errorf("got %T, expected *os.PathError", err) + } + for _, dir := range dirs { _, err := Stat(filepath.Join(tempDir, dir)) if inReadonly(dir) { |