From 61be9189f12c018f90542cba023c3404e069dca2 Mon Sep 17 00:00:00 2001 From: Dan Kegel Date: Wed, 29 Dec 2021 09:36:01 -0800 Subject: [PATCH] os: add a few upstream tests for Read and ReadAt, fix problems they exposed. There are more upstream tests to pull in, but this is plenty for today. --- src/os/file.go | 8 ++- src/os/file_anyos.go | 2 +- src/os/file_unix.go | 2 +- src/os/os_test.go | 140 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 4 deletions(-) diff --git a/src/os/file.go b/src/os/file.go index 33f204cd..7b4585bc 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -101,6 +101,7 @@ func Create(name string) (*File, error) { // read and any error encountered. At end of file, Read returns 0, io.EOF. func (f *File) Read(b []byte) (n int, err error) { n, err = f.handle.Read(b) + // TODO: want to always wrap, like upstream, but ReadFile() compares against exactly io.EOF? if err != nil && err != io.EOF { err = &PathError{"read", f.name, err} } @@ -120,8 +121,11 @@ func (f *File) ReadAt(b []byte, offset int64) (n int, err error) { for len(b) > 0 { m, e := f.handle.ReadAt(b, offset) if e != nil { - if err != io.EOF { - err = &PathError{"readat", f.name, err} + // TODO: want to always wrap, like upstream, but TestReadAtEOF compares against exactly io.EOF? + if e != io.EOF { + err = &PathError{"readat", f.name, e} + } else { + err = e } break } diff --git a/src/os/file_anyos.go b/src/os/file_anyos.go index 0b070a5f..77f214a0 100644 --- a/src/os/file_anyos.go +++ b/src/os/file_anyos.go @@ -118,7 +118,7 @@ type unixFileHandle uintptr func (f unixFileHandle) Read(b []byte) (n int, err error) { n, err = syscall.Read(syscallFd(f), b) err = handleSyscallError(err) - if n == 0 && err == nil { + if n == 0 && len(b) > 0 && err == nil { err = io.EOF } return diff --git a/src/os/file_unix.go b/src/os/file_unix.go index 51e1fffd..d10b910c 100644 --- a/src/os/file_unix.go +++ b/src/os/file_unix.go @@ -50,7 +50,7 @@ func tempDir() string { func (f unixFileHandle) ReadAt(b []byte, offset int64) (n int, err error) { n, err = syscall.Pread(syscallFd(f), b, offset) err = handleSyscallError(err) - if n == 0 && err == nil { + if n == 0 && len(b) > 0 && err == nil { err = io.EOF } return diff --git a/src/os/os_test.go b/src/os/os_test.go index 03e1bf42..1dedf871 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -8,6 +8,7 @@ import ( "io" . "os" "runtime" + "strings" "testing" ) @@ -24,6 +25,57 @@ func newFile(testName string, t *testing.T) (f *File) { return } +// Read with length 0 should not return EOF. +func TestRead0(t *testing.T) { + f := newFile("TestRead0", t) + defer Remove(f.Name()) + defer f.Close() + + const data = "hello, world\n" + io.WriteString(f, data) + f.Close() + f, err := Open(f.Name()) + if err != nil { + t.Errorf("failed to reopen") + } + + b := make([]byte, 0) + n, err := f.Read(b) + if n != 0 || err != nil { + t.Errorf("Read(0) = %d, %v, want 0, nil", n, err) + } + b = make([]byte, 5) + n, err = f.Read(b) + if n <= 0 || err != nil { + t.Errorf("Read(5) = %d, %v, want >0, nil", n, err) + } +} + +// ReadAt with length 0 should not return EOF. +func TestReadAt0(t *testing.T) { + if runtime.GOOS == "windows" { + t.Log("TODO: implement Pread for Windows") + return + } + f := newFile("TestReadAt0", t) + defer Remove(f.Name()) + defer f.Close() + + const data = "hello, world\n" + io.WriteString(f, data) + + b := make([]byte, 0) + n, err := f.ReadAt(b, 0) + if n != 0 || err != nil { + t.Errorf("ReadAt(0,0) = %d, %v, want 0, nil", n, err) + } + b = make([]byte, 5) + n, err = f.ReadAt(b, 0) + if n <= 0 || err != nil { + t.Errorf("ReadAt(5,0) = %d, %v, want >0, nil", n, err) + } +} + func checkMode(t *testing.T, path string, mode FileMode) { dir, err := Stat(path) if err != nil { @@ -55,3 +107,91 @@ func TestReadAt(t *testing.T) { t.Fatalf("ReadAt 7: have %q want %q", string(b), "world") } } + +// Verify that ReadAt doesn't affect seek offset. +// In the Plan 9 kernel, there used to be a bug in the implementation of +// the pread syscall, where the channel offset was erroneously updated after +// calling pread on a file. +func TestReadAtOffset(t *testing.T) { + if runtime.GOOS == "windows" { + t.Log("TODO: implement Pread for Windows") + return + } + f := newFile("TestReadAtOffset", t) + defer Remove(f.Name()) + defer f.Close() + + const data = "hello, world\n" + io.WriteString(f, data) + f.Close() + f, err := Open(f.Name()) + if err != nil { + t.Errorf("failed to reopen") + } + + b := make([]byte, 5) + + n, err := f.ReadAt(b, 7) + if err != nil || n != len(b) { + t.Fatalf("ReadAt 7: %d, %v", n, err) + } + if string(b) != "world" { + t.Fatalf("ReadAt 7: have %q want %q", string(b), "world") + } + + n, err = f.Read(b) + if err != nil || n != len(b) { + t.Fatalf("Read: %d, %v", n, err) + } + if string(b) != "hello" { + t.Fatalf("Read: have %q want %q", string(b), "hello") + } +} + +// Verify that ReadAt doesn't allow negative offset. +func TestReadAtNegativeOffset(t *testing.T) { + if runtime.GOOS == "windows" { + t.Log("TODO: implement Pread for Windows") + return + } + f := newFile("TestReadAtNegativeOffset", t) + defer Remove(f.Name()) + defer f.Close() + + const data = "hello, world\n" + io.WriteString(f, data) + f.Close() + f, err := Open(f.Name()) + if err != nil { + t.Errorf("failed to reopen") + } + + b := make([]byte, 5) + + n, err := f.ReadAt(b, -10) + + const wantsub = "negative offset" + if !strings.Contains(err.Error(), wantsub) || n != 0 { + t.Errorf("ReadAt(-10) = %v, %v; want 0, ...%q...", n, err, wantsub) + } +} + +func TestReadAtEOF(t *testing.T) { + if runtime.GOOS == "windows" { + t.Log("TODO: implement Pread for Windows") + return + } + f := newFile("TestReadAtEOF", t) + defer Remove(f.Name()) + defer f.Close() + + _, err := f.ReadAt(make([]byte, 10), 0) + switch err { + case io.EOF: + // all good + case nil: + t.Fatalf("ReadAt succeeded") + default: + t.Fatalf("ReadAt failed: %s", err) + } +}