From a6af1041f6889fddd71c2a08308f52637b3f345d Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 10 Apr 2019 11:06:58 -0700 Subject: [PATCH] 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 Reviewed-by: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/syscall/dirent_bsd_test.go | 59 +++++++++++++++++++++++++++++++++- src/syscall/syscall_darwin.go | 34 ++++++++++++++------ 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/src/syscall/dirent_bsd_test.go b/src/syscall/dirent_bsd_test.go index e5b8357af7..c0ae2a91b9 100644 --- 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 422f3d4425..e5d0d5c386 100644 --- 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 }