Improve error management for openpty (#57)

Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net>
creack · Jan 13, 2018 · 282ce0e · 282ce0e
1 parent f8811ff
commit 282ce0e
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 17 deletions.
diff --git a/pty_darwin.go b/pty_darwin.go
@@ -13,19 +13,23 @@ func open() (pty, tty *os.File, err error) {
 		return nil, nil, err
 	}
 	p := os.NewFile(uintptr(pFD), "/dev/ptmx")
+	// In case of error after this point, make sure we close the ptmx fd.
+	defer func() {
+		if err != nil {
+			_ = p.Close() // Best effort.
+		}
+	}()
 
 	sname, err := ptsname(p)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	err = grantpt(p)
-	if err != nil {
+	if err := grantpt(p); err != nil {
 		return nil, nil, err
 	}
 
-	err = unlockpt(p)
-	if err != nil {
+	if err := unlockpt(p); err != nil {
 		return nil, nil, err
 	}
 

diff --git a/pty_dragonfly.go b/pty_dragonfly.go
@@ -14,19 +14,23 @@ func open() (pty, tty *os.File, err error) {
 	if err != nil {
 		return nil, nil, err
 	}
+	// In case of error after this point, make sure we close the ptmx fd.
+	defer func() {
+		if err != nil {
+			_ = p.Close() // Best effort.
+		}
+	}()
 
 	sname, err := ptsname(p)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	err = grantpt(p)
-	if err != nil {
+	if err := grantpt(p); err != nil {
 		return nil, nil, err
 	}
 
-	err = unlockpt(p)
-	if err != nil {
+	if err := unlockpt(p); err != nil {
 		return nil, nil, err
 	}
 

diff --git a/pty_freebsd.go b/pty_freebsd.go
@@ -7,22 +7,28 @@ import (
 	"unsafe"
 )
 
-func posix_openpt(oflag int) (fd int, err error) {
+func posixOpenpt(oflag int) (fd int, err error) {
 	r0, _, e1 := syscall.Syscall(syscall.SYS_POSIX_OPENPT, uintptr(oflag), 0, 0)
 	fd = int(r0)
 	if e1 != 0 {
 		err = e1
 	}
-	return
+	return fd, err
 }
 
 func open() (pty, tty *os.File, err error) {
-	fd, err := posix_openpt(syscall.O_RDWR | syscall.O_CLOEXEC)
+	fd, err := posixOpenpt(syscall.O_RDWR | syscall.O_CLOEXEC)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	p := os.NewFile(uintptr(fd), "/dev/pts")
+	// In case of error after this point, make sure we close the pts fd.
+	defer func() {
+		if err != nil {
+			_ = p.Close() // Best effort.
+		}
+	}()
+
 	sname, err := ptsname(p)
 	if err != nil {
 		return nil, nil, err
@@ -42,7 +48,7 @@ func isptmaster(fd uintptr) (bool, error) {
 
 var (
 	emptyFiodgnameArg fiodgnameArg
-	ioctl_FIODGNAME   = _IOW('f', 120, unsafe.Sizeof(emptyFiodgnameArg))
+	ioctlFIODGNAME    = _IOW('f', 120, unsafe.Sizeof(emptyFiodgnameArg))
 )
 
 func ptsname(f *os.File) (string, error) {
@@ -59,8 +65,7 @@ func ptsname(f *os.File) (string, error) {
 		buf = make([]byte, n)
 		arg = fiodgnameArg{Len: n, Buf: (*byte)(unsafe.Pointer(&buf[0]))}
 	)
-	err = ioctl(f.Fd(), ioctl_FIODGNAME, uintptr(unsafe.Pointer(&arg)))
-	if err != nil {
+	if err := ioctl(f.Fd(), ioctlFIODGNAME, uintptr(unsafe.Pointer(&arg))); err != nil {
 		return "", err
 	}
 

diff --git a/pty_linux.go b/pty_linux.go
@@ -12,14 +12,19 @@ func open() (pty, tty *os.File, err error) {
 	if err != nil {
 		return nil, nil, err
 	}
+	// In case of error after this point, make sure we close the ptmx fd.
+	defer func() {
+		if err != nil {
+			_ = p.Close() // Best effort.
+		}
+	}()
 
 	sname, err := ptsname(p)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	err = unlockpt(p)
-	if err != nil {
+	if err := unlockpt(p); err != nil {
 		return nil, nil, err
 	}