system: optimise string formatting

Signed-off-by: Ophestra <cat@gensokyo.uk>

system/acl_test.go

@@ -46,20 +46,20 @@ func TestUpdatePermType(t *testing.T) {
 	}
 }
 
-func TestACL_String(t *testing.T) {
+func TestACLString(t *testing.T) {
 	testCases := []struct {
 		want  string
 		et    Enablement
 		perms []acl.Perm
 	}{
-		{`--- type: Process path: "/nonexistent"`, Process, []acl.Perm{}},
-		{`r-- type: User path: "/nonexistent"`, User, []acl.Perm{acl.Read}},
-		{`-w- type: Wayland path: "/nonexistent"`, EWayland, []acl.Perm{acl.Write}},
-		{`--x type: X11 path: "/nonexistent"`, EX11, []acl.Perm{acl.Execute}},
-		{`rw- type: D-Bus path: "/nonexistent"`, EDBus, []acl.Perm{acl.Read, acl.Write}},
-		{`r-x type: PulseAudio path: "/nonexistent"`, EPulse, []acl.Perm{acl.Read, acl.Execute}},
-		{`rwx type: User path: "/nonexistent"`, User, []acl.Perm{acl.Read, acl.Write, acl.Execute}},
-		{`rwx type: Process path: "/nonexistent"`, Process, []acl.Perm{acl.Read, acl.Write, acl.Write, acl.Execute}},
+		{`--- type: process path: "/nonexistent"`, Process, []acl.Perm{}},
+		{`r-- type: user path: "/nonexistent"`, User, []acl.Perm{acl.Read}},
+		{`-w- type: wayland path: "/nonexistent"`, EWayland, []acl.Perm{acl.Write}},
+		{`--x type: x11 path: "/nonexistent"`, EX11, []acl.Perm{acl.Execute}},
+		{`rw- type: dbus path: "/nonexistent"`, EDBus, []acl.Perm{acl.Read, acl.Write}},
+		{`r-x type: pulseaudio path: "/nonexistent"`, EPulse, []acl.Perm{acl.Read, acl.Execute}},
+		{`rwx type: user path: "/nonexistent"`, User, []acl.Perm{acl.Read, acl.Write, acl.Execute}},
+		{`rwx type: process path: "/nonexistent"`, Process, []acl.Perm{acl.Read, acl.Write, acl.Write, acl.Execute}},
 	}
 
 	for _, tc := range testCases {

system/enablement.go

@@ -1,68 +1,47 @@
 package system
 
 import (
+	"fmt"
 	"strings"
 )
 
-type (
-	// Enablement represents an optional system resource
-	Enablement uint8
-	// Enablements represents optional system resources to share
-	Enablements uint64
-)
+// Enablement represents optional system resources.
+type Enablement byte
 
 const (
-	EWayland Enablement = iota
+	EWayland Enablement = 1 << iota
 	EX11
 	EDBus
 	EPulse
+
+	EM
 )
 
-var enablementString = [...]string{
-	EWayland: "Wayland",
-	EX11:     "X11",
-	EDBus:    "D-Bus",
-	EPulse:   "PulseAudio",
-}
-
-const ELen = len(enablementString)
-
 func (e Enablement) String() string {
-	if int(e) >= ELen {
-		return "<invalid enablement>"
-	}
-	return enablementString[e]
-}
+	switch e {
+	case 0:
+		return "(no enablements)"
+	case EWayland:
+		return "wayland"
+	case EX11:
+		return "x11"
+	case EDBus:
+		return "dbus"
+	case EPulse:
+		return "pulseaudio"
+	default:
+		buf := new(strings.Builder)
+		buf.Grow(32)
 
-func (e Enablement) Mask() Enablements {
-	return 1 << e
-}
-
-// Has returns whether a feature is enabled
-func (es *Enablements) Has(e Enablement) bool {
-	return *es&e.Mask() != 0
-}
-
-// Set enables a feature
-func (es *Enablements) Set(e Enablement) {
-	if es.Has(e) {
-		panic("enablement " + e.String() + " set twice")
-	}
-
-	*es |= e.Mask()
-}
-
-func (es *Enablements) String() string {
-	buf := new(strings.Builder)
-	for i := Enablement(0); i < Enablement(ELen); i++ {
-		if es.Has(i) {
-			buf.WriteString(", " + i.String())
+		for i := Enablement(1); i < EM; i <<= 1 {
+			if e&i != 0 {
+				buf.WriteString(", " + i.String())
+			}
 		}
-	}
 
-	if buf.Len() == 0 {
-		buf.WriteString("(No enablements)")
+		if buf.Len() == 0 {
+			return fmt.Sprintf("e%x", byte(e))
+		}
+		return strings.TrimPrefix(buf.String(), ", ")
 	}
-
-	return strings.TrimPrefix(buf.String(), ", ")
 }

system/enablement_test.go

@@ -0,0 +1,43 @@
+package system_test
+
+import (
+	"testing"
+
+	"git.gensokyo.uk/security/fortify/system"
+)
+
+func TestEnablementString(t *testing.T) {
+	testCases := []struct {
+		flags system.Enablement
+		want  string
+	}{
+		{0, "(no enablements)"},
+		{system.EWayland, "wayland"},
+		{system.EX11, "x11"},
+		{system.EDBus, "dbus"},
+		{system.EPulse, "pulseaudio"},
+		{system.EWayland | system.EX11, "wayland, x11"},
+		{system.EWayland | system.EDBus, "wayland, dbus"},
+		{system.EWayland | system.EPulse, "wayland, pulseaudio"},
+		{system.EX11 | system.EDBus, "x11, dbus"},
+		{system.EX11 | system.EPulse, "x11, pulseaudio"},
+		{system.EDBus | system.EPulse, "dbus, pulseaudio"},
+		{system.EWayland | system.EX11 | system.EDBus, "wayland, x11, dbus"},
+		{system.EWayland | system.EX11 | system.EPulse, "wayland, x11, pulseaudio"},
+		{system.EWayland | system.EDBus | system.EPulse, "wayland, dbus, pulseaudio"},
+		{system.EX11 | system.EDBus | system.EPulse, "x11, dbus, pulseaudio"},
+		{system.EWayland | system.EX11 | system.EDBus | system.EPulse, "wayland, x11, dbus, pulseaudio"},
+
+		{1 << 5, "e20"},
+		{1 << 6, "e40"},
+		{1 << 7, "e80"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.want, func(t *testing.T) {
+			if got := tc.flags.String(); got != tc.want {
+				t.Errorf("String: %q, want %q", got, tc.want)
+			}
+		})
+	}
+}

system/mkdir.go

@@ -78,7 +78,7 @@ func (m *Mkdir) Path() string {
 }
 
 func (m *Mkdir) String() string {
-	t := "Ensure"
+	t := "ensure"
 	if m.ephemeral {
 		t = TypeString(m.Type())
 	}

system/mkdir_test.go

@@ -41,20 +41,20 @@ func TestEphemeral(t *testing.T) {
 	}
 }
 
-func TestMkdir_String(t *testing.T) {
+func TestMkdirString(t *testing.T) {
 	testCases := []struct {
 		want      string
 		ephemeral bool
 		et        Enablement
 	}{
-		{"Ensure", false, User},
-		{"Ensure", false, Process},
-		{"Ensure", false, EWayland},
+		{"ensure", false, User},
+		{"ensure", false, Process},
+		{"ensure", false, EWayland},
 
-		{"Wayland", true, EWayland},
-		{"X11", true, EX11},
-		{"D-Bus", true, EDBus},
-		{"PulseAudio", true, EPulse},
+		{"wayland", true, EWayland},
+		{"x11", true, EX11},
+		{"dbus", true, EDBus},
+		{"pulseaudio", true, EPulse},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.want, func(t *testing.T) {

system/op.go

@@ -5,28 +5,29 @@ import (
 	"context"
 	"errors"
 	"log"
+	"strings"
 	"sync"
 )
 
 const (
 	// User type is reverted at final launcher exit.
-	User = Enablement(ELen)
+	User = EM << iota
 	// Process type is unconditionally reverted on exit.
-	Process = Enablement(ELen + 1)
+	Process
+
+	CM
 )
 
 // Criteria specifies types of Op to revert.
-type Criteria struct {
-	*Enablements
-}
+type Criteria Enablement
 
 func (ec *Criteria) hasType(o Op) bool {
 	// nil criteria: revert everything except User
-	if ec.Enablements == nil {
+	if ec == nil {
 		return o.Type() != User
 	}
 
-	return ec.Has(o.Type())
+	return Enablement(*ec)&o.Type() != 0
 }
 
 // Op is a reversible system operation.
@@ -48,11 +49,22 @@ type Op interface {
 func TypeString(e Enablement) string {
 	switch e {
 	case User:
-		return "User"
+		return "user"
 	case Process:
-		return "Process"
+		return "process"
 	default:
-		return e.String()
+		buf := new(strings.Builder)
+		buf.Grow(48)
+		if v := e &^ User &^ Process; v != 0 {
+			buf.WriteString(v.String())
+		}
+
+		for i := User; i < CM; i <<= 1 {
+			if e&i != 0 {
+				buf.WriteString(", " + TypeString(i))
+			}
+		}
+		return strings.TrimPrefix(buf.String(), ", ")
 	}
 }
 
@@ -110,7 +122,7 @@ func (sys *I) Commit(ctx context.Context) error {
 		if sp != nil {
 			// rollback partial commit
 			msg.Verbosef("commit faulted after %d ops, rolling back partial commit", len(sp.ops))
-			if err := sp.Revert(&Criteria{nil}); err != nil {
+			if err := sp.Revert(nil); err != nil {
 				log.Println("errors returned reverting partial commit:", err)
 			}
 		}

system/op_internal_test.go

@@ -47,10 +47,10 @@ func (ptc tcOp) test(t *testing.T, gotOps []Op, wantOps []Op, fn string) {
 				ec   *Criteria
 				want bool
 			}{
-				{"nil", newCriteria(), ptc.et != User},
+				{"nil", nil, ptc.et != User},
 				{"self", newCriteria(ptc.et), true},
-				{"all", newCriteria(EWayland, EX11, EDBus, EPulse, User, Process), true},
-				{"enablements", newCriteria(EWayland, EX11, EDBus, EPulse), ptc.et != User && ptc.et != Process},
+				{"all", newCriteria(EWayland | EX11 | EDBus | EPulse | User | Process), true},
+				{"enablements", newCriteria(EWayland | EX11 | EDBus | EPulse), ptc.et != User && ptc.et != Process},
 			}
 
 			for _, tc := range testCases {
@@ -65,15 +65,4 @@ func (ptc tcOp) test(t *testing.T, gotOps []Op, wantOps []Op, fn string) {
 	}
 }
 
-func newCriteria(labels ...Enablement) *Criteria {
-	ec := new(Criteria)
-	if len(labels) == 0 {
-		return ec
-	}
-
-	ec.Enablements = new(Enablements)
-	for _, e := range labels {
-		ec.Set(e)
-	}
-	return ec
-}
+func newCriteria(e Enablement) *Criteria { return (*Criteria)(&e) }

system/op_test.go

@@ -37,15 +37,17 @@ func TestTypeString(t *testing.T) {
 		{system.EX11, system.EX11.String()},
 		{system.EDBus, system.EDBus.String()},
 		{system.EPulse, system.EPulse.String()},
-		{system.User, "User"},
-		{system.Process, "Process"},
+		{system.User, "user"},
+		{system.Process, "process"},
+		{system.User | system.Process, "user, process"},
+		{system.EWayland | system.User | system.Process, "wayland, user, process"},
+		{system.EX11 | system.Process, "x11, process"},
 	}
 
 	for _, tc := range testCases {
 		t.Run("label type string "+tc.want, func(t *testing.T) {
 			if got := system.TypeString(tc.e); got != tc.want {
-				t.Errorf("TypeString(%d) = %v, want %v",
-					tc.e,
+				t.Errorf("TypeString: %q, want %q",
 					got, tc.want)
 			}
 		})