diff --git a/doc.go b/doc.go index c0235ce..833c9f5 100644 --- a/doc.go +++ b/doc.go @@ -16,35 +16,38 @@ func Open() (Pty, Tty, error) { return open() } +// FdHolder surfaces the Fd() method of the underlying handle. type FdHolder interface { Fd() uintptr } -// Pty for terminal control in current process -// for unix systems, the real type is *os.File -// for windows, the real type is a *WindowsPty for ConPTY handle +// Pty for terminal control in current process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsPty for ConPTY handle. type Pty interface { - // FdHolder Fd intended to resize Tty of child process in current process + // FdHolder is intended to resize / control ioctls of the TTY of the child process in current process. FdHolder Name() string - // WriteString is only used to identify Pty and Tty + // WriteString is only used to identify Pty and Tty. WriteString(s string) (n int, err error) - SetDeadline(t time.Time) error + SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } -// Tty for data i/o in child process -// for unix systems, the real type is *os.File -// for windows, the real type is a *WindowsTty, which is a combination of two pipe file +// Tty for data I/O in child process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsTty, which is a combination of two pipe file. type Tty interface { - // FdHolder Fd only intended for manual InheritSize from Pty + // FdHolder Fd only intended for manual InheritSize from Pty. FdHolder Name() string - SetDeadline(t time.Time) error + SetDeadline(t time.Time) error // TODO: Maybe move to FdHolder? io.ReadWriteCloser } diff --git a/pty_windows.go b/pty_windows.go index cf417b4..c10e7d0 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -14,6 +14,7 @@ import ( ) const ( + // Ref: https://pkg.go.dev/golang.org/x/sys/windows#pkg-constants PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = 0x20016 ) @@ -28,11 +29,11 @@ type WindowsTty struct { } var ( - // NOTE(security): as noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL - // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure - // the kernel32.dll is loaded from windows system path + // NOTE(security): As noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL + // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure + // the kernel32.dll is loaded from windows system path. // - // ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL + // Ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL kernel32DLL = windows.NewLazyDLL("kernel32.dll") // https://docs.microsoft.com/en-us/windows/console/createpseudoconsole @@ -51,6 +52,7 @@ func open() (_ Pty, _ Tty, err error) { consoleR, pw, err := os.Pipe() if err != nil { + // Closing everything. Best effort. _ = consoleW.Close() _ = pr.Close() return nil, nil, err @@ -58,9 +60,13 @@ func open() (_ Pty, _ Tty, err error) { var consoleHandle windows.Handle - err = procCreatePseudoConsole(windows.Handle(consoleR.Fd()), windows.Handle(consoleW.Fd()), - 0, &consoleHandle) - if err != nil { + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + if err := procCreatePseudoConsole( + windows.Handle(consoleR.Fd()), + windows.Handle(consoleW.Fd()), + 0, + &consoleHandle); err != nil { + // Closing everything. Best effort. _ = consoleW.Close() _ = pr.Close() _ = pw.Close() @@ -68,15 +74,12 @@ func open() (_ Pty, _ Tty, err error) { return nil, nil, err } - // These pipes can be closed here without any worry - err = consoleW.Close() - if err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + // These pipes can be closed here without any worry. + if err := consoleW.Close(); err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console write handle: %w", err) } - - err = consoleR.Close() - if err != nil { - return nil, nil, fmt.Errorf("failed to close pseudo console handle: %w", err) + if err := consoleR.Close(); err != nil { + return nil, nil, fmt.Errorf("failed to close pseudo console read handle: %w", err) } return &WindowsPty{ @@ -111,9 +114,7 @@ func (p *WindowsPty) WriteString(s string) (int, error) { } func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttributeListContainer) error { - var err error - - if err = attrList.Update( + if err := attrList.Update( PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, unsafe.Pointer(p.handle), unsafe.Sizeof(p.handle), @@ -125,16 +126,15 @@ func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttri } func (p *WindowsPty) Close() error { + // Best effort. _ = p.r.Close() _ = p.w.Close() - err := closePseudoConsole.Find() - if err != nil { + if err := closePseudoConsole.Find(); err != nil { return err } - _, _, err = closePseudoConsole.Call(uintptr(p.handle)) - + _, _, err := closePseudoConsole.Call(uintptr(p.handle)) return err } @@ -159,7 +159,7 @@ func (t *WindowsTty) Write(p []byte) (int, error) { } func (t *WindowsTty) Close() error { - _ = t.r.Close() + _ = t.r.Close() // Best effort. return t.w.Close() } @@ -168,20 +168,17 @@ func (t *WindowsTty) SetDeadline(value time.Time) error { } func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { - var r0 uintptr - var err error - - err = createPseudoConsole.Find() - if err != nil { + if err := createPseudoConsole.Find(); err != nil { return err } - r0, _, err = createPseudoConsole.Call( - (windowsCoord{X: 80, Y: 30}).Pack(), // size: default 80x30 window - uintptr(hInput), // console input - uintptr(hOutput), // console output - uintptr(dwFlags), // console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported - uintptr(unsafe.Pointer(consoleHandle)), // console handler value return + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := createPseudoConsole.Call( + (windowsCoord{X: 80, Y: 30}).Pack(), // Size: default 80x30 window. + uintptr(hInput), // Console input. + uintptr(hOutput), // Console output. + uintptr(dwFlags), // Console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported. + uintptr(unsafe.Pointer(consoleHandle)), // Console handler value return. ) if int32(r0) < 0 { diff --git a/run_windows.go b/run_windows.go index dd5201a..d63ea7a 100644 --- a/run_windows.go +++ b/run_windows.go @@ -40,23 +40,23 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, } defer func() { - // unlike unix command exec, do not close tty unless error happened + // Unlike unix command exec, do not close tty unless error happened. if err != nil { - _ = pty.Close() + _ = pty.Close() // Best effort. } }() if sz != nil { - if err = Setsize(pty, sz); err != nil { + if err := Setsize(pty, sz); err != nil { return nil, err } } - // unlike unix command exec, do not set stdin/stdout/stderr + // Unlike unix command exec, do not set stdin/stdout/stderr. c.SysProcAttr = attrs - // do not use os/exec.Start since we need to append console handler to startup info + // Do not use os/exec.Start since we need to append console handler to startup info. w := windowExecCmd{ cmd: c, @@ -64,8 +64,7 @@ func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, conPty: pty.(*WindowsPty), } - err = w.Start() - if err != nil { + if err := w.Start(); err != nil { return nil, err } @@ -83,7 +82,7 @@ func (c *windowExecCmd) Start() error { return errors.New("exec: already started") } - var argv0 = c.cmd.Path + argv0 := c.cmd.Path var argv0p *uint16 var argvp *uint16 var dirp *uint16 @@ -131,7 +130,7 @@ func (c *windowExecCmd) Start() error { // Windows CreateProcess takes the command line as a single string: // use attr.CmdLine if set, else build the command line by escaping - // and joining each argument with spaces + // and joining each argument with spaces. if sys.CmdLine != "" { cmdline = sys.CmdLine } else { @@ -225,7 +224,7 @@ func (c *windowExecCmd) Start() error { if err != nil { return err } - + go c.waitProcess(c.cmd.Process) return nil diff --git a/winsize_windows.go b/winsize_windows.go index 5b96004..c9a3111 100644 --- a/winsize_windows.go +++ b/winsize_windows.go @@ -8,14 +8,15 @@ import ( "unsafe" ) -// types from golang.org/x/sys/windows -// copy of https://pkg.go.dev/golang.org/x/sys/windows#Coord +// Types from golang.org/x/sys/windows. + +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#Coord type windowsCoord struct { X int16 Y int16 } -// copy of https://pkg.go.dev/golang.org/x/sys/windows#SmallRect +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#SmallRect type windowsSmallRect struct { Left int16 Top int16 @@ -23,7 +24,7 @@ type windowsSmallRect struct { Bottom int16 } -// copy of https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo type windowsConsoleScreenBufferInfo struct { Size windowsCoord CursorPosition windowsCoord @@ -38,15 +39,13 @@ func (c windowsCoord) Pack() uintptr { // Setsize resizes t to ws. func Setsize(t FdHolder, ws *Winsize) error { - var r0 uintptr - var err error - - err = resizePseudoConsole.Find() - if err != nil { + if err := resizePseudoConsole.Find(); err != nil { return err } - r0, _, err = resizePseudoConsole.Call( + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := resizePseudoConsole.Call( t.Fd(), (windowsCoord{X: int16(ws.Cols), Y: int16(ws.Rows)}).Pack(), ) @@ -64,15 +63,13 @@ func Setsize(t FdHolder, ws *Winsize) error { // GetsizeFull returns the full terminal size description. func GetsizeFull(t FdHolder) (size *Winsize, err error) { - err = getConsoleScreenBufferInfo.Find() - if err != nil { + if err := getConsoleScreenBufferInfo.Find(); err != nil { return nil, err } var info windowsConsoleScreenBufferInfo - var r0 uintptr - - r0, _, err = getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) if int32(r0) < 0 { if r0&0x1fff0000 == 0x00070000 { r0 &= 0xffff