ufs: improve error handling

Signed-off-by: Matthew Penner <me@matthewp.io>

internal/ufs/error.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	iofs "io/fs"
 	"os"
+	"syscall"
 
 	"golang.org/x/sys/unix"
 )
@@ -48,9 +49,9 @@ type PathError = iofs.PathError
 // SyscallError records an error from a specific system call.
 type SyscallError = os.SyscallError
 
-// NewSyscallError returns, as an error, a new SyscallError
-// with the given system call name and error details.
-// As a convenience, if err is nil, NewSyscallError returns nil.
+// NewSyscallError returns, as an error, a new [*os.SyscallError] with the
+// given system call name and error details. As a convenience, if err is nil,
+// [NewSyscallError] returns nil.
 func NewSyscallError(syscall string, err error) error {
 	return os.NewSyscallError(syscall, err)
 }
@@ -61,62 +62,122 @@ func convertErrorType(err error) error {
 	if err == nil {
 		return nil
 	}
+
 	var pErr *PathError
-	switch {
-	case errors.As(err, &pErr):
-		switch {
-		// File exists
-		case errors.Is(pErr.Err, unix.EEXIST):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrExist,
-			}
-		// Is a directory
-		case errors.Is(pErr.Err, unix.EISDIR):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrIsDirectory,
-			}
-		// Not a directory
-		case errors.Is(pErr.Err, unix.ENOTDIR):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrNotDirectory,
-			}
-		// No such file or directory
-		case errors.Is(pErr.Err, unix.ENOENT):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrNotExist,
-			}
-		// Operation not permitted
-		case errors.Is(pErr.Err, unix.EPERM):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrPermission,
-			}
-		// Invalid cross-device link
-		case errors.Is(pErr.Err, unix.EXDEV):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrBadPathResolution,
-			}
-		// Too many levels of symbolic links
-		case errors.Is(pErr.Err, unix.ELOOP):
-			return &PathError{
-				Op:   pErr.Op,
-				Path: pErr.Path,
-				Err:  ErrBadPathResolution,
-			}
-			// TODO: EROFS
-			// TODO: ENOTEMPTY
+	if errors.As(err, &pErr) {
+		if errno, ok := pErr.Err.(syscall.Errno); ok {
+			return errnoToPathError(errno, pErr.Op, pErr.Path)
+		}
+		return pErr
+	}
+
+	// If the error wasn't already a path error and is a errno, wrap it with
+	// details that we can use to know there is something wrong with our
+	// error wrapping somewhere.
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		return &PathError{
+			Op:   "!(UNKNOWN)",
+			Path: "!(UNKNOWN)",
+			Err:  err,
 		}
 	}
+
 	return err
 }
+
+// ensurePathError ensures that err is a PathError. The op and path arguments
+// are only used of the error isn't already a PathError.
+func ensurePathError(err error, op, path string) error {
+	if err == nil {
+		return nil
+	}
+
+	// Check if the error is already a PathError.
+	var pErr *PathError
+	if errors.As(err, &pErr) {
+		// If underlying error is a errno, convert it.
+		//
+		// DO NOT USE `errors.As` or whatever here, the error will either be
+		// an errno, or it will be wrapped already.
+		if errno, ok := pErr.Err.(syscall.Errno); ok {
+			return errnoToPathError(errno, pErr.Op, pErr.Path)
+		}
+		// Return the PathError as-is without modification.
+		return pErr
+	}
+
+	// If the error is directly an errno, convert it to a PathError.
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		return errnoToPathError(errno, op, path)
+	}
+
+	// Otherwise just wrap it as a PathError without any additional changes.
+	return &PathError{
+		Op:   op,
+		Path: path,
+		Err:  err,
+	}
+}
+
+// errnoToPathError converts an errno into a proper path error.
+func errnoToPathError(err syscall.Errno, op, path string) error {
+	switch err {
+	// File exists
+	case unix.EEXIST:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrExist,
+		}
+	// Is a directory
+	case unix.EISDIR:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrIsDirectory,
+		}
+	// Not a directory
+	case unix.ENOTDIR:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrNotDirectory,
+		}
+	// No such file or directory
+	case unix.ENOENT:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrNotExist,
+		}
+	// Operation not permitted
+	case unix.EPERM:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrPermission,
+		}
+	// Invalid cross-device link
+	case unix.EXDEV:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrBadPathResolution,
+		}
+	// Too many levels of symbolic links
+	case unix.ELOOP:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  ErrBadPathResolution,
+		}
+	default:
+		return &PathError{
+			Op:   op,
+			Path: path,
+			Err:  err,
+		}
+	}
+}

