Skip to content

Commit

Permalink
os: reset dirinfo when seeking on Darwin
Browse files Browse the repository at this point in the history
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 <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
randall77 committed Dec 5, 2019
1 parent d72dce8 commit e3c7ffc
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/os/dir_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions src/os/dir_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/os/file_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/syscall/fs_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
51 changes: 51 additions & 0 deletions test/fixedbugs/issue35767.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}

0 comments on commit e3c7ffc

Please sign in to comment.