From f8a6585538d2d6e6788a01e9539928fc732400a0 Mon Sep 17 00:00:00 2001 From: "Kirill A. Korinsky" Date: Sat, 8 Jun 2024 11:00:32 +0100 Subject: [PATCH] Avoid using syscall for errors on unix syscall is deprecated and non updated library whcih should be replaced by new one with name x/sys. Anyway, it may work on some platform and some errors migth be missed on some. A good example is OpenBSD which has lack of syscall.EBADMSG on amd64 and 386 but has on arm64. Here I moved all errors to two files: one for unix which uses sys.unix, and one for everything else which uses syscall as fallback. --- copy/copy_darwin.go | 3 ++- copy/copy_linux.go | 3 ++- copy/mkdir.go | 5 +++-- diskwriter.go | 5 ++--- errors_nounix.go | 15 +++++++++++++++ errors_unix.go | 21 +++++++++++++++++++++ filter.go | 7 +++---- followlinks.go | 5 ++--- fs.go | 13 ++++++------- hardlinks.go | 5 ++--- receive.go | 3 +-- send.go | 3 +-- stat_unix.go | 2 +- tarwriter.go | 3 +-- validator.go | 7 +++---- 15 files changed, 65 insertions(+), 35 deletions(-) create mode 100644 errors_nounix.go create mode 100644 errors_unix.go diff --git a/copy/copy_darwin.go b/copy/copy_darwin.go index 0cdc00a8..a297044b 100644 --- a/copy/copy_darwin.go +++ b/copy/copy_darwin.go @@ -8,12 +8,13 @@ import ( "os" "github.com/pkg/errors" + "github.com/tonistiigi/fsutil" "golang.org/x/sys/unix" ) func copyFile(source, target string) error { if err := unix.Clonefileat(unix.AT_FDCWD, source, unix.AT_FDCWD, target, unix.CLONE_NOFOLLOW); err != nil { - if err != unix.EINVAL && err != unix.EXDEV { + if err != fsutil.EINVAL && err != fsutil.EXDEV { return err } } else { diff --git a/copy/copy_linux.go b/copy/copy_linux.go index 9b046c53..b30b2f50 100644 --- a/copy/copy_linux.go +++ b/copy/copy_linux.go @@ -7,6 +7,7 @@ import ( "syscall" "github.com/pkg/errors" + "github.com/tonistiigi/fsutil" "golang.org/x/sys/unix" ) @@ -105,7 +106,7 @@ func copyFileContent(dst, src *os.File) error { n, err := unix.CopyFileRange(int(src.Fd()), nil, int(dst.Fd()), nil, desired, 0) if err != nil { // matches go/src/internal/poll/copy_file_range_linux.go - if (err != unix.ENOSYS && err != unix.EXDEV && err != unix.EPERM && err != syscall.EIO && err != unix.EOPNOTSUPP && err != syscall.EINVAL) || !first { + if (err != fsutil.ENOSYS && err != fsutil.EXDEV && err != fsutil.EPERM && err != fsutil.EIO && err != fsutil.EOPNOTSUPP && err != fsutil.EINVAL) || !first { return errors.Wrap(err, "copy file range failed") } diff --git a/copy/mkdir.go b/copy/mkdir.go index b19c2c61..84a4de35 100644 --- a/copy/mkdir.go +++ b/copy/mkdir.go @@ -2,8 +2,9 @@ package fs import ( "os" - "syscall" "time" + + "github.com/tonistiigi/fsutil" ) // MkdirAll is forked os.MkdirAll @@ -14,7 +15,7 @@ func MkdirAll(path string, perm os.FileMode, user Chowner, tm *time.Time) ([]str if dir.IsDir() { return nil, nil } - return nil, &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR} + return nil, &os.PathError{Op: "mkdir", Path: path, Err: fsutil.ENOTDIR} } // Slow path: make sure parent exists and then call Mkdir for path. diff --git a/diskwriter.go b/diskwriter.go index 83389262..e0e0be58 100644 --- a/diskwriter.go +++ b/diskwriter.go @@ -9,7 +9,6 @@ import ( "path/filepath" "strconv" "sync" - "syscall" "time" "github.com/opencontainers/go-digest" @@ -123,7 +122,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "change without stat info"}) + return errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "change without stat info"}) } statCopy := stat.Clone() @@ -164,7 +163,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er switch { case fi.IsDir(): if err := os.Mkdir(newPath, fi.Mode()); err != nil { - if errors.Is(err, syscall.EEXIST) { + if errors.Is(err, EEXIST) { // we saw a race to create this directory, so try again return dw.HandleChange(kind, p, fi, nil) } diff --git a/errors_nounix.go b/errors_nounix.go new file mode 100644 index 00000000..8b16176a --- /dev/null +++ b/errors_nounix.go @@ -0,0 +1,15 @@ +//go:build !unix +// +build !unix + +package fsutil + +import "syscall" + +const ( + EBADMSG = syscall.EBADMSG + EEXIST = syscall.EEXIST + EINVAL = syscall.EINVAL + EISDIR = syscall.EISDIR + ENOENT = syscall.ENOENT + ENOTDIR = syscall.ENOTDIR +) diff --git a/errors_unix.go b/errors_unix.go new file mode 100644 index 00000000..922fd1fb --- /dev/null +++ b/errors_unix.go @@ -0,0 +1,21 @@ +//go:build unix +// +build unix + +package fsutil + +import "golang.org/x/sys/unix" + +const ( + EBADMSG = unix.EBADMSG + EEXIST = unix.EEXIST + EINVAL = unix.EINVAL + EIO = unix.EIO + EISDIR = unix.EISDIR + ENOENT = unix.ENOENT + ENOSYS = unix.ENOSYS + ENOTDIR = unix.ENOTDIR + ENOTSUP = unix.ENOTSUP + EOPNOTSUPP = unix.EOPNOTSUPP + EPERM = unix.EPERM + EXDEV = unix.EXDEV +) diff --git a/filter.go b/filter.go index 9cee8ad2..10a4a7ad 100644 --- a/filter.go +++ b/filter.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "github.com/moby/patternmatcher" "github.com/pkg/errors" @@ -318,7 +317,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc } stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"}) } select { @@ -346,7 +345,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc } parentStat, ok := parentFi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"}) } select { @@ -425,5 +424,5 @@ func patternWithoutTrailingGlob(p *patternmatcher.Pattern) string { } func isNotExist(err error) bool { - return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) + return errors.Is(err, os.ErrNotExist) || errors.Is(err, ENOTDIR) } diff --git a/followlinks.go b/followlinks.go index 559291d7..478e8f86 100644 --- a/followlinks.go +++ b/followlinks.go @@ -8,7 +8,6 @@ import ( "runtime" "sort" strings "strings" - "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -122,7 +121,7 @@ func (r *symlinkResolver) readSymlink(p string, allowWildcard bool) ([]string, e } stat, ok := fi.Sys().(*types.Stat) if !ok { - return nil, errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return nil, errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "fileinfo without stat info"}) } link := filepath.Clean(stat.Linkname) @@ -180,7 +179,7 @@ func readDir(fs FS, root string) ([]os.DirEntry, error) { } if p == root { if !entry.IsDir() { - return errors.WithStack(&os.PathError{Op: "walk", Path: root, Err: syscall.ENOTDIR}) + return errors.WithStack(&os.PathError{Op: "walk", Path: root, Err: ENOTDIR}) } out = make([]gofs.DirEntry, 0) return nil diff --git a/fs.go b/fs.go index d990ea5a..3ada140a 100644 --- a/fs.go +++ b/fs.go @@ -9,7 +9,6 @@ import ( "path/filepath" "sort" "strings" - "syscall" "time" "github.com/pkg/errors" @@ -32,7 +31,7 @@ func NewFS(root string) (FS, error) { return nil, err } if !fi.IsDir() { - return nil, errors.WithStack(&os.PathError{Op: "stat", Path: root, Err: syscall.ENOTDIR}) + return nil, errors.WithStack(&os.PathError{Op: "stat", Path: root, Err: ENOTDIR}) } return &fs{ @@ -102,10 +101,10 @@ func SubDirFS(dirs []Dir) (FS, error) { m := map[string]Dir{} for _, d := range dirs { if path.Base(d.Stat.Path) != d.Stat.Path { - return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EISDIR, Op: "invalid path"}) + return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EISDIR, Op: "invalid path"}) } if _, ok := m[d.Stat.Path]; ok { - return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EEXIST, Op: "duplicate path"}) + return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EEXIST, Op: "duplicate path"}) } m[d.Stat.Path] = d } @@ -127,7 +126,7 @@ func (fs *subDirFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc fi := &StatInfo{d.Stat.Clone()} if !fi.IsDir() { - return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.ENOTDIR, Op: "walk subdir"}) + return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: ENOTDIR, Op: "walk subdir"}) } dStat := d.Stat.Clone() if err := fn(d.Stat.Path, &DirEntryInfo{Stat: dStat}, nil); err != nil { @@ -144,7 +143,7 @@ func (fs *subDirFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc } stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EBADMSG, Op: "fileinfo without stat info"}) } stat.Path = path.Join(d.Stat.Path, stat.Path) if stat.Linkname != "" { @@ -171,7 +170,7 @@ func (fs *subDirFS) Open(p string) (io.ReadCloser, error) { } d, ok := fs.m[parts[0]] if !ok { - return nil, errors.WithStack(&os.PathError{Path: parts[0], Err: syscall.ENOENT, Op: "open"}) + return nil, errors.WithStack(&os.PathError{Path: parts[0], Err: ENOENT, Op: "open"}) } return d.FS.Open(parts[1]) } diff --git a/hardlinks.go b/hardlinks.go index d9bf2fc1..8460d862 100644 --- a/hardlinks.go +++ b/hardlinks.go @@ -5,7 +5,6 @@ import ( "io" gofs "io/fs" "os" - "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -32,7 +31,7 @@ func (v *Hardlinks) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "change without stat info"}) + return errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "change without stat info"}) } if fi.IsDir() || fi.Mode()&os.ModeSymlink != 0 { @@ -80,7 +79,7 @@ func (r *hardlinkFilter) Walk(ctx context.Context, target string, fn gofs.WalkDi stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"}) } if stat.Linkname != "" { diff --git a/receive.go b/receive.go index 2f94778a..a9f65e4c 100644 --- a/receive.go +++ b/receive.go @@ -36,7 +36,6 @@ import ( "os" "path/filepath" "sync" - "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -221,7 +220,7 @@ func (r *receiver) run(ctx context.Context) error { path := filepath.FromSlash(p.Stat.Path) if filepath.ToSlash(path) != p.Stat.Path { // e.g. a linux path foo/bar\baz cannot be represented on windows - return errors.WithStack(&os.PathError{Path: p.Stat.Path, Err: syscall.EINVAL, Op: "unrepresentable path"}) + return errors.WithStack(&os.PathError{Path: p.Stat.Path, Err: EINVAL, Op: "unrepresentable path"}) } p.Stat.Path = path p.Stat.Linkname = filepath.FromSlash(p.Stat.Linkname) diff --git a/send.go b/send.go index e4a31563..cc33f722 100644 --- a/send.go +++ b/send.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "sync" - "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -158,7 +157,7 @@ func (s *sender) walk(ctx context.Context) error { } stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"}) } stat.Path = filepath.ToSlash(stat.Path) stat.Linkname = filepath.ToSlash(stat.Linkname) diff --git a/stat_unix.go b/stat_unix.go index def901e3..78ccf8a8 100644 --- a/stat_unix.go +++ b/stat_unix.go @@ -15,7 +15,7 @@ import ( func loadXattr(origpath string, stat *types.Stat) error { xattrs, err := sysx.LListxattr(origpath) if err != nil { - if errors.Is(err, syscall.ENOTSUP) { + if errors.Is(err, ENOTSUP) { return nil } return errors.Wrapf(err, "failed to xattr %s", origpath) diff --git a/tarwriter.go b/tarwriter.go index 06b7bda9..a5a289ac 100644 --- a/tarwriter.go +++ b/tarwriter.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -26,7 +25,7 @@ func WriteTar(ctx context.Context, fs FS, w io.Writer) error { } stat, ok := fi.Sys().(*types.Stat) if !ok { - return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"}) } hdr, err := tar.FileInfoHeader(fi, stat.Linkname) if err != nil { diff --git a/validator.go b/validator.go index 8b0dd91e..52502f0c 100644 --- a/validator.go +++ b/validator.go @@ -5,7 +5,6 @@ import ( "path/filepath" "sort" "strings" - "syscall" "github.com/pkg/errors" ) @@ -28,10 +27,10 @@ func (v *Validator) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err v.parentDirs = make([]parent, 1, 10) } if p != filepath.Clean(p) { - return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "unclean path"}) + return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "unclean path"}) } if filepath.IsAbs(p) { - return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "absolute path"}) + return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "absolute path"}) } dir := filepath.Dir(p) base := filepath.Base(p) @@ -39,7 +38,7 @@ func (v *Validator) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err dir = "" } if dir == ".." || strings.HasPrefix(p, filepath.FromSlash("../")) { - return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "escape check"}) + return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "escape check"}) } // find a parent dir from saved records