helper/args: simplify argument parsing and eliminate excess memory copies

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

dbus/dbus.go

@@ -54,8 +54,6 @@ func (p *Proxy) Seal(session, system *Config) error {
 		return errors.New("no configuration to seal")
 	}
 
-	seal := helper.NewArgs()
-
 	var args []string
 	if session != nil {
 		args = append(args, session.Args(p.session)...)
@@ -63,11 +61,12 @@ func (p *Proxy) Seal(session, system *Config) error {
 	if system != nil {
 		args = append(args, system.Args(p.system)...)
 	}
-	if err := seal.Seal(args); err != nil {
+	if seal, err := helper.NewCheckedArgs(args); err != nil {
 		return err
+	} else {
+		p.seal = seal
 	}
 
-	p.seal = seal
 	return nil
 }
 

helper/args.go

@@ -1,100 +1,59 @@
 package helper
 
 import (
-	"bytes"
 	"errors"
-	"fmt"
 	"io"
 	"strings"
-	"sync"
 )
 
 var (
 	ErrContainsNull = errors.New("argument contains null character")
 )
 
-// Args is sealed with a slice of arguments for writing to the helper args FD.
+type argsFD []string
-// The sealing args is checked to not contain null characters.
-// Attempting to seal an instance twice will cause a panic.
-type Args interface {
-	Seal(args []string) error
-	io.WriterTo
-	fmt.Stringer
-}
 
-// argsFD implements Args for helpers expecting null terminated arguments to a file descriptor.
+// checks whether any element contains the null character
-// argsFD must not be copied after first use.
+// must be called before args use and args must not be modified after call
-type argsFD struct {
+func (a argsFD) check() error {
-	seal []byte
+	for _, arg := range a {
-	sync.RWMutex
+		for _, b := range arg {
-}
+			if b == '\x00' {
-
+				return ErrContainsNull
-func (a *argsFD) Seal(args []string) error {
+			}
-	a.Lock()
-	defer a.Unlock()
-
-	if a.seal != nil {
-		panic("args sealed twice")
-	}
-
-	seal := bytes.Buffer{}
-
-	n := 0
-	for _, arg := range args {
-		// reject argument strings containing null
-		if hasNull(arg) {
-			return ErrContainsNull
 		}
-
-		// accumulate buffer size
-		n += len(arg) + 1
-	}
-	seal.Grow(n)
-
-	// write null terminated arguments
-	for _, arg := range args {
-		seal.WriteString(arg)
-		seal.WriteByte('\x00')
 	}
 
-	a.seal = seal.Bytes()
 	return nil
 }
 
-func (a *argsFD) WriteTo(w io.Writer) (int64, error) {
+func (a argsFD) WriteTo(w io.Writer) (int64, error) {
-	a.RLock()
+	// assuming already checked
-	defer a.RUnlock()
 
-	if a.seal == nil {
+	nt := 0
-		panic("attempted to activate unsealed args")
+	// write null terminated arguments
+	for _, arg := range a {
+		n, err := w.Write([]byte(arg + "\x00"))
+		nt += n
+
+		if err != nil {
+			return int64(nt), err
+		}
 	}
 
-	n, err := w.Write(a.seal)
+	return int64(nt), nil
-	return int64(n), err
 }
 
-func (a *argsFD) String() string {
+func (a argsFD) String() string {
 	if a == nil {
 		return "(invalid helper args)"
 	}
 
-	if a.seal == nil {
+	return strings.Join(a, " ")
-		return "(unsealed helper args)"
-	}
-
-	return strings.ReplaceAll(string(a.seal), "\x00", " ")
 }
 
-func hasNull(s string) bool {
+// NewCheckedArgs returns a checked argument writer for args.
-	for _, b := range s {
+// Callers must not retain any references to args.
-		if b == '\x00' {
+func NewCheckedArgs(args []string) (io.WriterTo, error) {
-			return true
+	a := argsFD(args)
-		}
+	return a, a.check()
-	}
-	return false
-}
-
-// NewArgs returns a new instance of Args
-func NewArgs() Args {
-	return new(argsFD)
 }

helper/helper.go

@@ -19,7 +19,6 @@ var (
 // Helper wraps *exec.Cmd and manages status and args fd.
 // Args is always 3 and status if set is always 4.
 type Helper struct {
-	lock sync.RWMutex
 	args io.WriterTo
 
 	statP [2]*os.File
@@ -32,6 +31,7 @@ type Helper struct {
 	// standard error. If non-nil, entry i becomes file descriptor 5+i.
 	ExtraFiles []*os.File
 
+	lock sync.RWMutex
 	*exec.Cmd
 }
 
@@ -177,7 +177,7 @@ func (h *Helper) Start() error {
 
 func New(wt io.WriterTo, name string, arg ...string) *Helper {
 	if wt == nil {
-		panic("attempted to create helper with nil argument writer")
+		panic("attempted to create helper with invalid argument writer")
 	}
 
 	return &Helper{args: wt, Cmd: exec.Command(name, arg...)}