helper: separate pipes from Helper

Upcoming bwrap helper implementation requires two sets of pipes to be managed, fd will also no longer be constant.

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>

1 files changed, 17 insertions, 119 deletions

diff --git a/helper/helper.go b/helper/helper.go
index af6887cc..ed4e4577 100644
--- a/helper/helper.go
+++ b/helper/helper.go
@@ -6,7 +6,6 @@ package helper
 import (
 	"errors"
 	"io"
-	"os"
 	"os/exec"
 	"sync"
 )
@@ -17,27 +16,21 @@ var (
 )
 
 const (
+	// FortifyHelper is set for the process launched by Helper.
 	FortifyHelper = "FORTIFY_HELPER"
+	// FortifyStatus is 1 when sync fd is enabled and 0 otherwise.
 	FortifyStatus = "FORTIFY_STATUS"
 )
 
 // Helper wraps *exec.Cmd and manages status and args fd.
 // Args is always 3 and status if set is always 4.
 type Helper struct {
-	args io.WriterTo
+	p *pipes
 
-	statP [2]*os.File
-	argsP [2]*os.File
-
-	ready chan error
-
-	// ExtraFiles specifies additional open files to be inherited by the
-	// new process. It does not include standard input, standard output, or
-	// standard error. If non-nil, entry i becomes file descriptor 5+i.
-	ExtraFiles []*os.File
+	argF func(argsFD, statFD int) []string
+	*exec.Cmd
 
 	lock sync.RWMutex
-	*exec.Cmd
 }
 
 func (h *Helper) StartNotify(ready chan error) error {
@@ -50,95 +43,24 @@ func (h *Helper) StartNotify(ready chan error) error {
 		return errors.New("exec: already started")
 	}
 
-	// create pipes
-	if pr, pw, err := os.Pipe(); err != nil {
+	h.p.ready = ready
+	if argsFD, statFD, err := h.p.prepareCmd(h.Cmd); err != nil {
 		return err
 	} else {
-		h.argsP[0], h.argsP[1] = pr, pw
+		h.Cmd.Args = append(h.Cmd.Args, h.argF(argsFD, statFD)...)
 	}
-	// create status pipes if ready signal is requested
-	var sv string
-	if ready != nil {
-		if pr, pw, err := os.Pipe(); err != nil {
-			return err
-		} else {
-			h.statP[0], h.statP[1] = pr, pw
-		}
 
-		sv = FortifyStatus + "=1"
-	} else {
-		sv = FortifyStatus + "=0"
-	}
-
-	// prepare extra files from caller
-	el := len(h.ExtraFiles)
 	if ready != nil {
-		el += 2
+		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=1")
 	} else {
-		el++
+		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=0")
 	}
-	ef := make([]*os.File, 0, el)
-	ef = append(ef, h.argsP[0])
-	if ready != nil {
-		ef = append(ef, h.statP[1])
-	}
-	ef = append(ef, h.ExtraFiles...)
 
-	// prepare and start process
-	h.Cmd.ExtraFiles = ef
-	h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", sv)
 	if err := h.Cmd.Start(); err != nil {
 		return err
 	}
-
-	statsP, argsP := h.statP[0], h.argsP[1]
-
-	// write arguments and close args pipe
-	if _, err := h.args.WriteTo(argsP); err != nil {
-		if err1 := h.Cmd.Process.Kill(); err1 != nil {
-			// should be unreachable
-			panic(err1.Error())
-		}
+	if err := h.p.readyWriteArgs(); err != nil {
 		return err
-	} else {
-		if err = argsP.Close(); err != nil {
-			if err1 := h.Cmd.Process.Kill(); err1 != nil {
-				// should be unreachable
-				panic(err1.Error())
-			}
-			return err
-		}
-	}
-
-	if ready != nil {
-		h.ready = ready
-
-		// monitor stat pipe
-		go func() {
-			n, err := statsP.Read(make([]byte, 1))
-			switch n {
-			case -1:
-				if err1 := h.Cmd.Process.Kill(); err1 != nil {
-					// should be unreachable
-					panic(err1.Error())
-				}
-				// ensure error is not nil
-				if err == nil {
-					err = ErrStatusFault
-				}
-				ready <- err
-			case 0:
-				// ensure error is not nil
-				if err == nil {
-					err = ErrStatusRead
-				}
-				ready <- err
-			case 1:
-				ready <- nil
-			default:
-				panic("unreachable") // unexpected read count
-			}
-		}()
 	}
 
 	return nil
@@ -155,38 +77,12 @@ func (h *Helper) Wait() error {
 		return errors.New("exec: Wait was already called")
 	}
 
-	// ensure pipe close
-	defer func() {
-		if err := h.argsP[0].Close(); err != nil && !errors.Is(err, os.ErrClosed) {
-			// unreachable
-			panic(err.Error())
-		}
-		if err := h.argsP[1].Close(); err != nil && !errors.Is(err, os.ErrClosed) {
-			// unreachable
-			panic(err.Error())
-		}
-
-		if h.ready != nil {
-			if err := h.statP[0].Close(); err != nil && !errors.Is(err, os.ErrClosed) {
-				// unreachable
-				panic(err.Error())
-			}
-			if err := h.statP[1].Close(); err != nil && !errors.Is(err, os.ErrClosed) {
-				// unreachable
-				panic(err.Error())
-			}
-		}
-	}()
-
+	defer h.p.mustClosePipes()
 	return h.Cmd.Wait()
 }
 
 func (h *Helper) Close() error {
-	if h.ready == nil {
-		panic("attempted to close helper with no status pipe")
-	}
-
-	return h.statP[0].Close()
+	return h.p.closeStatus()
 }
 
 func (h *Helper) Start() error {
@@ -195,10 +91,12 @@ func (h *Helper) Start() error {
 
 var execCommand = exec.Command
 
-func New(wt io.WriterTo, name string, arg ...string) *Helper {
+// New initialises a new Helper instance with wt as the null-terminated argument writer.
+// Function argF returns an array of arguments passed directly to the child process.
+func New(wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) *Helper {
 	if wt == nil {
 		panic("attempted to create helper with invalid argument writer")
 	}
 
-	return &Helper{args: wt, Cmd: execCommand(name, arg...)}
+	return &Helper{p: &pipes{args: wt}, argF: argF, Cmd: execCommand(name)}
 }