Skip to content

Commit

Permalink
Minor cosmetic fixes, add few comments/questions.
Browse files Browse the repository at this point in the history
  • Loading branch information
creack committed Oct 28, 2023
1 parent c3e5fb6 commit 96e45d8
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 70 deletions.
25 changes: 14 additions & 11 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
65 changes: 31 additions & 34 deletions pty_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
)

const (
// Ref: https://pkg.go.dev/golang.org/x/sys/windows#pkg-constants
PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = 0x20016
)

Expand All @@ -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/[email protected]?GOOS=windows#LoadDLL
// Ref: https://pkg.go.dev/[email protected]?GOOS=windows#LoadDLL
kernel32DLL = windows.NewLazyDLL("kernel32.dll")

// https://docs.microsoft.com/en-us/windows/console/createpseudoconsole
Expand All @@ -51,32 +52,34 @@ 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
}

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()
_ = consoleR.Close()
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{
Expand Down Expand Up @@ -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),
Expand All @@ -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
}

Expand All @@ -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()
}

Expand All @@ -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 {
Expand Down
19 changes: 9 additions & 10 deletions run_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,32 +40,31 @@ 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,
waitCalled: false,
conPty: pty.(*WindowsPty),
}

err = w.Start()
if err != nil {
if err := w.Start(); err != nil {
return nil, err
}

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -225,7 +224,7 @@ func (c *windowExecCmd) Start() error {
if err != nil {
return err
}

go c.waitProcess(c.cmd.Process)

return nil
Expand Down
27 changes: 12 additions & 15 deletions winsize_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,23 @@ 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
Right int16
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
Expand All @@ -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(),
)
Expand All @@ -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
Expand Down

0 comments on commit 96e45d8

Please sign in to comment.