From 3a78ec47d18dafac14bc77da7c02909331109114 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 4 Nov 2024 20:49:40 -0800 Subject: [PATCH] types/result, util/lineread: add package for a result type, use it This adds a new generic result type (motivated by golang/go#70084) to try it out, and uses it in the lineread package, changing that package to return iterators: sometimes over []byte (when the input is all in memory), but sometimes iterators over results of []byte, if errors might happen at runtime. Updates #12912 Updates golang/go#70084 Change-Id: Iacdc1070e661b5fb163907b1e8b07ac7d51d3f83 Signed-off-by: Brad Fitzpatrick --- cmd/derper/depaware.txt | 1 + cmd/k8s-operator/depaware.txt | 1 + cmd/stund/depaware.txt | 1 + cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + hostinfo/hostinfo.go | 22 +++++++-------- hostinfo/hostinfo_linux.go | 11 +++++--- ipn/ipnlocal/ssh.go | 20 +++++++------- ssh/tailssh/tailssh_test.go | 11 +++----- ssh/tailssh/user.go | 16 +++++------ types/result/result.go | 49 ++++++++++++++++++++++++++++++++++ util/lineread/lineread.go | 47 +++++++++++++++++++++++++++----- util/lineread/lineread_test.go | 30 +++++++++++++++++++++ version/distro/distro.go | 14 +++++----- 14 files changed, 173 insertions(+), 52 deletions(-) create mode 100644 types/result/result.go create mode 100644 util/lineread/lineread_test.go diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index e20c4e556da8f7..f8610472af4bf0 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -140,6 +140,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa tailscale.com/types/persist from tailscale.com/ipn tailscale.com/types/preftype from tailscale.com/ipn tailscale.com/types/ptr from tailscale.com/hostinfo+ + tailscale.com/types/result from tailscale.com/util/lineread tailscale.com/types/structs from tailscale.com/ipn+ tailscale.com/types/tkatype from tailscale.com/client/tailscale+ tailscale.com/types/views from tailscale.com/ipn+ diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index d62f2e225ca7ec..8f6b0a9c3f9542 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -775,6 +775,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/ipn+ tailscale.com/types/ptr from tailscale.com/cmd/k8s-operator+ + tailscale.com/types/result from tailscale.com/util/lineread tailscale.com/types/structs from tailscale.com/control/controlclient+ tailscale.com/types/tkatype from tailscale.com/client/tailscale+ tailscale.com/types/views from tailscale.com/appc+ diff --git a/cmd/stund/depaware.txt b/cmd/stund/depaware.txt index a35f59516ee328..e6f565625d86b2 100644 --- a/cmd/stund/depaware.txt +++ b/cmd/stund/depaware.txt @@ -67,6 +67,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar tailscale.com/types/logger from tailscale.com/tsweb tailscale.com/types/opt from tailscale.com/envknob+ tailscale.com/types/ptr from tailscale.com/tailcfg+ + tailscale.com/types/result from tailscale.com/util/lineread tailscale.com/types/structs from tailscale.com/tailcfg+ tailscale.com/types/tkatype from tailscale.com/tailcfg+ tailscale.com/types/views from tailscale.com/net/tsaddr+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index cce76a81e0bfb5..65cb0fdd07b545 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -148,6 +148,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/types/persist from tailscale.com/ipn tailscale.com/types/preftype from tailscale.com/cmd/tailscale/cli+ tailscale.com/types/ptr from tailscale.com/hostinfo+ + tailscale.com/types/result from tailscale.com/util/lineread tailscale.com/types/structs from tailscale.com/ipn+ tailscale.com/types/tkatype from tailscale.com/types/key+ tailscale.com/types/views from tailscale.com/tailcfg+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 53e4790d38eeb8..f053576c45c841 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -364,6 +364,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/ipn+ tailscale.com/types/ptr from tailscale.com/control/controlclient+ + tailscale.com/types/result from tailscale.com/util/lineread tailscale.com/types/structs from tailscale.com/control/controlclient+ tailscale.com/types/tkatype from tailscale.com/tka+ tailscale.com/types/views from tailscale.com/ipn/ipnlocal+ diff --git a/hostinfo/hostinfo.go b/hostinfo/hostinfo.go index 3233a422dd6c32..a97226b1acf14b 100644 --- a/hostinfo/hostinfo.go +++ b/hostinfo/hostinfo.go @@ -231,12 +231,12 @@ func desktop() (ret opt.Bool) { } seenDesktop := false - lineread.File("/proc/net/unix", func(line []byte) error { + for lr := range lineread.File("/proc/net/unix") { + line, _ := lr.Value() seenDesktop = seenDesktop || mem.Contains(mem.B(line), mem.S(" @/tmp/dbus-")) seenDesktop = seenDesktop || mem.Contains(mem.B(line), mem.S(".X11-unix")) seenDesktop = seenDesktop || mem.Contains(mem.B(line), mem.S("/wayland-1")) - return nil - }) + } ret.Set(seenDesktop) // Only cache after a minute - compositors might not have started yet. @@ -305,21 +305,21 @@ func inContainer() opt.Bool { ret.Set(true) return ret } - lineread.File("/proc/1/cgroup", func(line []byte) error { + for lr := range lineread.File("/proc/1/cgroup") { + line, _ := lr.Value() if mem.Contains(mem.B(line), mem.S("/docker/")) || mem.Contains(mem.B(line), mem.S("/lxc/")) { ret.Set(true) - return io.EOF // arbitrary non-nil error to stop loop + break } - return nil - }) - lineread.File("/proc/mounts", func(line []byte) error { + } + for lr := range lineread.File("/proc/mounts") { + line, _ := lr.Value() if mem.Contains(mem.B(line), mem.S("lxcfs /proc/cpuinfo fuse.lxcfs")) { ret.Set(true) - return io.EOF + break } - return nil - }) + } return ret } diff --git a/hostinfo/hostinfo_linux.go b/hostinfo/hostinfo_linux.go index 53d4187bc0c677..465365861d606e 100644 --- a/hostinfo/hostinfo_linux.go +++ b/hostinfo/hostinfo_linux.go @@ -106,15 +106,18 @@ func linuxVersionMeta() (meta versionMeta) { } m := map[string]string{} - lineread.File(propFile, func(line []byte) error { + for lr := range lineread.File(propFile) { + line, err := lr.Value() + if err != nil { + break + } eq := bytes.IndexByte(line, '=') if eq == -1 { - return nil + continue } k, v := string(line[:eq]), strings.Trim(string(line[eq+1:]), `"'`) m[k] = v - return nil - }) + } if v := m["VERSION_CODENAME"]; v != "" { meta.DistroCodeName = v diff --git a/ipn/ipnlocal/ssh.go b/ipn/ipnlocal/ssh.go index fbeb19bd15bd18..a525b286a09659 100644 --- a/ipn/ipnlocal/ssh.go +++ b/ipn/ipnlocal/ssh.go @@ -80,30 +80,32 @@ func (b *LocalBackend) getSSHUsernames(req *tailcfg.C2NSSHUsernamesRequest) (*ta if err != nil { return nil, err } - lineread.Reader(bytes.NewReader(out), func(line []byte) error { + for line := range lineread.Bytes(out) { line = bytes.TrimSpace(line) if len(line) == 0 || line[0] == '_' { - return nil + continue } add(string(line)) - return nil - }) + } default: - lineread.File("/etc/passwd", func(line []byte) error { + for lr := range lineread.File("/etc/passwd") { + line, err := lr.Value() + if err != nil { + break + } line = bytes.TrimSpace(line) if len(line) == 0 || line[0] == '#' || line[0] == '_' { - return nil + continue } if mem.HasSuffix(mem.B(line), mem.S("/nologin")) || mem.HasSuffix(mem.B(line), mem.S("/false")) { - return nil + continue } colon := bytes.IndexByte(line, ':') if colon != -1 { add(string(line[:colon])) } - return nil - }) + } } return res, nil } diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 9e4f5ffd3d4811..36fe686735021d 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -1123,14 +1123,11 @@ func TestSSH(t *testing.T) { func parseEnv(out []byte) map[string]string { e := map[string]string{} - lineread.Reader(bytes.NewReader(out), func(line []byte) error { - i := bytes.IndexByte(line, '=') - if i == -1 { - return nil + for line := range lineread.Bytes(out) { + if i := bytes.IndexByte(line, '='); i != -1 { + e[string(line[:i])] = string(line[i+1:]) } - e[string(line[:i])] = string(line[i+1:]) - return nil - }) + } return e } diff --git a/ssh/tailssh/user.go b/ssh/tailssh/user.go index 33ebb4db729de2..8cd93a52b9e4c8 100644 --- a/ssh/tailssh/user.go +++ b/ssh/tailssh/user.go @@ -6,7 +6,6 @@ package tailssh import ( - "io" "os" "os/exec" "os/user" @@ -110,15 +109,16 @@ func defaultPathForUser(u *user.User) string { } func defaultPathForUserOnNixOS(u *user.User) string { - var path string - lineread.File("/etc/pam/environment", func(lineb []byte) error { + for lr := range lineread.File("/etc/pam/environment") { + lineb, err := lr.Value() + if err != nil { + return "" + } if v := pathFromPAMEnvLine(lineb, u); v != "" { - path = v - return io.EOF // stop iteration + return v } - return nil - }) - return path + } + return "" } func pathFromPAMEnvLine(line []byte, u *user.User) (path string) { diff --git a/types/result/result.go b/types/result/result.go new file mode 100644 index 00000000000000..6bd1c2ea620044 --- /dev/null +++ b/types/result/result.go @@ -0,0 +1,49 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package result contains the Of result type, which is +// either a value or an error. +package result + +// Of is either a T value or an error. +// +// Think of it like Rust or Swift's result types. +// It's named "Of" because the fully qualified name +// for callers reads result.Of[T]. +type Of[T any] struct { + v T // valid if Err is nil; invalid if Err is non-nil + err error +} + +// Value returns a new result with value v, +// without an error. +func Value[T any](v T) Of[T] { + return Of[T]{v: v} +} + +// Error returns a new result with error err. +// If err is nil, the returned result is equivalent +// to calling Value with T's zero value. +func Error[T any](err error) Of[T] { + return Of[T]{err: err} +} + +// MustValue returns r's result value. +// It panics if r.Err returns non-nil. +func (r Of[T]) MustValue() T { + if r.err != nil { + panic(r.err) + } + return r.v +} + +// Value returns r's result value and error. +func (r Of[T]) Value() (T, error) { + return r.v, r.err +} + +// Err returns r's error, if any. +// When r.Err returns nil, it's safe to call r.MustValue without it panicking. +func (r Of[T]) Err() error { + return r.err +} diff --git a/util/lineread/lineread.go b/util/lineread/lineread.go index 6b01d2b69ffd76..f7f9e22e74117b 100644 --- a/util/lineread/lineread.go +++ b/util/lineread/lineread.go @@ -6,19 +6,52 @@ package lineread import ( "bufio" + "bytes" "io" + "iter" "os" + + "tailscale.com/types/result" ) -// File opens name and calls fn for each line. It returns an error if the Open failed -// or once fn returns an error. -func File(name string, fn func(line []byte) error) error { +// File returns an iterator that reads lines from the named file. +func File(name string) iter.Seq[result.Of[[]byte]] { f, err := os.Open(name) - if err != nil { - return err + return func(yield func(result.Of[[]byte]) bool) { + if err != nil { + yield(result.Error[[]byte](err)) + return + } + defer f.Close() + bs := bufio.NewScanner(f) + for bs.Scan() { + if !yield(result.Value(bs.Bytes())) { + return + } + } + if err := bs.Err(); err != nil { + yield(result.Error[[]byte](err)) + } + } +} + +// Bytes returns an iterator over the lines in bs. +// The returned substrings don't include the trailing newline. +// Lines may be empty. +func Bytes(bs []byte) iter.Seq[[]byte] { + return func(yield func([]byte) bool) { + for len(bs) > 0 { + i := bytes.IndexByte(bs, '\n') + if i < 0 { + yield(bs) + return + } + if !yield(bs[:i]) { + return + } + bs = bs[i+1:] + } } - defer f.Close() - return Reader(f, fn) } // Reader calls fn for each line. diff --git a/util/lineread/lineread_test.go b/util/lineread/lineread_test.go new file mode 100644 index 00000000000000..dfe9b5ab0436fe --- /dev/null +++ b/util/lineread/lineread_test.go @@ -0,0 +1,30 @@ +package lineread + +import ( + "slices" + "strings" + "testing" +) + +func TestBytesLines(t *testing.T) { + var got []string + for line := range Bytes([]byte("foo\n\nbar\nbaz")) { + got = append(got, string(line)) + } + want := []string{"foo", "", "bar", "baz"} + if !slices.Equal(got, want) { + t.Errorf("got %q; want %q", got, want) + } +} + +func TestReader(t *testing.T) { + var got []string + Reader(strings.NewReader("foo\n\nbar\nbaz"), func(line []byte) error { + got = append(got, string(line)) + return nil + }) + want := []string{"foo", "", "bar", "baz"} + if !slices.Equal(got, want) { + t.Errorf("got %q; want %q", got, want) + } +} diff --git a/version/distro/distro.go b/version/distro/distro.go index 8865a834b97d3b..c7fd3d65e0814d 100644 --- a/version/distro/distro.go +++ b/version/distro/distro.go @@ -6,7 +6,6 @@ package distro import ( "bytes" - "io" "os" "runtime" "strconv" @@ -132,18 +131,21 @@ func DSMVersion() int { return v } // But when run from the command line, we have to read it from the file: - lineread.File("/etc/VERSION", func(line []byte) error { + for lr := range lineread.File("/etc/VERSION") { + line, err := lr.Value() + if err != nil { + break // but otherwise ignore + } line = bytes.TrimSpace(line) if string(line) == `majorversion="7"` { v = 7 - return io.EOF + break } if string(line) == `majorversion="6"` { v = 6 - return io.EOF + break } - return nil - }) + } return v }) }