From d5c321e29aacf4df83d2a036d0faed22af4ebcad Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Tue, 21 Feb 2023 15:56:22 +0800 Subject: [PATCH] adds platform.Readdirnames and uses in gojs (#1149) This adds `platform.Readdirnames` which is preparation work before doing something similar for reading the directory. We use this in gojs as it doesn't actually need dirents, rather just names. Signed-off-by: Adrian Cole --- imports/wasi_snapshot_preview1/fs.go | 18 +++---- internal/gojs/fs.go | 34 ++++++------ internal/platform/dir.go | 35 ++++++++++++ internal/platform/dir_test.go | 79 ++++++++++++++++++++++++++++ internal/platform/stat.go | 6 +-- internal/platform/stat_test.go | 12 ++--- internal/sysfs/rootfs_test.go | 8 +-- internal/sysfs/sysfs.go | 33 +++++++----- internal/sysfs/sysfs_test.go | 2 +- 9 files changed, 167 insertions(+), 60 deletions(-) create mode 100644 internal/platform/dir.go create mode 100644 internal/platform/dir_test.go diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index 4d3c575a..0e045768 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -21,10 +21,10 @@ import ( // The following interfaces are used until we finalize our own FD-scoped file. type ( - // syncer is implemented by os.File in file_posix.go - syncer interface{ Sync() error } - // truncater is implemented by os.File in file_posix.go - truncater interface{ Truncate(size int64) error } + // syncFile is implemented by os.File in file_posix.go + syncFile interface{ Sync() error } + // truncateFile is implemented by os.File in file_posix.go + truncateFile interface{ Truncate(size int64) error } ) // fdAdvise is the WASI function named FdAdviseName which provides file @@ -106,7 +106,7 @@ func fdAllocateFn(_ context.Context, mod api.Module, params []uint64) Errno { return ErrnoSuccess } - osf, ok := f.File.(truncater) + osf, ok := f.File.(truncateFile) if !ok { return ErrnoBadf } @@ -405,9 +405,9 @@ func fdFilestatSetSizeFn(_ context.Context, mod api.Module, params []uint64) Err // Check to see if the file descriptor is available if f, ok := fsc.LookupFile(fd); !ok { return ErrnoBadf - } else if truncater, ok := f.File.(truncater); !ok { + } else if truncateFile, ok := f.File.(truncateFile); !ok { return ErrnoBadf // possibly a fake file - } else if err := truncater.Truncate(int64(size)); err != nil { + } else if err := truncateFile.Truncate(int64(size)); err != nil { return ToErrno(err) } return ErrnoSuccess @@ -1158,9 +1158,9 @@ func fdSyncFn(_ context.Context, mod api.Module, params []uint64) Errno { // Check to see if the file descriptor is available if f, ok := fsc.LookupFile(fd); !ok { return ErrnoBadf - } else if syncer, ok := f.File.(syncer); !ok { + } else if syncFile, ok := f.File.(syncFile); !ok { return ErrnoBadf // possibly a fake file - } else if err := syncer.Sync(); err != nil { + } else if err := syncFile.Sync(); err != nil { return ToErrno(err) } return ErrnoSuccess diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go index d691b324..a1c7d1e3 100644 --- a/internal/gojs/fs.go +++ b/internal/gojs/fs.go @@ -84,12 +84,12 @@ var ( // The following interfaces are used until we finalize our own FD-scoped file. type ( - // chmoder is implemented by os.File in file_posix.go - chmoder interface{ Chmod(fs.FileMode) error } - // syncer is implemented by os.File in file_posix.go - syncer interface{ Sync() error } - // truncater is implemented by os.File in file_posix.go - truncater interface{ Truncate(size int64) error } + // chmodFile is implemented by os.File in file_posix.go + chmodFile interface{ Chmod(fs.FileMode) error } + // syncFile is implemented by os.File in file_posix.go + syncFile interface{ Sync() error } + // truncateFile is implemented by os.File in file_posix.go + truncateFile interface{ Truncate(size int64) error } ) // jsfsOpen implements implements jsFn for syscall.Open @@ -374,14 +374,12 @@ func syscallReaddir(_ context.Context, mod api.Module, name string) (*objectArra } defer f.Close() //nolint - if d, ok := f.(fs.ReadDirFile); !ok { - return nil, syscall.ENOTDIR - } else if l, err := d.ReadDir(-1); err != nil { + if names, err := platform.Readdirnames(f, -1); err != nil { return nil, err } else { - entries := make([]interface{}, 0, len(l)) - for _, e := range l { - entries = append(entries, e.Name()) + entries := make([]interface{}, 0, len(names)) + for _, e := range names { + entries = append(entries, e) } return &objectArray{entries}, nil } @@ -547,10 +545,10 @@ func (jsfsFchmod) invoke(ctx context.Context, mod api.Module, args ...interface{ var err error if f, ok := fsc.LookupFile(fd); !ok { err = syscall.EBADF - } else if chmoder, ok := f.File.(chmoder); !ok { + } else if chmodFile, ok := f.File.(chmodFile); !ok { err = syscall.EBADF // possibly a fake file } else { - err = chmoder.Chmod(fs.FileMode(mode)) + err = chmodFile.Chmod(fs.FileMode(mode)) } return jsfsInvoke(ctx, mod, callback, err) @@ -638,10 +636,10 @@ func (jsfsFtruncate) invoke(ctx context.Context, mod api.Module, args ...interfa var err error if f, ok := fsc.LookupFile(fd); !ok { err = syscall.EBADF - } else if truncater, ok := f.File.(truncater); !ok { + } else if truncateFile, ok := f.File.(truncateFile); !ok { err = syscall.EBADF // possibly a fake file } else { - err = truncater.Truncate(length) + err = truncateFile.Truncate(length) } return jsfsInvoke(ctx, mod, callback, err) @@ -709,10 +707,10 @@ func (jsfsFsync) invoke(ctx context.Context, mod api.Module, args ...interface{} var err error if f, ok := fsc.LookupFile(fd); !ok { err = syscall.EBADF - } else if syncer, ok := f.File.(syncer); !ok { + } else if syncFile, ok := f.File.(syncFile); !ok { err = syscall.EBADF // possibly a fake file } else { - err = syncer.Sync() + err = syncFile.Sync() } return jsfsInvoke(ctx, mod, callback, err) diff --git a/internal/platform/dir.go b/internal/platform/dir.go new file mode 100644 index 00000000..98a58ede --- /dev/null +++ b/internal/platform/dir.go @@ -0,0 +1,35 @@ +package platform + +import ( + "io/fs" + "syscall" +) + +// readdirnamesFile is implemented by os.File in dir.go +// Note: we use this until we finalize our own FD-scoped file. +type readdirnamesFile interface { + Readdirnames(n int) (names []string, err error) +} + +// Readdirnames is like the function on os.File, but for fs.File. This returns +// syscall.ENOTDIR if not a directory or syscall.EIO if closed or read +// redundantly. +func Readdirnames(f fs.File, n int) (names []string, err error) { + switch f := f.(type) { + case readdirnamesFile: + names, err = f.Readdirnames(n) + case fs.ReadDirFile: + var entries []fs.DirEntry + entries, err = f.ReadDir(n) + if err == nil { + names = make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + } + default: + err = syscall.ENOTDIR + } + err = UnwrapOSError(err) + return +} diff --git a/internal/platform/dir_test.go b/internal/platform/dir_test.go new file mode 100644 index 00000000..25cb614e --- /dev/null +++ b/internal/platform/dir_test.go @@ -0,0 +1,79 @@ +package platform_test + +import ( + "io/fs" + "os" + "runtime" + "sort" + "syscall" + "testing" + + "github.com/tetratelabs/wazero/internal/fstest" + "github.com/tetratelabs/wazero/internal/platform" + "github.com/tetratelabs/wazero/internal/testing/require" +) + +func TestReaddirnames(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + require.NoError(t, fstest.WriteTestFiles(tmpDir)) + dirFS := os.DirFS(tmpDir) + + tests := []struct { + name string + fs fs.FS + }{ + {name: "os.DirFS", fs: dirFS}, // To test readdirnamesFile + {name: "fstest.MapFS", fs: fstest.FS}, // To test adaptation of ReadDirFile + } + + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + dirF, err := tc.fs.Open(".") + require.NoError(t, err) + defer dirF.Close() + + t.Run("dir", func(t *testing.T) { + names, err := platform.Readdirnames(dirF, -1) + require.NoError(t, err) + sort.Strings(names) + require.Equal(t, []string{"animals.txt", "dir", "empty.txt", "emptydir", "sub"}, names) + + // read again even though it is exhausted + _, err = platform.Readdirnames(dirF, 100) + require.EqualErrno(t, syscall.EIO, err) + }) + + // windows and fstest.MapFS allow you to read a closed dir + if runtime.GOOS != "windows" && tc.name != "fstest.MapFS" { + t.Run("closed dir", func(t *testing.T) { + require.NoError(t, dirF.Close()) + _, err := platform.Readdirnames(dirF, -1) + require.EqualErrno(t, syscall.EIO, err) + }) + } + + fileF, err := tc.fs.Open("empty.txt") + require.NoError(t, err) + defer fileF.Close() + + t.Run("file", func(t *testing.T) { + _, err := platform.Readdirnames(fileF, -1) + require.EqualErrno(t, syscall.ENOTDIR, err) + }) + + subdirF, err := tc.fs.Open("sub") + require.NoError(t, err) + defer subdirF.Close() + + t.Run("subdir", func(t *testing.T) { + names, err := platform.Readdirnames(subdirF, -1) + require.NoError(t, err) + require.Equal(t, []string{"test.txt"}, names) + }) + }) + } +} diff --git a/internal/platform/stat.go b/internal/platform/stat.go index 528d6bb7..9f4d79c5 100644 --- a/internal/platform/stat.go +++ b/internal/platform/stat.go @@ -62,12 +62,12 @@ func StatFile(f fs.File, st *Stat_t) (err error) { return fillStatFile(st, f, t) } -// fder is implemented by os.File in file_unix.go and file_windows.go +// fdFile is implemented by os.File in file_unix.go and file_windows.go // Note: we use this until we finalize our own FD-scoped file. -type fder interface{ Fd() (fd uintptr) } +type fdFile interface{ Fd() (fd uintptr) } func fillStatFile(stat *Stat_t, f fs.File, t fs.FileInfo) (err error) { - if of, ok := f.(fder); !ok { // possibly fake filesystem + if of, ok := f.(fdFile); !ok { // possibly fake filesystem fillStatFromFileInfo(stat, t) } else { err = fillStatFromOpenFile(stat, of.Fd(), t) diff --git a/internal/platform/stat_test.go b/internal/platform/stat_test.go index 79340d25..d1ea72e8 100644 --- a/internal/platform/stat_test.go +++ b/internal/platform/stat_test.go @@ -47,9 +47,7 @@ func TestStatFile(t *testing.T) { var stat Stat_t tmpDirF, err := OpenFile(tmpDir, syscall.O_RDONLY, 0) - if err != nil { - return - } + require.NoError(t, err) defer tmpDirF.Close() t.Run("dir", func(t *testing.T) { @@ -68,9 +66,7 @@ func TestStatFile(t *testing.T) { file := path.Join(tmpDir, "file") require.NoError(t, os.WriteFile(file, nil, 0o400)) fileF, err := OpenFile(file, syscall.O_RDONLY, 0) - if err != nil { - return - } + require.NoError(t, err) defer fileF.Close() t.Run("file", func(t *testing.T) { @@ -87,9 +83,7 @@ func TestStatFile(t *testing.T) { subdir := path.Join(tmpDir, "sub") require.NoError(t, os.Mkdir(subdir, 0o500)) subdirF, err := OpenFile(subdir, syscall.O_RDONLY, 0) - if err != nil { - return - } + require.NoError(t, err) defer subdirF.Close() t.Run("subdir", func(t *testing.T) { diff --git a/internal/sysfs/rootfs_test.go b/internal/sysfs/rootfs_test.go index bb42f5a6..34d2c24f 100644 --- a/internal/sysfs/rootfs_test.go +++ b/internal/sysfs/rootfs_test.go @@ -13,6 +13,7 @@ import ( gofstest "testing/fstest" "github.com/tetratelabs/wazero/internal/fstest" + "github.com/tetratelabs/wazero/internal/platform" testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" ) @@ -114,13 +115,8 @@ func TestNewRootFS(t *testing.T) { } func readDirNames(t *testing.T, f fs.File) []string { - entries, err := f.(fs.ReadDirFile).ReadDir(-1) + names, err := platform.Readdirnames(f, -1) require.NoError(t, err) - - names := make([]string, 0, len(entries)) - for _, e := range entries { - names = append(names, e.Name()) - } sort.Strings(names) return names } diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go index e099b615..4baf5b8e 100644 --- a/internal/sysfs/sysfs.go +++ b/internal/sysfs/sysfs.go @@ -233,6 +233,8 @@ type FS interface { // readFile declares all read interfaces defined on os.File used by wazero. type readFile interface { + fdFile // for the number of links. + readdirnamesFile fs.ReadDirFile io.ReaderAt // for pread io.Seeker // fallback for ReaderAt for embed:fs @@ -243,22 +245,25 @@ type file interface { readFile io.Writer io.WriterAt // for pwrite - chmoder - syncer - truncater - fder // for the number of links. + chmodFile + syncFile + truncateFile } // The following interfaces are used until we finalize our own FD-scoped file. type ( - // chmoder is implemented by os.File in file_posix.go - chmoder interface{ Chmod(fs.FileMode) error } - // syncer is implemented by os.File in file_posix.go - syncer interface{ Sync() error } - // truncater is implemented by os.File in file_posix.go - truncater interface{ Truncate(size int64) error } - // fder is implemented by os.File in file_unix.go and file_windows.go - fder interface{ Fd() (fd uintptr) } + // chmodFile is implemented by os.File in file_posix.go + chmodFile interface{ Chmod(fs.FileMode) error } + // fdFile is implemented by os.File in file_unix.go and file_windows.go + fdFile interface{ Fd() (fd uintptr) } + // syncFile is implemented by os.File in file_posix.go + syncFile interface{ Sync() error } + // readdirnamesFile is implemented by os.File in dir.go + readdirnamesFile interface { + Readdirnames(n int) (names []string, err error) + } + // truncateFile is implemented by os.File in file_posix.go + truncateFile interface{ Truncate(size int64) error } ) // ReaderAtOffset gets an io.Reader from a fs.File that reads from an offset, @@ -280,14 +285,14 @@ func ReaderAtOffset(f fs.File, offset int64) io.Reader { // FileDatasync is like syscall.Fdatasync except that's only defined in linux. func FileDatasync(f fs.File) (err error) { - if fd, ok := f.(fder); ok { + if fd, ok := f.(fdFile); ok { if err := platform.Fdatasync(fd.Fd()); err != syscall.ENOSYS { return err } } // Attempt to sync everything, even if we only need to sync the data. - if s, ok := f.(syncer); ok { + if s, ok := f.(syncFile); ok { err = s.Sync() } return diff --git a/internal/sysfs/sysfs_test.go b/internal/sysfs/sysfs_test.go index 1b59ed25..cf4cdd0a 100644 --- a/internal/sysfs/sysfs_test.go +++ b/internal/sysfs/sysfs_test.go @@ -560,7 +560,7 @@ func TestWriterAtOffset_Unsupported(t *testing.T) { // to below. Effectively, this only tests that things don't error. func Test_FileSync(t *testing.T) { testSync(t, func(f fs.File) error { - return f.(syncer).Sync() + return f.(syncFile).Sync() }) }