From e3c7ffcd957785bc9d3e1f2db9219bd74a96bbb1 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 4 Dec 2019 14:58:04 -0800 Subject: [PATCH] os: reset dirinfo when seeking on Darwin The first Readdirnames calls opendir and caches the result. The behavior of that cached opendir result isn't specified on a seek of the underlying fd. Free the opendir result on a seek so that we'll allocate a new one the next time around. Also fix wasm behavior in this regard, so that a seek to the file start resets the Readdirnames position, regardless of platform. p.s. I hate the Readdirnames API. Fixes #35767. Change-Id: Ieffb61b3c5cdd42591f69ab13f932003966f2297 Reviewed-on: https://go-review.googlesource.com/c/go/+/209961 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Emmanuel Odeke Reviewed-by: Brad Fitzpatrick --- src/os/dir_darwin.go | 10 +++++++ src/os/dir_unix.go | 2 ++ src/os/file_unix.go | 1 + src/syscall/fs_js.go | 8 +++--- test/fixedbugs/issue35767.go | 51 ++++++++++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 test/fixedbugs/issue35767.go diff --git a/src/os/dir_darwin.go b/src/os/dir_darwin.go index 2f9ba78d68..a274dd1268 100644 --- a/src/os/dir_darwin.go +++ b/src/os/dir_darwin.go @@ -24,6 +24,16 @@ func (d *dirInfo) close() { d.dir = 0 } +func (f *File) seekInvalidate() { + if f.dirinfo == nil { + return + } + // Free cached dirinfo, so we allocate a new one if we + // access this file as a directory again. See #35767. + f.dirinfo.close() + f.dirinfo = nil +} + func (f *File) readdirnames(n int) (names []string, err error) { if f.dirinfo == nil { dir, call, errno := f.pfd.OpenDir() diff --git a/src/os/dir_unix.go b/src/os/dir_unix.go index e0c4989756..2856a2dc0f 100644 --- a/src/os/dir_unix.go +++ b/src/os/dir_unix.go @@ -26,6 +26,8 @@ const ( func (d *dirInfo) close() {} +func (f *File) seekInvalidate() {} + func (f *File) readdirnames(n int) (names []string, err error) { // If this file has no dirinfo, create one. if f.dirinfo == nil { diff --git a/src/os/file_unix.go b/src/os/file_unix.go index 31c43eb61e..6945937fd6 100644 --- a/src/os/file_unix.go +++ b/src/os/file_unix.go @@ -295,6 +295,7 @@ func (f *File) pwrite(b []byte, off int64) (n int, err error) { // relative to the current offset, and 2 means relative to the end. // It returns the new offset and an error, if any. func (f *File) seek(offset int64, whence int) (ret int64, err error) { + f.seekInvalidate() ret, err = f.pfd.Seek(offset, whence) runtime.KeepAlive(f) return ret, err diff --git a/src/syscall/fs_js.go b/src/syscall/fs_js.go index f7079e9d09..16d9f58b8c 100644 --- a/src/syscall/fs_js.go +++ b/src/syscall/fs_js.go @@ -34,6 +34,7 @@ var ( type jsFile struct { path string entries []string + dirIdx int // entries[:dirIdx] have already been returned in ReadDirent pos int64 seeked bool } @@ -141,8 +142,8 @@ func ReadDirent(fd int, buf []byte) (int, error) { } n := 0 - for len(f.entries) > 0 { - entry := f.entries[0] + for f.dirIdx < len(f.entries) { + entry := f.entries[f.dirIdx] l := 2 + len(entry) if l > len(buf) { break @@ -152,7 +153,7 @@ func ReadDirent(fd int, buf []byte) (int, error) { copy(buf[2:], entry) buf = buf[l:] n += l - f.entries = f.entries[1:] + f.dirIdx++ } return n, nil @@ -470,6 +471,7 @@ func Seek(fd int, offset int64, whence int) (int64, error) { } f.seeked = true + f.dirIdx = 0 // Reset directory read position. See issue 35767. f.pos = newPos return newPos, nil } diff --git a/test/fixedbugs/issue35767.go b/test/fixedbugs/issue35767.go new file mode 100644 index 0000000000..b930faa127 --- /dev/null +++ b/test/fixedbugs/issue35767.go @@ -0,0 +1,51 @@ +// run + +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "log" + "os" +) + +func main() { + wd, err := os.Getwd() + if err != nil { + log.Fatal(err) + } + f, err := os.Open(wd) + if err != nil { + log.Fatal(err) + } + dirnames1, err := f.Readdirnames(0) + if err != nil { + log.Fatal(err) + } + + ret, err := f.Seek(0, 0) + if err != nil { + log.Fatal(err) + } + if ret != 0 { + log.Fatalf("seek result not zero: %d", ret) + } + + dirnames2, err := f.Readdirnames(0) + if err != nil { + log.Fatal(err) + return + } + + if len(dirnames1) != len(dirnames2) { + log.Fatalf("listings have different lengths: %d and %d\n", len(dirnames1), len(dirnames2)) + } + for i, n1 := range dirnames1 { + n2 := dirnames2[i] + if n1 != n2 { + log.Fatalf("different name i=%d n1=%s n2=%s\n", i, n1, n2) + } + } +} -- 2.50.0