From 6f719bc3c19d56279d2d76d3bcf8fb66ace2d2fb Mon Sep 17 00:00:00 2001 From: Ophestra Date: Tue, 2 Sep 2025 04:54:34 +0900 Subject: [PATCH] system: update doc commands and remove mutex The mutex is not really doing anything, none of these methods make sense when called concurrently anyway. The copylocks analysis is still satisfied by the noCopy struct. Signed-off-by: Ophestra --- internal/app/app_nixos_linux_test.go | 3 +- internal/app/app_pd_linux_test.go | 5 +- internal/app/app_test.go | 2 +- internal/app/export_test.go | 9 ++- internal/app/process.go | 2 +- internal/app/seal.go | 7 ++- system/acl.go | 36 ++++++------ system/acl_test.go | 10 ++-- system/dbus.go | 52 +++++++++-------- system/enablement.go | 2 +- system/link.go | 29 +++++----- system/mkdir.go | 40 +++++--------- system/mkdir_test.go | 10 ++-- system/{op.go => system.go} | 80 +++++++++++++++------------ system/{op_test.go => system_test.go} | 28 +++++----- system/tmpfiles.go | 30 +++++----- system/tmpfiles_test.go | 14 ++--- system/wayland.go | 32 +++++------ system/xhost.go | 23 ++++---- system/xhost_test.go | 6 +- 20 files changed, 210 insertions(+), 210 deletions(-) rename system/{op.go => system.go} (56%) rename system/{op_test.go => system_test.go} (81%) diff --git a/internal/app/app_nixos_linux_test.go b/internal/app/app_nixos_linux_test.go index e2aa5a4..b6f5e44 100644 --- a/internal/app/app_nixos_linux_test.go +++ b/internal/app/app_nixos_linux_test.go @@ -1,6 +1,7 @@ package app_test import ( + "context" "syscall" "hakurei.app/container" @@ -74,7 +75,7 @@ var testCasesNixos = []sealTestCase{ 0x4c, 0xf0, 0x73, 0xbd, 0xb4, 0x6e, 0xb5, 0xc1, }, - system.New(1000001). + system.New(context.TODO(), 1000001). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/1", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/1", acl.Read, acl.Write, acl.Execute). diff --git a/internal/app/app_pd_linux_test.go b/internal/app/app_pd_linux_test.go index 384f5a0..7bc9473 100644 --- a/internal/app/app_pd_linux_test.go +++ b/internal/app/app_pd_linux_test.go @@ -1,6 +1,7 @@ package app_test import ( + "context" "os" "syscall" @@ -23,7 +24,7 @@ var testCasesPd = []sealTestCase{ 0xbd, 0x01, 0x78, 0x0e, 0xb9, 0xa6, 0x07, 0xac, }, - system.New(1000000). + system.New(context.TODO(), 1000000). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/0", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/0", acl.Read, acl.Write, acl.Execute). @@ -115,7 +116,7 @@ var testCasesPd = []sealTestCase{ 0x82, 0xd4, 0x13, 0x36, 0x9b, 0x64, 0xce, 0x7c, }, - system.New(1000009). + system.New(context.TODO(), 1000009). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/9", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/9", acl.Read, acl.Write, acl.Execute). diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 8b96cfb..ab94b1c 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -30,7 +30,7 @@ func TestApp(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - a := app.NewWithID(tc.id, tc.os) + a := app.NewWithID(t.Context(), tc.id, tc.os) var ( gotSys *system.I gotContainer *container.Params diff --git a/internal/app/export_test.go b/internal/app/export_test.go index b7ee7f7..3e5db9f 100644 --- a/internal/app/export_test.go +++ b/internal/app/export_test.go @@ -1,17 +1,16 @@ package app import ( + "context" + "hakurei.app/container" "hakurei.app/internal/app/state" "hakurei.app/internal/sys" "hakurei.app/system" ) -func NewWithID(id state.ID, os sys.State) *App { - a := new(App) - a.id = newID(&id) - a.sys = os - return a +func NewWithID(ctx context.Context, id state.ID, os sys.State) *App { + return &App{id: newID(&id), sys: os, ctx: ctx} } func AppIParams(a *App, seal *Outcome) (*system.I, *container.Params) { diff --git a/internal/app/process.go b/internal/app/process.go index 4435569..b89f0fc 100644 --- a/internal/app/process.go +++ b/internal/app/process.go @@ -61,7 +61,7 @@ func (seal *Outcome) Run(rs *RunState) error { // read comp value early to allow for early failure hsuPath := internal.MustHsuPath() - if err := seal.sys.Commit(seal.ctx); err != nil { + if err := seal.sys.Commit(); err != nil { return err } store := state.NewMulti(seal.runDirPath.String()) diff --git a/internal/app/seal.go b/internal/app/seal.go index ec42dc2..047c3a7 100644 --- a/internal/app/seal.go +++ b/internal/app/seal.go @@ -146,8 +146,11 @@ type hsuUser struct { } func (seal *Outcome) finalise(ctx context.Context, sys sys.State, config *hst.Config) error { + if ctx == nil { + panic("invalid call to finalise") + } if seal.ctx != nil { - panic("finalise called twice") + panic("attempting to finalise twice") } seal.ctx = ctx @@ -306,7 +309,7 @@ func (seal *Outcome) finalise(ctx context.Context, sys sys.State, config *hst.Co share := &shareHost{seal: seal, sc: sys.Paths()} seal.runDirPath = share.sc.RunDirPath - seal.sys = system.New(seal.user.uid.unwrap()) + seal.sys = system.New(seal.ctx, seal.user.uid.unwrap()) seal.sys.Ensure(share.sc.SharePath.String(), 0711) { diff --git a/system/acl.go b/system/acl.go index 82777b9..0432056 100644 --- a/system/acl.go +++ b/system/acl.go @@ -9,37 +9,33 @@ import ( "hakurei.app/system/acl" ) -// UpdatePerm appends an ephemeral acl update Op. +// UpdatePerm appends [ACLUpdateOp] to [I] with the [Process] criteria. func (sys *I) UpdatePerm(path string, perms ...acl.Perm) *I { sys.UpdatePermType(Process, path, perms...) - return sys } -// UpdatePermType appends an acl update Op. +// UpdatePermType appends [ACLUpdateOp] to [I]. func (sys *I) UpdatePermType(et Enablement, path string, perms ...acl.Perm) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &ACL{et, path, perms}) - + sys.ops = append(sys.ops, &ACLUpdateOp{et, path, perms}) return sys } -type ACL struct { +// ACLUpdateOp maintains [acl.Perms] on a file until its [Enablement] is no longer satisfied. +type ACLUpdateOp struct { et Enablement path string perms acl.Perms } -func (a *ACL) Type() Enablement { return a.et } +func (a *ACLUpdateOp) Type() Enablement { return a.et } -func (a *ACL) apply(sys *I) error { +func (a *ACLUpdateOp) apply(sys *I) error { msg.Verbose("applying ACL", a) return newOpError("acl", acl.Update(a.path, sys.uid, a.perms...), false) } -func (a *ACL) revert(sys *I, ec *Criteria) error { +func (a *ACLUpdateOp) revert(sys *I, ec *Criteria) error { if ec.hasType(a) { msg.Verbose("stripping ACL", a) err := acl.Update(a.path, sys.uid) @@ -55,17 +51,17 @@ func (a *ACL) revert(sys *I, ec *Criteria) error { } } -func (a *ACL) Is(o Op) bool { - a0, ok := o.(*ACL) - return ok && a0 != nil && - a.et == a0.et && - a.path == a0.path && - slices.Equal(a.perms, a0.perms) +func (a *ACLUpdateOp) Is(o Op) bool { + target, ok := o.(*ACLUpdateOp) + return ok && a != nil && target != nil && + a.et == target.et && + a.path == target.path && + slices.Equal(a.perms, target.perms) } -func (a *ACL) Path() string { return a.path } +func (a *ACLUpdateOp) Path() string { return a.path } -func (a *ACL) String() string { +func (a *ACLUpdateOp) String() string { return fmt.Sprintf("%s type: %s path: %q", a.perms, TypeString(a.et), a.path) } diff --git a/system/acl_test.go b/system/acl_test.go index 886a62e..6b050bb 100644 --- a/system/acl_test.go +++ b/system/acl_test.go @@ -18,9 +18,9 @@ func TestUpdatePerm(t *testing.T) { for _, tc := range testCases { t.Run(tc.path+permSubTestSuffix(tc.perms), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.UpdatePerm(tc.path, tc.perms...) - (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACL{Process, tc.path, tc.perms}}, "UpdatePerm") + (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACLUpdateOp{Process, tc.path, tc.perms}}, "UpdatePerm") }) } } @@ -40,9 +40,9 @@ func TestUpdatePermType(t *testing.T) { for _, tc := range testCases { t.Run(tc.path+"_"+TypeString(tc.et)+permSubTestSuffix(tc.perms), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.UpdatePermType(tc.et, tc.path, tc.perms...) - tc.test(t, sys.ops, []Op{&ACL{tc.et, tc.path, tc.perms}}, "UpdatePermType") + tc.test(t, sys.ops, []Op{&ACLUpdateOp{tc.et, tc.path, tc.perms}}, "UpdatePermType") }) } } @@ -65,7 +65,7 @@ func TestACLString(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - a := &ACL{et: tc.et, perms: tc.perms, path: container.Nonexistent} + a := &ACLUpdateOp{et: tc.et, perms: tc.perms, path: container.Nonexistent} if got := a.String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) diff --git a/system/dbus.go b/system/dbus.go index b891089..d1ff06a 100644 --- a/system/dbus.go +++ b/system/dbus.go @@ -17,6 +17,7 @@ var ( ErrDBusConfig = errors.New("dbus config not supplied") ) +// MustProxyDBus calls ProxyDBus and panics if an error is returned. func (sys *I) MustProxyDBus(sessionPath string, session *dbus.Config, systemPath string, system *dbus.Config) *I { if _, err := sys.ProxyDBus(session, system, sessionPath, systemPath); err != nil { panic(err.Error()) @@ -25,8 +26,9 @@ func (sys *I) MustProxyDBus(sessionPath string, session *dbus.Config, systemPath } } +// ProxyDBus finalises configuration and appends [DBusProxyOp] to [I]. func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath string) (func(), error) { - d := new(DBus) + d := new(DBusProxyOp) // session bus is required as otherwise this is effectively a very expensive noop if session == nil { @@ -39,7 +41,7 @@ func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath st d.sessionBus[0], d.systemBus[0] = dbus.Address() d.sessionBus[1], d.systemBus[1] = sessionPath, systemPath - d.out = &scanToFmsg{msg: new(strings.Builder)} + d.out = &linePrefixWriter{println: log.Println, prefix: "(dbus) ", msg: new(strings.Builder)} if final, err := dbus.Finalise(d.sessionBus, d.systemBus, session, system); err != nil { if errors.Is(err, syscall.EINVAL) { return nil, newOpErrorMessage("dbus", err, @@ -65,20 +67,22 @@ func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath st return d.out.Dump, nil } -type DBus struct { +// DBusProxyOp starts xdg-dbus-proxy via [dbus] and terminates it on revert. +// This [Op] is always [Process] scoped. +type DBusProxyOp struct { proxy *dbus.Proxy // populated during apply final *dbus.Final - out *scanToFmsg + out *linePrefixWriter // whether system bus proxy is enabled system bool sessionBus, systemBus dbus.ProxyPair } -func (d *DBus) Type() Enablement { return Process } +func (d *DBusProxyOp) Type() Enablement { return Process } -func (d *DBus) apply(sys *I) error { +func (d *DBusProxyOp) apply(sys *I) error { msg.Verbosef("session bus proxy on %q for upstream %q", d.sessionBus[1], d.sessionBus[0]) if d.system { msg.Verbosef("system bus proxy on %q for upstream %q", d.systemBus[1], d.systemBus[0]) @@ -94,7 +98,7 @@ func (d *DBus) apply(sys *I) error { return nil } -func (d *DBus) revert(*I, *Criteria) error { +func (d *DBusProxyOp) revert(*I, *Criteria) error { // criteria ignored here since dbus is always process-scoped msg.Verbose("terminating message bus proxy") d.proxy.Close() @@ -108,30 +112,34 @@ func (d *DBus) revert(*I, *Criteria) error { fmt.Sprintf("message bus proxy error: %v", err), true) } -func (d *DBus) Is(o Op) bool { - d0, ok := o.(*DBus) - return ok && d0 != nil && - ((d.proxy == nil && d0.proxy == nil) || - (d.proxy != nil && d0.proxy != nil && d.proxy.String() == d0.proxy.String())) +func (d *DBusProxyOp) Is(o Op) bool { + target, ok := o.(*DBusProxyOp) + return ok && d != nil && target != nil && + ((d.proxy == nil && target.proxy == nil) || + (d.proxy != nil && target.proxy != nil && + d.proxy.String() == target.proxy.String())) } -func (d *DBus) Path() string { return "(dbus proxy)" } -func (d *DBus) String() string { return d.proxy.String() } +func (d *DBusProxyOp) Path() string { return "(dbus proxy)" } +func (d *DBusProxyOp) String() string { return d.proxy.String() } -type scanToFmsg struct { - msg *strings.Builder - msgbuf []string +// linePrefixWriter calls println with a prefix for every line written. +type linePrefixWriter struct { + prefix string + println func(v ...any) + msg *strings.Builder + msgbuf []string mu sync.RWMutex } -func (s *scanToFmsg) Write(p []byte) (n int, err error) { +func (s *linePrefixWriter) Write(p []byte) (n int, err error) { s.mu.Lock() defer s.mu.Unlock() return s.write(p, 0) } -func (s *scanToFmsg) write(p []byte, a int) (int, error) { +func (s *linePrefixWriter) write(p []byte, a int) (int, error) { if i := bytes.IndexByte(p, '\n'); i == -1 { n, _ := s.msg.Write(p) return a + n, nil @@ -141,7 +149,7 @@ func (s *scanToFmsg) write(p []byte, a int) (int, error) { // allow container init messages through v := s.msg.String() if strings.HasPrefix(v, "init: ") { - log.Println("(dbus) " + v) + s.println(s.prefix + v) } else { s.msgbuf = append(s.msgbuf, v) } @@ -151,10 +159,10 @@ func (s *scanToFmsg) write(p []byte, a int) (int, error) { } } -func (s *scanToFmsg) Dump() { +func (s *linePrefixWriter) Dump() { s.mu.RLock() for _, m := range s.msgbuf { - log.Println("(dbus) " + m) + s.println(s.prefix + m) } s.mu.RUnlock() } diff --git a/system/enablement.go b/system/enablement.go index f3e74e0..9c1342d 100644 --- a/system/enablement.go +++ b/system/enablement.go @@ -5,7 +5,7 @@ import ( "strings" ) -// Enablement represents optional system resources. +// Enablement represents an optional host service to export to the target user. type Enablement byte const ( diff --git a/system/link.go b/system/link.go index 022ee4e..4ba74ff 100644 --- a/system/link.go +++ b/system/link.go @@ -5,32 +5,29 @@ import ( "os" ) -// Link registers an Op that links dst to src. +// Link appends [HardlinkOp] to [I] the [Process] criteria. func (sys *I) Link(oldname, newname string) *I { return sys.LinkFileType(Process, oldname, newname) } -// LinkFileType registers a file linking Op labelled with type et. +// LinkFileType appends [HardlinkOp] to [I]. func (sys *I) LinkFileType(et Enablement, oldname, newname string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Hardlink{et, newname, oldname}) - + sys.ops = append(sys.ops, &HardlinkOp{et, newname, oldname}) return sys } -type Hardlink struct { +// HardlinkOp maintains a hardlink until its [Enablement] is no longer satisfied. +type HardlinkOp struct { et Enablement dst, src string } -func (l *Hardlink) Type() Enablement { return l.et } +func (l *HardlinkOp) Type() Enablement { return l.et } -func (l *Hardlink) apply(*I) error { +func (l *HardlinkOp) apply(*I) error { msg.Verbose("linking", l) return newOpError("hardlink", os.Link(l.src, l.dst), false) } -func (l *Hardlink) revert(_ *I, ec *Criteria) error { +func (l *HardlinkOp) revert(_ *I, ec *Criteria) error { if ec.hasType(l) { msg.Verbosef("removing hard link %q", l.dst) return newOpError("hardlink", os.Remove(l.dst), true) @@ -40,6 +37,10 @@ func (l *Hardlink) revert(_ *I, ec *Criteria) error { } } -func (l *Hardlink) Is(o Op) bool { l0, ok := o.(*Hardlink); return ok && l0 != nil && *l == *l0 } -func (l *Hardlink) Path() string { return l.src } -func (l *Hardlink) String() string { return fmt.Sprintf("%q from %q", l.dst, l.src) } +func (l *HardlinkOp) Is(o Op) bool { + target, ok := o.(*HardlinkOp) + return ok && l != nil && target != nil && *l == *target +} + +func (l *HardlinkOp) Path() string { return l.src } +func (l *HardlinkOp) String() string { return fmt.Sprintf("%q from %q", l.dst, l.src) } diff --git a/system/mkdir.go b/system/mkdir.go index 51fc274..4fba4af 100644 --- a/system/mkdir.go +++ b/system/mkdir.go @@ -6,38 +6,30 @@ import ( "os" ) -// Ensure the existence and mode of a directory. +// Ensure appends [MkdirOp] to [I] with its [Enablement] ignored. func (sys *I) Ensure(name string, perm os.FileMode) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Mkdir{User, name, perm, false}) - + sys.ops = append(sys.ops, &MkdirOp{User, name, perm, false}) return sys } -// Ephemeral ensures the temporary existence and mode of a directory through the life of et. +// Ephemeral appends an ephemeral [MkdirOp] to [I]. func (sys *I) Ephemeral(et Enablement, name string, perm os.FileMode) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Mkdir{et, name, perm, true}) - + sys.ops = append(sys.ops, &MkdirOp{et, name, perm, true}) return sys } -type Mkdir struct { +// MkdirOp ensures the existence of a directory. +// For ephemeral, the directory is destroyed once [Enablement] is no longer satisfied. +type MkdirOp struct { et Enablement path string perm os.FileMode ephemeral bool } -func (m *Mkdir) Type() Enablement { - return m.et -} +func (m *MkdirOp) Type() Enablement { return m.et } -func (m *Mkdir) apply(*I) error { +func (m *MkdirOp) apply(*I) error { msg.Verbose("ensuring directory", m) // create directory @@ -52,7 +44,7 @@ func (m *Mkdir) apply(*I) error { } } -func (m *Mkdir) revert(_ *I, ec *Criteria) error { +func (m *MkdirOp) revert(_ *I, ec *Criteria) error { if !m.ephemeral { // skip non-ephemeral dir and do not log anything return nil @@ -67,16 +59,14 @@ func (m *Mkdir) revert(_ *I, ec *Criteria) error { } } -func (m *Mkdir) Is(o Op) bool { - m0, ok := o.(*Mkdir) - return ok && m0 != nil && *m == *m0 +func (m *MkdirOp) Is(o Op) bool { + target, ok := o.(*MkdirOp) + return ok && m != nil && target != nil && *m == *target } -func (m *Mkdir) Path() string { - return m.path -} +func (m *MkdirOp) Path() string { return m.path } -func (m *Mkdir) String() string { +func (m *MkdirOp) String() string { t := "ensure" if m.ephemeral { t = TypeString(m.Type()) diff --git a/system/mkdir_test.go b/system/mkdir_test.go index 706b3d1..8fc12a1 100644 --- a/system/mkdir_test.go +++ b/system/mkdir_test.go @@ -19,9 +19,9 @@ func TestEnsure(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name+"_"+tc.perm.String(), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Ensure(tc.name, tc.perm) - (&tcOp{User, tc.name}).test(t, sys.ops, []Op{&Mkdir{User, tc.name, tc.perm, false}}, "Ensure") + (&tcOp{User, tc.name}).test(t, sys.ops, []Op{&MkdirOp{User, tc.name, tc.perm, false}}, "Ensure") }) } } @@ -36,9 +36,9 @@ func TestEphemeral(t *testing.T) { } for _, tc := range testCases { t.Run(tc.path+"_"+tc.perm.String()+"_"+TypeString(tc.et), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Ephemeral(tc.et, tc.path, tc.perm) - tc.test(t, sys.ops, []Op{&Mkdir{tc.et, tc.path, tc.perm, true}}, "Ephemeral") + tc.test(t, sys.ops, []Op{&MkdirOp{tc.et, tc.path, tc.perm, true}}, "Ephemeral") }) } } @@ -60,7 +60,7 @@ func TestMkdirString(t *testing.T) { } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - m := &Mkdir{ + m := &MkdirOp{ et: tc.et, path: container.Nonexistent, perm: 0701, diff --git a/system/op.go b/system/system.go similarity index 56% rename from system/op.go rename to system/system.go index 25f3fa2..e8fa43f 100644 --- a/system/op.go +++ b/system/system.go @@ -1,4 +1,4 @@ -// Package system provides tools for safely interacting with the operating system. +// Package system provides helpers to apply and revert groups of operations to the system. package system import ( @@ -6,11 +6,10 @@ import ( "errors" "log" "strings" - "sync" ) const ( - // User type is reverted at final launcher exit. + // User type is reverted at final instance exit. User = EM << iota // Process type is unconditionally reverted on exit. Process @@ -32,12 +31,10 @@ func (ec *Criteria) hasType(o Op) bool { // Op is a reversible system operation. type Op interface { - // Type returns Op's enablement type. + // Type returns [Op]'s enablement type, for matching a revert criteria. Type() Enablement - // apply the Op apply(sys *I) error - // revert reverses the Op if criteria is met revert(sys *I, ec *Criteria) error Is(o Op) bool @@ -45,7 +42,7 @@ type Op interface { String() string } -// TypeString returns the string representation of a type stored as an [Enablement]. +// TypeString extends [Enablement.String] to support [User] and [Process]. func TypeString(e Enablement) string { switch e { case User: @@ -68,35 +65,39 @@ func TypeString(e Enablement) string { } } -// New initialises sys with no-op verbose functions. -func New(uid int) (sys *I) { - sys = new(I) - sys.uid = uid - return +// New returns the address of a new [I] targeting uid. +func New(ctx context.Context, uid int) (sys *I) { + if ctx == nil || uid < 0 { + panic("invalid call to New") + } + return &I{ctx: ctx, uid: uid} } -// An I provides indirect bulk operating system interaction. I must not be copied. +// An I provides deferred operating system interaction. [I] must not be copied. +// Methods of [I] must not be used concurrently. type I struct { + _ noCopy + uid int ops []Op ctx context.Context - // whether sys has been reverted - state bool - - lock sync.Mutex + // the behaviour of Commit is only defined for up to one call + committed bool + // the behaviour of Revert is only defined for up to one call + reverted bool } func (sys *I) UID() int { return sys.uid } -// Equal returns whether all [Op] instances held by v is identical to that of sys. -func (sys *I) Equal(v *I) bool { - if v == nil || sys.uid != v.uid || len(sys.ops) != len(v.ops) { +// Equal returns whether all [Op] instances held by sys matches that of target. +func (sys *I) Equal(target *I) bool { + if target == nil || sys.uid != target.uid || len(sys.ops) != len(target.ops) { return false } for i, o := range sys.ops { - if !o.Is(v.ops[i]) { + if !o.Is(target.ops[i]) { return false } } @@ -104,18 +105,15 @@ func (sys *I) Equal(v *I) bool { return true } -// Commit applies all [Op] held by [I] and reverts successful [Op] on first error encountered. +// Commit applies all [Op] held by [I] and reverts all successful [Op] on first error encountered. // Commit must not be called more than once. -func (sys *I) Commit(ctx context.Context) error { - sys.lock.Lock() - defer sys.lock.Unlock() - - if sys.ctx != nil { - panic("sys instance committed twice") +func (sys *I) Commit() error { + if sys.committed { + panic("attempting to commit twice") } - sys.ctx = ctx + sys.committed = true - sp := New(sys.uid) + sp := New(sys.ctx, sys.uid) sp.ops = make([]Op, 0, len(sys.ops)) // prevent copies during commits defer func() { // sp is set to nil when all ops are applied @@ -144,13 +142,10 @@ func (sys *I) Commit(ctx context.Context) error { // Revert reverts all [Op] meeting [Criteria] held by [I]. func (sys *I) Revert(ec *Criteria) error { - sys.lock.Lock() - defer sys.lock.Unlock() - - if sys.state { - panic("sys instance reverted twice") + if sys.reverted { + panic("attempting to revert twice") } - sys.state = true + sys.reverted = true // collect errors errs := make([]error, len(sys.ops)) @@ -162,3 +157,16 @@ func (sys *I) Revert(ec *Criteria) error { // errors.Join filters nils return errors.Join(errs...) } + +// noCopy may be added to structs which must not be copied +// after the first use. +// +// See https://golang.org/issues/8005#issuecomment-190753527 +// for details. +// +// Note that it must not be embedded, due to the Lock and Unlock methods. +type noCopy struct{} + +// Lock is a no-op used by -copylocks checker from `go vet`. +func (*noCopy) Lock() {} +func (*noCopy) Unlock() {} diff --git a/system/op_test.go b/system/system_test.go similarity index 81% rename from system/op_test.go rename to system/system_test.go index d11bcfa..323e35b 100644 --- a/system/op_test.go +++ b/system/system_test.go @@ -19,7 +19,7 @@ func TestNew(t *testing.T) { for _, tc := range testCases { t.Run("sys initialised with uid "+strconv.Itoa(tc.uid), func(t *testing.T) { - if got := system.New(tc.uid); got.UID() != tc.uid { + if got := system.New(t.Context(), tc.uid); got.UID() != tc.uid { t.Errorf("New(%d) uid = %d, want %d", tc.uid, got.UID(), tc.uid) @@ -63,57 +63,57 @@ func TestI_Equal(t *testing.T) { }{ { "simple UID", - system.New(150), - system.New(150), + system.New(t.Context(), 150), + system.New(t.Context(), 150), true, }, { "simple UID differ", - system.New(150), - system.New(151), + system.New(t.Context(), 150), + system.New(t.Context(), 151), false, }, { "simple UID nil", - system.New(150), + system.New(t.Context(), 150), nil, false, }, { "op length mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op value mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0644), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op type mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). CopyFile(new([]byte), "/home/ophestra/xdg/config/pulse/cookie", 0, 256), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op equals", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), true, diff --git a/system/tmpfiles.go b/system/tmpfiles.go index 44d170f..2850ba2 100644 --- a/system/tmpfiles.go +++ b/system/tmpfiles.go @@ -9,20 +9,16 @@ import ( "syscall" ) -// CopyFile registers an Op that copies from src. -// A buffer is initialised with size cap and the Op faults if bytes read exceed n. +// CopyFile appends [TmpfileOp] to [I]. func (sys *I) CopyFile(payload *[]byte, src string, cap int, n int64) *I { buf := new(bytes.Buffer) buf.Grow(cap) - - sys.lock.Lock() - sys.ops = append(sys.ops, &Tmpfile{payload, src, n, buf}) - sys.lock.Unlock() - + sys.ops = append(sys.ops, &TmpfileOp{payload, src, n, buf}) return sys } -type Tmpfile struct { +// TmpfileOp reads up to n bytes from src and writes the resulting byte slice to payload. +type TmpfileOp struct { payload *[]byte src string @@ -30,8 +26,8 @@ type Tmpfile struct { buf *bytes.Buffer } -func (t *Tmpfile) Type() Enablement { return Process } -func (t *Tmpfile) apply(*I) error { +func (t *TmpfileOp) Type() Enablement { return Process } +func (t *TmpfileOp) apply(*I) error { msg.Verbose("copying", t) if t.payload == nil { @@ -59,12 +55,12 @@ func (t *Tmpfile) apply(*I) error { *t.payload = t.buf.Bytes() return nil } -func (t *Tmpfile) revert(*I, *Criteria) error { t.buf.Reset(); return nil } +func (t *TmpfileOp) revert(*I, *Criteria) error { t.buf.Reset(); return nil } -func (t *Tmpfile) Is(o Op) bool { - t0, ok := o.(*Tmpfile) - return ok && t0 != nil && - t.src == t0.src && t.n == t0.n +func (t *TmpfileOp) Is(o Op) bool { + target, ok := o.(*TmpfileOp) + return ok && t != nil && target != nil && + t.src == target.src && t.n == target.n } -func (t *Tmpfile) Path() string { return t.src } -func (t *Tmpfile) String() string { return fmt.Sprintf("up to %d bytes from %q", t.n, t.src) } +func (t *TmpfileOp) Path() string { return t.src } +func (t *TmpfileOp) String() string { return fmt.Sprintf("up to %d bytes from %q", t.n, t.src) } diff --git a/system/tmpfiles_test.go b/system/tmpfiles_test.go index 27a21a0..1ed07c6 100644 --- a/system/tmpfiles_test.go +++ b/system/tmpfiles_test.go @@ -15,10 +15,10 @@ func TestCopyFile(t *testing.T) { } for _, tc := range testCases { t.Run("copy file "+tc.path+" with cap = "+strconv.Itoa(tc.cap)+" n = "+strconv.Itoa(int(tc.n)), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.CopyFile(new([]byte), tc.path, tc.cap, tc.n) tc.test(t, sys.ops, []Op{ - &Tmpfile{nil, tc.path, tc.n, nil}, + &TmpfileOp{nil, tc.path, tc.n, nil}, }, "CopyFile") }) } @@ -33,10 +33,10 @@ func TestLink(t *testing.T) { } for _, tc := range testCases { t.Run("link file "+tc.dst+" from "+tc.src, func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Link(tc.src, tc.dst) (&tcOp{Process, tc.src}).test(t, sys.ops, []Op{ - &Hardlink{Process, tc.dst, tc.src}, + &HardlinkOp{Process, tc.dst, tc.src}, }, "Link") }) } @@ -52,10 +52,10 @@ func TestLinkFileType(t *testing.T) { } for _, tc := range testCases { t.Run("link file "+tc.dst+" from "+tc.path+" with type "+TypeString(tc.et), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.LinkFileType(tc.et, tc.path, tc.dst) tc.test(t, sys.ops, []Op{ - &Hardlink{tc.et, tc.dst, tc.path}, + &HardlinkOp{tc.et, tc.dst, tc.path}, }, "LinkFileType") }) } @@ -73,7 +73,7 @@ func TestTmpfile_String(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - if got := (&Tmpfile{src: tc.src, n: tc.n}).String(); got != tc.want { + if got := (&TmpfileOp{src: tc.src, n: tc.n}).String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) } }) diff --git a/system/wayland.go b/system/wayland.go index f08f58b..999feee 100644 --- a/system/wayland.go +++ b/system/wayland.go @@ -9,17 +9,16 @@ import ( "hakurei.app/system/wayland" ) -// Wayland sets up a wayland socket with a security context attached. +// Wayland appends [WaylandOp] to [I]. func (sys *I) Wayland(syncFd **os.File, dst, src, appID, instanceID string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Wayland{syncFd, dst, src, appID, instanceID, wayland.Conn{}}) - + sys.ops = append(sys.ops, &WaylandOp{syncFd, dst, src, appID, instanceID, wayland.Conn{}}) return sys } -type Wayland struct { +// WaylandOp maintains a wayland socket with security-context-v1 attached via [wayland]. +// The socket stops accepting connections once the pipe referred to by sync is closed. +// The socket is pathname only and is destroyed on revert. +type WaylandOp struct { sync **os.File dst, src string appID, instanceID string @@ -27,9 +26,9 @@ type Wayland struct { conn wayland.Conn } -func (w *Wayland) Type() Enablement { return Process } +func (w *WaylandOp) Type() Enablement { return Process } -func (w *Wayland) apply(sys *I) error { +func (w *WaylandOp) apply(sys *I) error { if w.sync == nil { // this is a misuse of the API; do not return a wrapped error return errors.New("invalid sync") @@ -59,7 +58,7 @@ func (w *Wayland) apply(sys *I) error { } } -func (w *Wayland) revert(_ *I, ec *Criteria) error { +func (w *WaylandOp) revert(_ *I, ec *Criteria) error { if ec.hasType(w) { msg.Verbosef("removing wayland socket on %q", w.dst) if err := os.Remove(w.dst); err != nil && !errors.Is(err, os.ErrNotExist) { @@ -74,11 +73,12 @@ func (w *Wayland) revert(_ *I, ec *Criteria) error { } } -func (w *Wayland) Is(o Op) bool { - w0, ok := o.(*Wayland) - return ok && w.dst == w0.dst && w.src == w0.src && - w.appID == w0.appID && w.instanceID == w0.instanceID +func (w *WaylandOp) Is(o Op) bool { + target, ok := o.(*WaylandOp) + return ok && w != nil && target != nil && + w.dst == target.dst && w.src == target.src && + w.appID == target.appID && w.instanceID == target.instanceID } -func (w *Wayland) Path() string { return w.dst } -func (w *Wayland) String() string { return fmt.Sprintf("wayland socket at %q", w.dst) } +func (w *WaylandOp) Path() string { return w.dst } +func (w *WaylandOp) String() string { return fmt.Sprintf("wayland socket at %q", w.dst) } diff --git a/system/xhost.go b/system/xhost.go index 27b2fc6..2ed1acf 100644 --- a/system/xhost.go +++ b/system/xhost.go @@ -4,27 +4,24 @@ import ( "hakurei.app/system/internal/xcb" ) -// ChangeHosts appends an X11 ChangeHosts command Op. +// ChangeHosts appends [XHostOp] to [I]. func (sys *I) ChangeHosts(username string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, XHost(username)) - + sys.ops = append(sys.ops, XHostOp(username)) return sys } -type XHost string +// XHostOp inserts the target user into X11 hosts and deletes it once its [Enablement] is no longer satisfied. +type XHostOp string -func (x XHost) Type() Enablement { return EX11 } +func (x XHostOp) Type() Enablement { return EX11 } -func (x XHost) apply(*I) error { +func (x XHostOp) apply(*I) error { msg.Verbosef("inserting entry %s to X11", x) return newOpError("xhost", xcb.ChangeHosts(xcb.HostModeInsert, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), false) } -func (x XHost) revert(_ *I, ec *Criteria) error { +func (x XHostOp) revert(_ *I, ec *Criteria) error { if ec.hasType(x) { msg.Verbosef("deleting entry %s from X11", x) return newOpError("xhost", @@ -35,6 +32,6 @@ func (x XHost) revert(_ *I, ec *Criteria) error { } } -func (x XHost) Is(o Op) bool { x0, ok := o.(XHost); return ok && x == x0 } -func (x XHost) Path() string { return string(x) } -func (x XHost) String() string { return string("SI:localuser:" + x) } +func (x XHostOp) Is(o Op) bool { target, ok := o.(XHostOp); return ok && x == target } +func (x XHostOp) Path() string { return string(x) } +func (x XHostOp) String() string { return string("SI:localuser:" + x) } diff --git a/system/xhost_test.go b/system/xhost_test.go index 4b36443..df0e816 100644 --- a/system/xhost_test.go +++ b/system/xhost_test.go @@ -8,10 +8,10 @@ func TestChangeHosts(t *testing.T) { testCases := []string{"chronos", "keyring", "cat", "kbd", "yonah"} for _, tc := range testCases { t.Run("append ChangeHosts operation for "+tc, func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.ChangeHosts(tc) (&tcOp{EX11, tc}).test(t, sys.ops, []Op{ - XHost(tc), + XHostOp(tc), }, "ChangeHosts") }) } @@ -26,7 +26,7 @@ func TestXHost_String(t *testing.T) { } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - if got := XHost(tc.username).String(); got != tc.want { + if got := XHostOp(tc.username).String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) } })