syscall: store skip count in file descriptor offset

Multiple calls to ReadDirent expect to return subsequent
portions of the directory listing. There's no place to store
our progress other than the file descriptor offset.

Fortunately, the file descriptor offset doesn't need to be
a real offset. We can store any int64 we want there.

Fixes #31368

Change-Id: I49e4e0e7ff707d3e96aa5d43e3b0199531013cde
Reviewed-on: https://go-review.googlesource.com/c/go/+/171477
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/syscall/dirent_bsd_test.go b/src/syscall/dirent_bsd_test.go
index e5b8357af70a185b3da8004994cf5ccb84062231..c0ae2a91b9b861c64fb47d66859839b451b2af41 100644 (file)
--- a/src/syscall/dirent_bsd_test.go
+++ b/src/syscall/dirent_bsd_test.go
@@ -8,6 +8,7 @@ package syscall_test
 
 import (
        "bytes"
+       "fmt"
        "io/ioutil"
        "os"
        "path/filepath"
@@ -16,6 +17,7 @@ import (
        "strings"
        "syscall"
        "testing"
+       "unsafe"
 )
 
 func TestDirent(t *testing.T) {
@@ -41,10 +43,10 @@ func TestDirent(t *testing.T) {
 
        buf := bytes.Repeat([]byte("DEADBEAF"), direntBufSize/8)
        fd, err := syscall.Open(d, syscall.O_RDONLY, 0)
-       defer syscall.Close(fd)
        if err != nil {
                t.Fatalf("syscall.open: %v", err)
        }
+       defer syscall.Close(fd)
        n, err := syscall.ReadDirent(fd, buf)
        if err != nil {
                t.Fatalf("syscall.readdir: %v", err)
@@ -74,3 +76,58 @@ func TestDirent(t *testing.T) {
                }
        }
 }
+
+func TestDirentRepeat(t *testing.T) {
+       const N = 100
+
+       // Make a directory containing N files
+       d, err := ioutil.TempDir("", "direntRepeat-test")
+       if err != nil {
+               t.Fatalf("tempdir: %v", err)
+       }
+       defer os.RemoveAll(d)
+
+       var files []string
+       for i := 0; i < N; i++ {
+               files = append(files, fmt.Sprintf("file%d", i))
+       }
+       for _, file := range files {
+               err = ioutil.WriteFile(filepath.Join(d, file), []byte("contents"), 0644)
+               if err != nil {
+                       t.Fatalf("writefile: %v", err)
+               }
+       }
+
+       // Read the directory entries using ReadDirent.
+       fd, err := syscall.Open(d, syscall.O_RDONLY, 0)
+       if err != nil {
+               t.Fatalf("syscall.open: %v", err)
+       }
+       defer syscall.Close(fd)
+       var files2 []string
+       for {
+               // Note: the buf is small enough that this loop will need to
+               // execute multiple times. See issue #31368.
+               buf := make([]byte, N*unsafe.Offsetof(syscall.Dirent{}.Name)/4)
+               n, err := syscall.ReadDirent(fd, buf)
+               if err != nil {
+                       t.Fatalf("syscall.readdir: %v", err)
+               }
+               if n == 0 {
+                       break
+               }
+               buf = buf[:n]
+               for len(buf) > 0 {
+                       var consumed int
+                       consumed, _, files2 = syscall.ParseDirent(buf, -1, files2)
+                       buf = buf[consumed:]
+               }
+       }
+
+       // Check results
+       sort.Strings(files)
+       sort.Strings(files2)
+       if strings.Join(files, "|") != strings.Join(files2, "|") {
+               t.Errorf("bad file list: want\n%q\ngot\n%q", files, files2)
+       }
+}

diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go
index 422f3d44250382272bdb440aa1bb2a96970850c9..e5d0d5c386578e9c6c4db667ce44ab35e7d3dd4c 100644 (file)
--- a/src/syscall/syscall_darwin.go
+++ b/src/syscall/syscall_darwin.go
@@ -368,6 +368,18 @@ func writelen(fd int, buf *byte, nbuf int) (n int, err error) {
 func Getdirentries(fd int, buf []byte, basep *uintptr) (n int, err error) {
        // Simulate Getdirentries using fdopendir/readdir_r/closedir.
        const ptrSize = unsafe.Sizeof(uintptr(0))
+
+       // We store the number of entries to skip in the seek
+       // offset of fd. See issue #31368.
+       // It's not the full required semantics, but should handle the case
+       // of calling Getdirentries or ReadDirent repeatedly.
+       // It won't handle assigning the results of lseek to *basep, or handle
+       // the directory being edited underfoot.
+       skip, err := Seek(fd, 0, 1 /* SEEK_CUR */)
+       if err != nil {
+               return 0, err
+       }
+
        // We need to duplicate the incoming file descriptor
        // because the caller expects to retain control of it, but
        // fdopendir expects to take control of its argument.
@@ -384,13 +396,8 @@ func Getdirentries(fd int, buf []byte, basep *uintptr) (n int, err error) {
                return 0, err
        }
        defer closedir(d)
-       // We keep the number of records already returned in *basep.
-       // It's not the full required semantics, but should handle the case
-       // of calling Getdirentries repeatedly.
-       // It won't handle assigning the results of lseek to *basep, or handle
-       // the directory being edited underfoot.
-       skip := *basep
-       *basep = 0
+
+       var cnt int64
        for {
                var entry Dirent
                var entryp *Dirent
@@ -403,13 +410,13 @@ func Getdirentries(fd int, buf []byte, basep *uintptr) (n int, err error) {
                }
                if skip > 0 {
                        skip--
-                       *basep++
+                       cnt++
                        continue
                }
                reclen := int(entry.Reclen)
                if reclen > len(buf) {
                        // Not enough room. Return for now.
-                       // *basep will let us know where we should start up again.
+                       // The counter will let us know where we should start up again.
                        // Note: this strategy for suspending in the middle and
                        // restarting is O(n^2) in the length of the directory. Oh well.
                        break
@@ -423,8 +430,15 @@ func Getdirentries(fd int, buf []byte, basep *uintptr) (n int, err error) {
                copy(buf, *(*[]byte)(unsafe.Pointer(&s)))
                buf = buf[reclen:]
                n += reclen
-               *basep++
+               cnt++
        }
+       // Set the seek offset of the input fd to record
+       // how many files we've already returned.
+       _, err = Seek(fd, cnt, 0 /* SEEK_SET */)
+       if err != nil {
+               return n, err
+       }
+
        return n, nil
 }