internal/ufs/fs_quota.go

@@ -7,8 +7,12 @@ import (
 	"sync/atomic"
 )
 
-// Quota .
-// TODO: document
+// Quota is a wrapper around [*UnixFS] that provides the ability to limit the
+// disk usage of the filesystem.
+//
+// NOTE: this is not a full complete quota filesystem, it provides utilities for
+// tracking and checking the usage of the filesystem. The only operation that is
+// automatically accounted against the quota are file deletions.
 type Quota struct {
 	// fs is the underlying filesystem that runs the actual I/O operations.
 	*UnixFS
@@ -28,8 +32,7 @@ type Quota struct {
 	usage atomic.Int64
 }
 
-// NewQuota .
-// TODO: document
+// NewQuota creates a new Quota filesystem using an existing UnixFS and a limit.
 func NewQuota(fs *UnixFS, limit int64) *Quota {
 	qfs := Quota{UnixFS: fs}
 	qfs.limit.Store(limit)
@@ -105,6 +108,9 @@ func (fs *Quota) CanFit(size int64) bool {
 	return false
 }
 
+// Remove removes the named file or (empty) directory.
+//
+// If there is an error, it will be of type [*PathError].
 func (fs *Quota) Remove(name string) error {
 	// For information on why this interface is used here, check its
 	// documentation.
@@ -129,7 +135,7 @@ func (fs *Quota) Remove(name string) 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.
+// If there is an error, it will be of type [*PathError].
 func (fs *Quota) RemoveAll(name string) error {
 	name, err := fs.unsafePath(name)
 	if err != nil {

internal/ufs/fs_unix.go

@@ -81,7 +81,16 @@ func (fs *UnixFS) Chmod(name string, mode FileMode) error {
 	if err != nil {
 		return err
 	}
-	return convertErrorType(unix.Fchmodat(dirfd, name, uint32(mode), 0))
+	return fs.fchmodat("chmod", dirfd, name, mode)
+}
+
+// Chmodat is like Chmod but it takes a dirfd and name instead of a full path.
+func (fs *UnixFS) Chmodat(dirfd int, name string, mode FileMode) error {
+	return fs.fchmodat("chmodat", dirfd, name, mode)
+}
+
+func (fs *UnixFS) fchmodat(op string, dirfd int, name string, mode FileMode) error {
+	return ensurePathError(unix.Fchmodat(dirfd, name, uint32(mode), 0), op, name)
 }
 
 // Chown changes the numeric uid and gid of the named file.
@@ -93,7 +102,7 @@ func (fs *UnixFS) Chmod(name string, mode FileMode) error {
 // On Windows or Plan 9, Chown always returns the syscall.EWINDOWS or
 // EPLAN9 error, wrapped in *PathError.
 func (fs *UnixFS) Chown(name string, uid, gid int) error {
-	return fs.fchown(name, uid, gid, 0)
+	return ensurePathError(fs.fchown(name, uid, gid, 0), "chown", name)
 }
 
 // Lchown changes the numeric uid and gid of the named file.
@@ -106,7 +115,7 @@ func (fs *UnixFS) Chown(name string, uid, gid int) error {
 func (fs *UnixFS) Lchown(name string, uid, gid int) error {
 	// With AT_SYMLINK_NOFOLLOW, Fchownat acts like Lchown but allows us to
 	// pass a dirfd.
-	return fs.fchown(name, uid, gid, AT_SYMLINK_NOFOLLOW)
+	return ensurePathError(fs.fchown(name, uid, gid, AT_SYMLINK_NOFOLLOW), "lchown", name)
 }
 
 // fchown is a re-usable Fchownat syscall used by Chown and Lchown.
@@ -116,19 +125,19 @@ func (fs *UnixFS) fchown(name string, uid, gid, flags int) error {
 	if err != nil {
 		return err
 	}
-	return convertErrorType(unix.Fchownat(dirfd, name, uid, gid, flags))
+	return unix.Fchownat(dirfd, name, uid, gid, flags)
 }
 
 // Chownat is like Chown but allows passing an existing directory file
 // descriptor rather than needing to resolve one.
 func (fs *UnixFS) Chownat(dirfd int, name string, uid, gid int) error {
-	return convertErrorType(unix.Fchownat(dirfd, name, uid, gid, 0))
+	return ensurePathError(unix.Fchownat(dirfd, name, uid, gid, 0), "chownat", name)
 }
 
 // Lchownat is like Lchown but allows passing an existing directory file
 // descriptor rather than needing to resolve one.
 func (fs *UnixFS) Lchownat(dirfd int, name string, uid, gid int) error {
-	return convertErrorType(unix.Fchownat(dirfd, name, uid, gid, AT_SYMLINK_NOFOLLOW))
+	return ensurePathError(unix.Fchownat(dirfd, name, uid, gid, AT_SYMLINK_NOFOLLOW), "lchownat", name)
 }
 
 // Chtimes changes the access and modification times of the named
@@ -160,11 +169,9 @@ func (fs *UnixFS) Chtimesat(dirfd int, name string, atime, mtime time.Time) erro
 	}
 	set(0, atime)
 	set(1, mtime)
+
 	// This does support `AT_SYMLINK_NOFOLLOW` as well if needed.
-	if err := unix.UtimesNanoAt(dirfd, name, utimes[0:], 0); err != nil {
-		return convertErrorType(&PathError{Op: "chtimes", Path: name, Err: err})
-	}
-	return nil
+	return ensurePathError(unix.UtimesNanoAt(dirfd, name, utimes[0:], 0), "chtimes", name)
 }
 
 // Create creates or truncates the named file. If the file already exists,
@@ -188,11 +195,15 @@ func (fs *UnixFS) Mkdir(name string, mode FileMode) error {
 	if err != nil {
 		return err
 	}
-	return fs.Mkdirat(dirfd, name, mode)
+	return fs.mkdirat("mkdir", dirfd, name, mode)
 }
 
 func (fs *UnixFS) Mkdirat(dirfd int, name string, mode FileMode) error {
-	return convertErrorType(unix.Mkdirat(dirfd, name, uint32(mode)))
+	return fs.mkdirat("mkdirat", dirfd, name, mode)
+}
+
+func (fs *UnixFS) mkdirat(op string, dirfd int, name string, mode FileMode) error {
+	return ensurePathError(unix.Mkdirat(dirfd, name, uint32(mode)), op, name)
 }
 
 // MkdirAll creates a directory named path, along with any necessary
@@ -313,7 +324,7 @@ func (fs *UnixFS) RemoveStat(name string) (FileInfo, error) {
 		err = fs.unlinkat(dirfd, name, 0)
 	}
 	if err != nil {
-		return s, convertErrorType(&PathError{Op: "remove", Path: name, Err: err})
+		return s, ensurePathError(err, "rename", name)
 	}
 	return s, nil
 }
@@ -362,7 +373,7 @@ func (fs *UnixFS) Remove(name string) error {
 	if err1 != unix.ENOTDIR {
 		err = err1
 	}
-	return convertErrorType(&PathError{Op: "remove", Path: name, Err: err})
+	return ensurePathError(err, "remove", name)
 }
 
 // RemoveAll removes path and any children it contains.
@@ -377,6 +388,7 @@ func (fs *UnixFS) RemoveAll(name string) error {
 	if err != nil {
 		return err
 	}
+
 	// While removeAll internally checks this, I want to make sure we check it
 	// and return the proper error so our tests can ensure that this will never
 	// be a possibility.
@@ -387,9 +399,29 @@ func (fs *UnixFS) RemoveAll(name string) error {
 			Err:  ErrBadPathResolution,
 		}
 	}
+
 	return fs.removeAll(name)
 }
 
+// RemoveContents recursively removes the contents of name.
+//
+// It removes everything it can but returns the first error
+// it encounters. If the path does not exist, RemoveContents
+// returns nil (no error).
+//
+// If there is an error, it will be of type [*PathError].
+func (fs *UnixFS) RemoveContents(name string) error {
+	name, err := fs.unsafePath(name)
+	if err != nil {
+		return err
+	}
+
+	// Unlike RemoveAll, we don't remove `name` itself, only it's contents.
+	// So there is no need to check for a name of `.` here.
+
+	return fs.removeContents(name)
+}
+
 func (fs *UnixFS) unlinkat(dirfd int, name string, flags int) error {
 	return ignoringEINTR(func() error {
 		return unix.Unlinkat(dirfd, name, flags)
@@ -418,11 +450,11 @@ func (fs *UnixFS) Rename(oldpath, newpath string) error {
 	// While unix.Renameat ends up throwing a "device or resource busy" error,
 	// that doesn't mean we are protecting the system properly.
 	if oldname == "." {
-		return convertErrorType(&PathError{
+		return &PathError{
 			Op:   "rename",
 			Path: oldname,
 			Err:  ErrBadPathResolution,
-		})
+		}
 	}
 	// Stat the old target to return proper errors.
 	if _, err := fs.Lstatat(olddirfd, oldname); err != nil {
@@ -433,11 +465,11 @@ func (fs *UnixFS) Rename(oldpath, newpath string) error {
 	if err != nil {
 		closeFd2()
 		if !errors.Is(err, ErrNotExist) {
-			return convertErrorType(err)
+			return err
 		}
 		var pathErr *PathError
 		if !errors.As(err, &pathErr) {
-			return convertErrorType(err)
+			return err
 		}
 		if err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
 			return err
@@ -455,25 +487,28 @@ func (fs *UnixFS) Rename(oldpath, newpath string) error {
 	// While unix.Renameat ends up throwing a "device or resource busy" error,
 	// that doesn't mean we are protecting the system properly.
 	if newname == "." {
-		return convertErrorType(&PathError{
+		return &PathError{
 			Op:   "rename",
 			Path: newname,
 			Err:  ErrBadPathResolution,
-		})
+		}
 	}
 	// Stat the new target to return proper errors.
 	_, err = fs.Lstatat(newdirfd, newname)
 	switch {
 	case err == nil:
-		return convertErrorType(&PathError{
+		return &PathError{
 			Op:   "rename",
 			Path: newname,
 			Err:  ErrExist,
-		})
+		}
 	case !errors.Is(err, ErrNotExist):
 		return err
 	}
-	return unix.Renameat(olddirfd, oldname, newdirfd, newname)
+	if err := unix.Renameat(olddirfd, oldname, newdirfd, newname); err != nil {
+		return &LinkError{Op: "rename", Old: oldpath, New: newpath, Err: err}
+	}
+	return nil
 }
 
 // Stat returns a FileInfo describing the named file.
@@ -527,7 +562,7 @@ func (fs *UnixFS) _fstatat(op string, dirfd int, name string, flags int) (FileIn
 	if err := ignoringEINTR(func() error {
 		return unix.Fstatat(dirfd, name, &s.sys, flags)
 	}); err != nil {
-		return nil, &PathError{Op: op, Path: name, Err: err}
+		return nil, ensurePathError(err, op, name)
 	}
 	fillFileStatFromSys(&s, name)
 	return &s, nil
@@ -563,23 +598,42 @@ func (fs *UnixFS) Touch(path string, flag int, mode FileMode) (File, error) {
 	if flag&O_CREATE == 0 {
 		flag |= O_CREATE
 	}
-	dirfd, name, closeFd, err := fs.safePath(path)
+	dirfd, name, closeFd, err, _ := fs.TouchPath(path)
 	defer closeFd()
-	if err == nil {
-		return fs.OpenFileat(dirfd, name, flag, mode)
-	}
-	if !errors.Is(err, ErrNotExist) {
+	if err != nil {
 		return nil, err
 	}
+	return fs.OpenFileat(dirfd, name, flag, mode)
+}
+
+// TouchPath is like SafePath except that it will create any missing directories
+// in the path. Unlike SafePath, TouchPath returns an additional boolean which
+// indicates whether the parent directories already existed, this is intended to
+// be used as a way to know if the final destination could already exist.
+func (fs *UnixFS) TouchPath(path string) (int, string, func(), error, bool) {
+	dirfd, name, closeFd, err := fs.safePath(path)
+	switch {
+	case err == nil:
+		return dirfd, name, closeFd, nil, true
+	case !errors.Is(err, ErrNotExist):
+		return dirfd, name, closeFd, err, false
+	}
+
 	var pathErr *PathError
 	if !errors.As(err, &pathErr) {
-		return nil, err
+		return dirfd, name, closeFd, err, false
 	}
 	if err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
-		return nil, err
+		return dirfd, name, closeFd, err, false
 	}
-	// Try to open the file one more time after creating its parent directories.
-	return fs.OpenFile(path, flag, mode)
+
+	// Close the previous file descriptor since we are going to be opening
+	// a new one.
+	closeFd()
+
+	// Run safe path again now that the parent directories have been created.
+	dirfd, name, closeFd, err = fs.safePath(path)
+	return dirfd, name, closeFd, err, false
 }
 
 // WalkDir walks the file tree rooted at root, calling fn for each file or
@@ -621,7 +675,7 @@ func (fs *UnixFS) openat(dirfd int, name string, flag int, mode FileMode) (int,
 		if err == unix.EINTR {
 			continue
 		}
-		return -1, convertErrorType(err)
+		return 0, err
 	}
 
 	// If we are using openat2, we don't need the additional security checks.
@@ -646,24 +700,29 @@ func (fs *UnixFS) openat(dirfd int, name string, flag int, mode FileMode) (int,
 		if !errors.As(err, &pErr) {
 			return fd, fmt.Errorf("failed to evaluate symlink: %w", convertErrorType(err))
 		}
+
+		// Update the final path to whatever directory or path didn't exist while
+		// recursing any symlinks.
 		finalPath = pErr.Path
+		// Ensure the error is wrapped correctly.
+		err = convertErrorType(err)
 	}
 
 	// Check if the path is within our root.
 	if !fs.unsafeIsPathInsideOfBase(finalPath) {
-		return fd, convertErrorType(&PathError{
-			Op:   "openat",
+		op := "openat"
+		if fs.useOpenat2 {
+			op = "openat2"
+		}
+		return fd, &PathError{
+			Op:   op,
 			Path: name,
 			Err:  ErrBadPathResolution,
-		})
+		}
 	}
 
-	// This handles any hanging errors.
-	if err != nil {
-		return fd, convertErrorType(err)
-	}
-
-	return fd, nil
+	// Return the file descriptor and any potential error.
+	return fd, err
 }
 
 // _openat is a wrapper around unix.Openat. This method should never be directly
@@ -681,11 +740,11 @@ func (fs *UnixFS) _openat(dirfd int, name string, flag int, mode uint32) (int, e
 	case err == nil:
 		return fd, nil
 	case err == unix.EINTR:
-		return -1, err
+		return fd, err
 	case err == unix.EAGAIN:
-		return -1, err
+		return fd, err
 	default:
-		return -1, &PathError{Op: "openat", Path: name, Err: err}
+		return fd, ensurePathError(err, "openat", name)
 	}
 }
 
@@ -720,11 +779,11 @@ func (fs *UnixFS) _openat2(dirfd int, name string, flag, mode uint64) (int, erro
 	case err == nil:
 		return fd, nil
 	case err == unix.EINTR:
-		return -1, err
+		return fd, err
 	case err == unix.EAGAIN:
-		return -1, err
+		return fd, err
 	default:
-		return -1, &PathError{Op: "openat2", Path: name, Err: err}
+		return fd, ensurePathError(err, "openat2", name)
 	}
 }
 
@@ -746,7 +805,7 @@ func (fs *UnixFS) safePath(path string) (dirfd int, file string, closeFd func(),
 	// Open the base path. We use this as the sandbox root for any further
 	// operations.
 	var fsDirfd int
-	fsDirfd, err = unix.Openat(AT_EMPTY_PATH, fs.basePath, O_DIRECTORY|O_RDONLY, 0)
+	fsDirfd, err = fs._openat(AT_EMPTY_PATH, fs.basePath, O_DIRECTORY|O_RDONLY, 0)
 	if err != nil {
 		return
 	}
@@ -772,7 +831,7 @@ func (fs *UnixFS) safePath(path string) (dirfd int, file string, closeFd func(),
 		// An error occurred while opening the directory, but we already opened
 		// the filesystem root, so we still need to ensure it gets closed.
 		closeFd = func() { _ = unix.Close(fsDirfd) }
-	} else {	
+	} else {
 		// Set closeFd to close the newly opened directory file descriptor.
 		closeFd = func() {
 			_ = unix.Close(dirfd)
@@ -780,22 +839,24 @@ func (fs *UnixFS) safePath(path string) (dirfd int, file string, closeFd func(),
 		}
 	}
 
-
 	// Return dirfd, name, the closeFd func, and err
 	return
 }
 
-// unsafePath prefixes the given path and prefixes it with the filesystem's
-// base path, cleaning the result. The path returned by this function may not
-// be inside the filesystem's base path, additional checks are required to
-// safely use paths returned by this function.
+// unsafePath strips and joins the given path with the filesystem's base path,
+// cleaning the result. The cleaned path is then checked if it starts with the
+// filesystem's base path to obvious any obvious path traversal escapes. The
+// fully resolved path (if symlinks are followed) may not be within the
+// filesystem's base path, additional checks are required to safely use paths
+// returned by this function.
 func (fs *UnixFS) unsafePath(path string) (string, error) {
-	// Calling filepath.Clean on the joined directory will resolve it to the
-	// absolute path, removing any ../ type of resolution arguments, and leaving
-	// us with a direct path link.
+	// Calling filepath.Clean on the path will resolve it to it's absolute path,
+	// removing any path traversal arguments (such as ..), leaving us with an
+	// absolute path we can then use.
 	//
-	// This will also trim the existing root path off the beginning of the path
-	// passed to the function since that can get a bit messy.
+	// This will also trim the filesystem's base path from the given path and
+	// join the base path back on to ensure the path starts with the base path
+	// without appending it twice.
 	r := filepath.Clean(filepath.Join(fs.basePath, strings.TrimPrefix(path, fs.basePath)))
 
 	if fs.unsafeIsPathInsideOfBase(r) {
@@ -822,6 +883,10 @@ func (fs *UnixFS) unsafePath(path string) (string, error) {
 
 // unsafeIsPathInsideOfBase checks if the given path is inside the filesystem's
 // base path.
+//
+// NOTE: this method doesn't clean the given path or attempt to join the
+// filesystem's base path. This is purely a basic prefix check against the
+// given path.
 func (fs *UnixFS) unsafeIsPathInsideOfBase(path string) bool {
 	return strings.HasPrefix(
 		strings.TrimSuffix(path, "/")+"/",

internal/ufs/mkdir_unix.go

@@ -10,10 +10,6 @@
 
 package ufs
 
-import (
-	"golang.org/x/sys/unix"
-)
-
 // mkdirAll is a recursive Mkdir implementation that properly handles symlinks.
 func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
 	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
@@ -30,7 +26,7 @@ func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
 		if dir.IsDir() {
 			return nil
 		}
-		return convertErrorType(&PathError{Op: "mkdir", Path: name, Err: unix.ENOTDIR})
+		return &PathError{Op: "mkdir", Path: name, Err: ErrNotDirectory}
 	}
 
 	// Slow path: make sure parent exists and then call Mkdir for path.

internal/ufs/removeall_unix.go

@@ -52,60 +52,69 @@ func removeAll(fs unixFS, path string) error {
 	parentDir, base := splitPath(path)
 
 	parent, err := fs.Open(parentDir)
-	if errors.Is(err, ErrNotExist) {
+	if err != nil {
+		if !errors.Is(err, ErrNotExist) {
+			return err
+		}
 		// If parent does not exist, base cannot exist. Fail silently
 		return nil
 	}
-	if err != nil {
-		return err
-	}
 	defer parent.Close()
 
 	if err := removeAllFrom(fs, parent, base); err != nil {
 		if pathErr, ok := err.(*PathError); ok {
 			pathErr.Path = parentDir + string(os.PathSeparator) + pathErr.Path
-			err = pathErr
+			err = convertErrorType(pathErr)
+		} else {
+			err = ensurePathError(err, "removeallfrom", base)
 		}
-		return convertErrorType(err)
+		return err
 	}
 	return nil
 }
 
-func removeAllFrom(fs unixFS, parent File, base string) error {
-	parentFd := int(parent.Fd())
-	// Simple case: if Unlink (aka remove) works, we're done.
-	err := fs.unlinkat(parentFd, base, 0)
-	if err == nil || errors.Is(err, ErrNotExist) {
+func (fs *UnixFS) removeContents(path string) error {
+	return removeContents(fs, path)
+}
+
+func removeContents(fs unixFS, path string) error {
+	if path == "" {
+		// fail silently to retain compatibility with previous behavior
+		// of RemoveAll. See issue https://go.dev/issue/28830.
 		return nil
 	}
 
-	// EISDIR means that we have a directory, and we need to
-	// remove its contents.
-	// EPERM or EACCES means that we don't have write permission on
-	// the parent directory, but this entry might still be a directory
-	// whose contents need to be removed.
-	// Otherwise, just return the error.
-	if err != unix.EISDIR && err != unix.EPERM && err != unix.EACCES {
-		return &PathError{Op: "unlinkat", Path: base, Err: err}
-	}
+	// RemoveAll recurses by deleting the path base from
+	// its parent directory
+	parentDir, base := splitPath(path)
 
-	// Is this a directory we need to recurse into?
-	var statInfo unix.Stat_t
-	statErr := ignoringEINTR(func() error {
-		return unix.Fstatat(parentFd, base, &statInfo, AT_SYMLINK_NOFOLLOW)
-	})
-	if statErr != nil {
-		if errors.Is(statErr, ErrNotExist) {
-			return nil
+	parent, err := fs.Open(parentDir)
+	if err != nil {
+		if !errors.Is(err, ErrNotExist) {
+			return err
 		}
-		return &PathError{Op: "fstatat", Path: base, Err: statErr}
-	}
-	if statInfo.Mode&unix.S_IFMT != unix.S_IFDIR {
-		// Not a directory; return the error from the unix.Unlinkat.
-		return &PathError{Op: "unlinkat", Path: base, Err: err}
+		// If parent does not exist, base cannot exist. Fail silently
+		return nil
 	}
+	defer parent.Close()
+
+	if err := removeContentsFrom(fs, parent, base); err != nil {
+		if pathErr, ok := err.(*PathError); ok {
+			pathErr.Path = parentDir + string(os.PathSeparator) + pathErr.Path
+			err = convertErrorType(pathErr)
+		} else {
+			err = ensurePathError(err, "removecontentsfrom", base)
+		}
+		return err
+	}
+	return nil
+}
+
+// removeContentsFrom recursively removes all descendants of parent without
+// removing parent itself. Parent must be a directory.
+func removeContentsFrom(fs unixFS, parent File, base string) error {
+	parentFd := int(parent.Fd())
 
-	// Remove the directory's entries.
 	var recurseErr error
 	for {
 		const reqSize = 1024
@@ -168,6 +177,50 @@ func removeAllFrom(fs unixFS, parent File, base string) error {
 		}
 	}
 
+	return nil
+}
+
+func removeAllFrom(fs unixFS, parent File, base string) error {
+	parentFd := int(parent.Fd())
+
+	// Simple case: if Unlink (aka remove) works, we're done.
+	err := fs.unlinkat(parentFd, base, 0)
+	if err == nil || errors.Is(err, ErrNotExist) {
+		return nil
+	}
+
+	// EISDIR means that we have a directory, and we need to
+	// remove its contents.
+	// EPERM or EACCES means that we don't have write permission on
+	// the parent directory, but this entry might still be a directory
+	// whose contents need to be removed.
+	// Otherwise, just return the error.
+	if err != unix.EISDIR && err != unix.EPERM && err != unix.EACCES {
+		return &PathError{Op: "unlinkat", Path: base, Err: err}
+	}
+
+	// Is this a directory we need to recurse into?
+	var statInfo unix.Stat_t
+	statErr := ignoringEINTR(func() error {
+		return unix.Fstatat(parentFd, base, &statInfo, AT_SYMLINK_NOFOLLOW)
+	})
+	if statErr != nil {
+		if errors.Is(statErr, ErrNotExist) {
+			return nil
+		}
+		return &PathError{Op: "fstatat", Path: base, Err: statErr}
+	}
+	if statInfo.Mode&unix.S_IFMT != unix.S_IFDIR {
+		// Not a directory; return the error from the unix.Unlinkat.
+		return &PathError{Op: "unlinkat", Path: base, Err: err}
+	}
+
+	// Remove all contents will remove the contents of the directory.
+	//
+	// It was split out of this function to allow the deletion of the
+	// contents of a directory, without deleting the directory itself.
+	recurseErr := removeContentsFrom(fs, parent, base)
+
 	// Remove the directory itself.
 	unlinkErr := fs.unlinkat(parentFd, base, AT_REMOVEDIR)
 	if unlinkErr == nil || errors.Is(unlinkErr, ErrNotExist) {
@@ -177,7 +230,8 @@ func removeAllFrom(fs unixFS, parent File, base string) error {
 	if recurseErr != nil {
 		return recurseErr
 	}
-	return &PathError{Op: "unlinkat", Path: base, Err: unlinkErr}
+
+	return ensurePathError(err, "unlinkat", base)
 }
 
 // openFdAt opens path relative to the directory in fd.

internal/ufs/walk_unix.go

@@ -199,7 +199,7 @@ func (fs *UnixFS) modeTypeFromDirent(de *unix.Dirent, fd int, name string) (File
 func (fs *UnixFS) modeType(dirfd int, name string) (FileMode, error) {
 	fi, err := fs.Lstatat(dirfd, name)
 	if err != nil {
-		return 0, fmt.Errorf("ufs: error finding mode type for %s during readDir: %w", name, convertErrorType(err))
+		return 0, fmt.Errorf("ufs: error finding mode type for %s during readDir: %w", name, err)
 	}
 	return fi.Mode() & ModeType, nil
 }
@@ -227,7 +227,7 @@ func (fs *UnixFS) readDir(fd int, name, relative string, b []byte) ([]DirEntry,
 				if err == unix.EINTR {
 					continue
 				}
-				return nil, fmt.Errorf("ufs: error with getdents during readDir: %w", convertErrorType(err))
+				return nil, ensurePathError(err, "getdents", name)
 			}
 			if n <= 0 {
 				// end of directory: normal exit