Repository: tetratelabs/wazero
Pull request #1149 (2023-02-21 07:28:48)

adds platform.Readdirnames and uses in gojs
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.


Patch:
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/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/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


Test patch:
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_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_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()
 	})
 }