From e1e46504a1939aff927ae47508b0523a7305fb18 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sun, 11 Jan 2026 04:22:04 +0900 Subject: [PATCH] container/check: return error backed by string type The struct turned out not necessary during initial implementation but was not unwrapped into its single string field. This change replaces it with the underlying string and removes the indirection. Signed-off-by: Ophestra --- container/check/absolute.go | 19 +++++++++++-------- container/check/absolute_test.go | 14 +++++++------- container/errors.go | 2 +- container/errors_test.go | 2 +- container/initoverlay_test.go | 10 ++++++++-- container/initsymlink.go | 2 +- container/initsymlink_test.go | 2 +- internal/outcome/sppulse_test.go | 8 ++++---- ldd/ldd_test.go | 2 +- 9 files changed, 35 insertions(+), 26 deletions(-) diff --git a/container/check/absolute.go b/container/check/absolute.go index 26f0def..d17d5e1 100644 --- a/container/check/absolute.go +++ b/container/check/absolute.go @@ -13,15 +13,18 @@ import ( ) // AbsoluteError is returned by [NewAbs] and holds the invalid pathname. -type AbsoluteError struct{ Pathname string } +type AbsoluteError string -func (e *AbsoluteError) Error() string { return fmt.Sprintf("path %q is not absolute", e.Pathname) } -func (e *AbsoluteError) Is(target error) bool { - var ce *AbsoluteError +func (e AbsoluteError) Error() string { + return fmt.Sprintf("path %q is not absolute", string(e)) +} + +func (e AbsoluteError) Is(target error) bool { + var ce AbsoluteError if !errors.As(target, &ce) { return errors.Is(target, syscall.EINVAL) } - return *e == *ce + return e == ce } // Absolute holds a pathname checked to be absolute. @@ -59,7 +62,7 @@ func (a *Absolute) Is(v *Absolute) bool { // NewAbs checks pathname and returns a new [Absolute] if pathname is absolute. func NewAbs(pathname string) (*Absolute, error) { if !path.IsAbs(pathname) { - return nil, &AbsoluteError{pathname} + return nil, AbsoluteError(pathname) } return unsafeAbs(pathname), nil } @@ -90,7 +93,7 @@ func (a *Absolute) GobEncode() ([]byte, error) { func (a *Absolute) GobDecode(data []byte) error { pathname := string(data) if !path.IsAbs(pathname) { - return &AbsoluteError{pathname} + return AbsoluteError(pathname) } a.pathname = unique.Make(pathname) return nil @@ -108,7 +111,7 @@ func (a *Absolute) UnmarshalJSON(data []byte) error { return err } if !path.IsAbs(pathname) { - return &AbsoluteError{pathname} + return AbsoluteError(pathname) } a.pathname = unique.Make(pathname) return nil diff --git a/container/check/absolute_test.go b/container/check/absolute_test.go index 107407f..25592a7 100644 --- a/container/check/absolute_test.go +++ b/container/check/absolute_test.go @@ -31,8 +31,8 @@ func TestAbsoluteError(t *testing.T) { }{ {"EINVAL", new(AbsoluteError), syscall.EINVAL, true}, {"not EINVAL", new(AbsoluteError), syscall.EBADE, false}, - {"ne val", new(AbsoluteError), &AbsoluteError{Pathname: "etc"}, false}, - {"equals", &AbsoluteError{Pathname: "etc"}, &AbsoluteError{Pathname: "etc"}, true}, + {"ne val", new(AbsoluteError), AbsoluteError("etc"), false}, + {"equals", AbsoluteError("etc"), AbsoluteError("etc"), true}, } for _, tc := range testCases { @@ -45,7 +45,7 @@ func TestAbsoluteError(t *testing.T) { t.Parallel() want := `path "etc" is not absolute` - if got := (&AbsoluteError{Pathname: "etc"}).Error(); got != want { + if got := (AbsoluteError("etc")).Error(); got != want { t.Errorf("Error: %q, want %q", got, want) } }) @@ -62,8 +62,8 @@ func TestNewAbs(t *testing.T) { wantErr error }{ {"good", "/etc", MustAbs("/etc"), nil}, - {"not absolute", "etc", nil, &AbsoluteError{Pathname: "etc"}}, - {"zero", "", nil, &AbsoluteError{Pathname: ""}}, + {"not absolute", "etc", nil, AbsoluteError("etc")}, + {"zero", "", nil, AbsoluteError("")}, } for _, tc := range testCases { @@ -84,7 +84,7 @@ func TestNewAbs(t *testing.T) { t.Parallel() defer func() { - wantPanic := &AbsoluteError{Pathname: "etc"} + wantPanic := AbsoluteError("etc") if r := recover(); !reflect.DeepEqual(r, wantPanic) { t.Errorf("MustAbs: panic = %v; want %v", r, wantPanic) @@ -175,7 +175,7 @@ func TestCodecAbsolute(t *testing.T) { `"/etc"`, `{"val":"/etc","magic":3236757504}`}, {"not absolute", nil, - &AbsoluteError{Pathname: "etc"}, + AbsoluteError("etc"), "\t\x7f\x05\x01\x02\xff\x82\x00\x00\x00\a\xff\x80\x00\x03etc", ",\xff\x83\x03\x01\x01\x06sCheck\x01\xff\x84\x00\x01\x02\x01\bPathname\x01\xff\x80\x00\x01\x05Magic\x01\x06\x00\x00\x00\t\x7f\x05\x01\x02\xff\x82\x00\x00\x00\x0f\xff\x84\x01\x03etc\x01\xfb\x01\x81\xda\x00\x00\x00", diff --git a/container/errors.go b/container/errors.go index 2bcb497..d7651aa 100644 --- a/container/errors.go +++ b/container/errors.go @@ -18,7 +18,7 @@ func messageFromError(err error) (m string, ok bool) { if m, ok = messagePrefixP[os.PathError]("cannot ", err); ok { return } - if m, ok = messagePrefixP[check.AbsoluteError](zeroString, err); ok { + if m, ok = messagePrefix[check.AbsoluteError](zeroString, err); ok { return } if m, ok = messagePrefix[OpRepeatError](zeroString, err); ok { diff --git a/container/errors_test.go b/container/errors_test.go index 5c55af4..71b4a9a 100644 --- a/container/errors_test.go +++ b/container/errors_test.go @@ -37,7 +37,7 @@ func TestMessageFromError(t *testing.T) { Err: stub.UniqueError(0xdeadbeef), }, "cannot mount /sysroot: unique error 3735928559 injected by the test suite", true}, - {"absolute", &check.AbsoluteError{Pathname: "etc/mtab"}, + {"absolute", check.AbsoluteError("etc/mtab"), `path "etc/mtab" is not absolute`, true}, {"repeat", OpRepeatError("autoetc"), diff --git a/container/initoverlay_test.go b/container/initoverlay_test.go index 1c0e642..0ff3cb4 100644 --- a/container/initoverlay_test.go +++ b/container/initoverlay_test.go @@ -312,7 +312,10 @@ func TestMountOverlayOp(t *testing.T) { }, }}, - {"ephemeral", new(Ops).OverlayEphemeral(check.MustAbs("/nix/store"), check.MustAbs("/mnt-root/nix/.ro-store")), Ops{ + {"ephemeral", new(Ops).OverlayEphemeral( + check.MustAbs("/nix/store"), + check.MustAbs("/mnt-root/nix/.ro-store"), + ), Ops{ &MountOverlayOp{ Target: check.MustAbs("/nix/store"), Lower: []*check.Absolute{check.MustAbs("/mnt-root/nix/.ro-store")}, @@ -320,7 +323,10 @@ func TestMountOverlayOp(t *testing.T) { }, }}, - {"readonly", new(Ops).OverlayReadonly(check.MustAbs("/nix/store"), check.MustAbs("/mnt-root/nix/.ro-store")), Ops{ + {"readonly", new(Ops).OverlayReadonly( + check.MustAbs("/nix/store"), + check.MustAbs("/mnt-root/nix/.ro-store"), + ), Ops{ &MountOverlayOp{ Target: check.MustAbs("/nix/store"), Lower: []*check.Absolute{check.MustAbs("/mnt-root/nix/.ro-store")}, diff --git a/container/initsymlink.go b/container/initsymlink.go index be0ce2a..df0ddc1 100644 --- a/container/initsymlink.go +++ b/container/initsymlink.go @@ -31,7 +31,7 @@ func (l *SymlinkOp) Valid() bool { return l != nil && l.Target != nil && l.LinkN func (l *SymlinkOp) early(_ *setupState, k syscallDispatcher) error { if l.Dereference { if !path.IsAbs(l.LinkName) { - return &check.AbsoluteError{Pathname: l.LinkName} + return check.AbsoluteError(l.LinkName) } if name, err := k.readlink(l.LinkName); err != nil { return err diff --git a/container/initsymlink_test.go b/container/initsymlink_test.go index e260b51..86d0be1 100644 --- a/container/initsymlink_test.go +++ b/container/initsymlink_test.go @@ -23,7 +23,7 @@ func TestSymlinkOp(t *testing.T) { Target: check.MustAbs("/etc/mtab"), LinkName: "etc/mtab", Dereference: true, - }, nil, &check.AbsoluteError{Pathname: "etc/mtab"}, nil, nil}, + }, nil, check.AbsoluteError("etc/mtab"), nil, nil}, {"readlink", &Params{ParentPerm: 0755}, &SymlinkOp{ Target: check.MustAbs("/etc/mtab"), diff --git a/internal/outcome/sppulse_test.go b/internal/outcome/sppulse_test.go index 70fae9e..f3f0635 100644 --- a/internal/outcome/sppulse_test.go +++ b/internal/outcome/sppulse_test.go @@ -108,7 +108,7 @@ func TestSpPulseOp(t *testing.T) { call("lookupEnv", stub.ExpectArgs{"PULSE_COOKIE"}, "proc/nonexistent/cookie", nil), }, nil, nil, &hst.AppError{ Step: "locate PulseAudio cookie", - Err: &check.AbsoluteError{Pathname: "proc/nonexistent/cookie"}, + Err: check.AbsoluteError("proc/nonexistent/cookie"), }, nil, nil, nil, nil, nil}, {"cookie loadFile", func(bool, bool) outcomeOp { @@ -272,7 +272,7 @@ func TestDiscoverPulseCookie(t *testing.T) { call("verbose", stub.ExpectArgs{[]any{(*check.Absolute)(nil)}}, nil, nil), }}, &hst.AppError{ Step: "locate PulseAudio cookie", - Err: &check.AbsoluteError{Pathname: "proc/nonexistent/pulse-cookie"}, + Err: check.AbsoluteError("proc/nonexistent/pulse-cookie"), }}, {"success override", fCheckPathname, stub.Expect{Calls: []stub.Call{ @@ -286,7 +286,7 @@ func TestDiscoverPulseCookie(t *testing.T) { call("verbose", stub.ExpectArgs{[]any{(*check.Absolute)(nil)}}, nil, nil), }}, &hst.AppError{ Step: "locate PulseAudio cookie", - Err: &check.AbsoluteError{Pathname: "proc/nonexistent/home"}, + Err: check.AbsoluteError("proc/nonexistent/home"), }}, {"home stat", fCheckPathname, stub.Expect{Calls: []stub.Call{ @@ -321,7 +321,7 @@ func TestDiscoverPulseCookie(t *testing.T) { call("verbose", stub.ExpectArgs{[]any{(*check.Absolute)(nil)}}, nil, nil), }}, &hst.AppError{ Step: "locate PulseAudio cookie", - Err: &check.AbsoluteError{Pathname: "proc/nonexistent/xdg"}, + Err: check.AbsoluteError("proc/nonexistent/xdg"), }}, {"xdg stat", fCheckPathname, stub.Expect{Calls: []stub.Call{ diff --git a/ldd/ldd_test.go b/ldd/ldd_test.go index a994d33..77f20a6 100644 --- a/ldd/ldd_test.go +++ b/ldd/ldd_test.go @@ -36,7 +36,7 @@ libzstd.so.1 = /usr/lib/libzstd.so.1 (0x7ff71bfd2000) {"path not absolute", ` libzstd.so.1 => usr/lib/libzstd.so.1 (0x7ff71bfd2000) -`, &check.AbsoluteError{Pathname: "usr/lib/libzstd.so.1"}}, +`, check.AbsoluteError("usr/lib/libzstd.so.1")}, {"unexpected segments", ` meow libzstd.so.1 => /usr/lib/libzstd.so.1 (0x7ff71bfd2000)