Commit 212d2f82 authored by Dan Caddigan's avatar Dan Caddigan Committed by Brad Fitzpatrick

os: add ErrClosed, return for use of closed File

This is clearer than syscall.EBADF.

Fixes #17320.

Change-Id: I14c6a362f9a6044c9b07cd7965499f4a83d2a860
Reviewed-on: https://go-review.googlesource.com/30614
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
parent 452bbfc1
......@@ -14,6 +14,7 @@ var (
ErrPermission = errors.New("permission denied")
ErrExist = errors.New("file already exists")
ErrNotExist = errors.New("file does not exist")
ErrClosed = errors.New("file already closed")
)
// PathError records an error and the operation and file path that caused it.
......
......@@ -91,10 +91,12 @@ var isExistTests = []isExistTest{
{&os.PathError{Err: os.ErrPermission}, false, false},
{&os.PathError{Err: os.ErrExist}, true, false},
{&os.PathError{Err: os.ErrNotExist}, false, true},
{&os.PathError{Err: os.ErrClosed}, false, false},
{&os.LinkError{Err: os.ErrInvalid}, false, false},
{&os.LinkError{Err: os.ErrPermission}, false, false},
{&os.LinkError{Err: os.ErrExist}, true, false},
{&os.LinkError{Err: os.ErrNotExist}, false, true},
{&os.LinkError{Err: os.ErrClosed}, false, false},
{&os.SyscallError{Err: os.ErrNotExist}, false, true},
{&os.SyscallError{Err: os.ErrExist}, true, false},
{nil, false, false},
......
......@@ -95,8 +95,8 @@ func (e *LinkError) Error() string {
// It returns the number of bytes read and an error, if any.
// EOF is signaled by a zero count with err set to io.EOF.
func (f *File) Read(b []byte) (n int, err error) {
if f == nil {
return 0, ErrInvalid
if err := f.checkValid("read"); err != nil {
return 0, err
}
n, e := f.read(b)
if n == 0 && len(b) > 0 && e == nil {
......@@ -113,8 +113,8 @@ func (f *File) Read(b []byte) (n int, err error) {
// ReadAt always returns a non-nil error when n < len(b).
// At end of file, that error is io.EOF.
func (f *File) ReadAt(b []byte, off int64) (n int, err error) {
if f == nil {
return 0, ErrInvalid
if err := f.checkValid("read"); err != nil {
return 0, err
}
for len(b) > 0 {
m, e := f.pread(b, off)
......@@ -136,8 +136,8 @@ func (f *File) ReadAt(b []byte, off int64) (n int, err error) {
// It returns the number of bytes written and an error, if any.
// Write returns a non-nil error when n != len(b).
func (f *File) Write(b []byte) (n int, err error) {
if f == nil {
return 0, ErrInvalid
if err := f.checkValid("write"); err != nil {
return 0, err
}
n, e := f.write(b)
if n < 0 {
......@@ -159,8 +159,8 @@ func (f *File) Write(b []byte) (n int, err error) {
// It returns the number of bytes written and an error, if any.
// WriteAt returns a non-nil error when n != len(b).
func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
if f == nil {
return 0, ErrInvalid
if err := f.checkValid("write"); err != nil {
return 0, err
}
for len(b) > 0 {
m, e := f.pwrite(b, off)
......@@ -181,8 +181,8 @@ func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
// It returns the new offset and an error, if any.
// The behavior of Seek on a file opened with O_APPEND is not specified.
func (f *File) Seek(offset int64, whence int) (ret int64, err error) {
if f == nil {
return 0, ErrInvalid
if err := f.checkValid("seek"); err != nil {
return 0, err
}
r, e := f.seek(offset, whence)
if e == nil && f.dirinfo != nil && r != 0 {
......@@ -197,9 +197,6 @@ func (f *File) Seek(offset int64, whence int) (ret int64, err error) {
// WriteString is like Write, but writes the contents of string s rather than
// a slice of bytes.
func (f *File) WriteString(s string) (n int, err error) {
if f == nil {
return 0, ErrInvalid
}
return f.Write([]byte(s))
}
......@@ -233,8 +230,8 @@ func Chdir(dir string) error {
// which must be a directory.
// If there is an error, it will be of type *PathError.
func (f *File) Chdir() error {
if f == nil {
return ErrInvalid
if err := f.checkValid("chdir"); err != nil {
return err
}
if e := syscall.Fchdir(f.fd); e != nil {
return &PathError{"chdir", f.name, e}
......@@ -278,3 +275,15 @@ func fixCount(n int, err error) (int, error) {
}
return n, err
}
// checkValid checks whether f is valid for use.
// If not, it returns an appropriate error, perhaps incorporating the operation name op.
func (f *File) checkValid(op string) error {
if f == nil {
return ErrInvalid
}
if f.fd == badFd {
return &PathError{op, f.name, ErrClosed}
}
return nil
}
......@@ -130,21 +130,21 @@ func OpenFile(name string, flag int, perm FileMode) (*File, error) {
// Close closes the File, rendering it unusable for I/O.
// It returns an error, if any.
func (f *File) Close() error {
if f == nil {
return ErrInvalid
if err := f.checkValid("close"); err != nil {
return err
}
return f.file.close()
}
func (file *file) close() error {
if file == nil || file.fd < 0 {
if file == nil || file.fd == badFd {
return ErrInvalid
}
var err error
if e := syscall.Close(file.fd); e != nil {
err = &PathError{"close", file.name, e}
}
file.fd = -1 // so it can't be closed again
file.fd = badFd // so it can't be closed again
// no need for a finalizer anymore
runtime.SetFinalizer(file, nil)
......
......@@ -57,8 +57,8 @@ func Chmod(name string, mode FileMode) error {
// Chmod changes the mode of the file to mode.
// If there is an error, it will be of type *PathError.
func (f *File) Chmod(mode FileMode) error {
if f == nil {
return ErrInvalid
if err := f.checkValid("chmod"); err != nil {
return err
}
if e := syscall.Fchmod(f.fd, syscallMode(mode)); e != nil {
return &PathError{"chmod", f.name, e}
......@@ -89,8 +89,8 @@ func Lchown(name string, uid, gid int) error {
// Chown changes the numeric uid and gid of the named file.
// If there is an error, it will be of type *PathError.
func (f *File) Chown(uid, gid int) error {
if f == nil {
return ErrInvalid
if err := f.checkValid("chown"); err != nil {
return err
}
if e := syscall.Fchown(f.fd, uid, gid); e != nil {
return &PathError{"chown", f.name, e}
......@@ -102,8 +102,8 @@ func (f *File) Chown(uid, gid int) error {
// It does not change the I/O offset.
// If there is an error, it will be of type *PathError.
func (f *File) Truncate(size int64) error {
if f == nil {
return ErrInvalid
if err := f.checkValid("truncate"); err != nil {
return err
}
if e := syscall.Ftruncate(f.fd, size); e != nil {
return &PathError{"truncate", f.name, e}
......@@ -115,11 +115,11 @@ func (f *File) Truncate(size int64) error {
// Typically, this means flushing the file system's in-memory copy
// of recently written data to disk.
func (f *File) Sync() error {
if f == nil {
return ErrInvalid
if err := f.checkValid("sync"); err != nil {
return err
}
if e := syscall.Fsync(f.fd); e != nil {
return NewSyscallError("fsync", e)
return &PathError{"sync", f.name, e}
}
return nil
}
......
......@@ -128,7 +128,7 @@ func (f *File) Close() error {
}
func (file *file) close() error {
if file == nil || file.fd < 0 {
if file == nil || file.fd == badFd {
return syscall.EINVAL
}
var err error
......
......@@ -193,7 +193,7 @@ func (file *file) close() error {
if e != nil {
err = &PathError{"close", file.name, e}
}
file.fd = syscall.InvalidHandle // so it can't be closed again
file.fd = badFd // so it can't be closed again
// no need for a finalizer anymore
runtime.SetFinalizer(file, nil)
......@@ -575,3 +575,5 @@ func Symlink(oldname, newname string) error {
}
return nil
}
const badFd = syscall.InvalidHandle
......@@ -229,6 +229,28 @@ func TestRead0(t *testing.T) {
}
}
// Reading a closed file should should return ErrClosed error
func TestReadClosed(t *testing.T) {
path := sfdir + "/" + sfname
file, err := Open(path)
if err != nil {
t.Fatal("open failed:", err)
}
file.Close() // close immediately
b := make([]byte, 100)
_, err = file.Read(b)
e, ok := err.(*PathError)
if !ok {
t.Fatalf("Read: %T(%v), want PathError", e, e)
}
if e.Err != ErrClosed {
t.Errorf("Read: %v, want PathError(ErrClosed)", e)
}
}
func testReaddirnames(dir string, contents []string, t *testing.T) {
file, err := Open(dir)
if err != nil {
......
......@@ -28,3 +28,5 @@ func sameFile(fs1, fs2 *fileStat) bool {
b := fs2.sys.(*syscall.Dir)
return a.Qid.Path == b.Qid.Path && a.Type == b.Type && a.Dev == b.Dev
}
const badFd = -1
......@@ -29,3 +29,5 @@ func (fs *fileStat) Sys() interface{} { return &fs.sys }
func sameFile(fs1, fs2 *fileStat) bool {
return fs1.sys.Dev == fs2.sys.Dev && fs1.sys.Ino == fs2.sys.Ino
}
const badFd = -1
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment