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 <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: randall77

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)
+	}
+}

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
 }