os: more descriptive error for File.ReadAt and File.WriteAt with negative offset.

The existing implementation does not provide a useful error message
if a negative offset is passed in File.ReadAt or File.WriteAt. This
change is to return descriptive errors. An error of type *PathError
is returned to keep it consistent with rest of the code.

There is no need to add an exported error variable since it's used only
in one file.

Fixes #19031

Change-Id: Ib94cab0afae8c5fe4dd97ed2887018a09b9f4538
Reviewed-on: https://go-review.googlesource.com/39136
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/os/file.go b/src/os/file.go
index 047217881f8943faad059cc79613f1c9a6455457..d61124b338e9a26909cba5bbd066087c41a33854 100644 (file)
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -37,6 +37,7 @@
 package os
 
 import (
+       "errors"
        "io"
        "syscall"
 )
@@ -117,6 +118,11 @@ func (f *File) ReadAt(b []byte, off int64) (n int, err error) {
        if err := f.checkValid("read"); err != nil {
                return 0, err
        }
+
+       if off < 0 {
+               return 0, &PathError{"readat", f.name, errors.New("negative offset")}
+       }
+
        for len(b) > 0 {
                m, e := f.pread(b, off)
                if e != nil {
@@ -164,6 +170,11 @@ func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
        if err := f.checkValid("write"); err != nil {
                return 0, err
        }
+
+       if off < 0 {
+               return 0, &PathError{"writeat", f.name, errors.New("negative offset")}
+       }
+
        for len(b) > 0 {
                m, e := f.pwrite(b, off)
                if e != nil {

diff --git a/src/os/os_test.go b/src/os/os_test.go
index ee9e0cf767355bb914704320372640711a5863cc..fcfcc436203a21f613a12862424eb98a7eca5624 100644 (file)
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -1451,6 +1451,26 @@ func TestReadAtOffset(t *testing.T) {
        }
 }
 
+// Verify that ReadAt doesn't allow negative offset.
+func TestReadAtNegativeOffset(t *testing.T) {
+       f := newFile("TestReadAtNegativeOffset", t)
+       defer Remove(f.Name())
+       defer f.Close()
+
+       const data = "hello, world\n"
+       io.WriteString(f, data)
+
+       f.Seek(0, 0)
+       b := make([]byte, 5)
+
+       n, err := f.ReadAt(b, -10)
+
+       const wantsub = "negative offset"
+       if !strings.Contains(fmt.Sprint(err), wantsub) || n != 0 {
+               t.Errorf("ReadAt(-10) = %v, %v; want 0, ...%q...", n, err, wantsub)
+       }
+}
+
 func TestWriteAt(t *testing.T) {
        f := newFile("TestWriteAt", t)
        defer Remove(f.Name())
@@ -1473,6 +1493,20 @@ func TestWriteAt(t *testing.T) {
        }
 }
 
+// Verify that WriteAt doesn't allow negative offset.
+func TestWriteAtNegativeOffset(t *testing.T) {
+       f := newFile("TestWriteAtNegativeOffset", t)
+       defer Remove(f.Name())
+       defer f.Close()
+
+       n, err := f.WriteAt([]byte("WORLD"), -10)
+
+       const wantsub = "negative offset"
+       if !strings.Contains(fmt.Sprint(err), wantsub) || n != 0 {
+               t.Errorf("WriteAt(-10) = %v, %v; want 0, ...%q...", n, err, wantsub)
+       }
+}
+
 func writeFile(t *testing.T, fname string, flag int, text string) string {
        f, err := OpenFile(fname, flag, 0666)
        if err != nil {