Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

Change-Id: If98dc381f71fd2e83fb07cf09dbc61af0f87baca
tailscale · Nov 5, 2024 · 3c5d48f · 3c5d48f
1 parent 3a78ec4
commit 3c5d48f
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 101 deletions.
diff --git a/net/netmon/interfaces_android.go b/net/netmon/interfaces_android.go
@@ -5,7 +5,6 @@ package netmon
 
 import (
 	"bytes"
-	"errors"
 	"log"
 	"net/netip"
 	"os/exec"
@@ -34,11 +33,6 @@ func init() {
 
 var procNetRouteErr atomic.Bool
 
-// errStopReading is a sentinel error value used internally by
-// lineread.File callers to stop reading. It doesn't escape to
-// callers/users.
-var errStopReading = errors.New("stop reading")
-
 /*
 Parse 10.0.0.1 out of:
 
@@ -54,44 +48,42 @@ func likelyHomeRouterIPAndroid() (ret netip.Addr, myIP netip.Addr, ok bool) {
 	}
 	lineNum := 0
 	var f []mem.RO
-	err := lineread.File(procNetRoutePath, func(line []byte) error {
+	for lr := range lineread.File(procNetRoutePath) {
+		line, err := lr.Value()
+		if err != nil {
+			procNetRouteErr.Store(true)
+			return likelyHomeRouterIP()
+		}
+
 		lineNum++
 		if lineNum == 1 {
 			// Skip header line.
-			return nil
+			continue
 		}
 		if lineNum > maxProcNetRouteRead {
-			return errStopReading
+			break
 		}
 		f = mem.AppendFields(f[:0], mem.B(line))
 		if len(f) < 4 {
-			return nil
+			continue
 		}
 		gwHex, flagsHex := f[2], f[3]
 		flags, err := mem.ParseUint(flagsHex, 16, 16)
 		if err != nil {
-			return nil // ignore error, skip line and keep going
+			continue // ignore error, skip line and keep going
 		}
 		if flags&(unix.RTF_UP|unix.RTF_GATEWAY) != unix.RTF_UP|unix.RTF_GATEWAY {
-			return nil
+			continue
 		}
 		ipu32, err := mem.ParseUint(gwHex, 16, 32)
 		if err != nil {
-			return nil // ignore error, skip line and keep going
+			continue // ignore error, skip line and keep going
 		}
 		ip := netaddr.IPv4(byte(ipu32), byte(ipu32>>8), byte(ipu32>>16), byte(ipu32>>24))
 		if ip.IsPrivate() {
 			ret = ip
-			return errStopReading
+			break
 		}
-		return nil
-	})
-	if errors.Is(err, errStopReading) {
-		err = nil
-	}
-	if err != nil {
-		procNetRouteErr.Store(true)
-		return likelyHomeRouterIP()
 	}
 	if ret.IsValid() {
 		// Try to get the local IP of the interface associated with

diff --git a/net/netmon/interfaces_darwin_test.go b/net/netmon/interfaces_darwin_test.go
@@ -4,7 +4,6 @@
 package netmon
 
 import (
-	"errors"
 	"io"
 	"net/netip"
 	"os/exec"
@@ -73,31 +72,34 @@ func likelyHomeRouterIPDarwinExec() (ret netip.Addr, netif string, ok bool) {
 	defer io.Copy(io.Discard, stdout) // clear the pipe to prevent hangs
 
 	var f []mem.RO
-	lineread.Reader(stdout, func(lineb []byte) error {
+	for lr := range lineread.Reader(stdout) {
+		lineb, err := lr.Value()
+		if err != nil {
+			break
+		}
 		line := mem.B(lineb)
 		if !mem.Contains(line, mem.S("default")) {
-			return nil
+			continue
 		}
 		f = mem.AppendFields(f[:0], line)
 		if len(f) < 4 || !f[0].EqualString("default") {
-			return nil
+			continue
 		}
 		ipm, flagsm, netifm := f[1], f[2], f[3]
 		if !mem.Contains(flagsm, mem.S("G")) {
-			return nil
+			continue
 		}
 		if mem.Contains(flagsm, mem.S("I")) {
-			return nil
+			continue
 		}
 		ip, err := netip.ParseAddr(string(mem.Append(nil, ipm)))
 		if err == nil && ip.IsPrivate() {
 			ret = ip
 			netif = netifm.StringCopy()
 			// We've found what we're looking for.
-			return errStopReadingNetstatTable
+			break
 		}
-		return nil
-	})
+	}
 	return ret, netif, ret.IsValid()
 }
 
@@ -110,5 +112,3 @@ func TestFetchRoutingTable(t *testing.T) {
 		}
 	}
 }
-
-var errStopReadingNetstatTable = errors.New("found private gateway")
diff --git a/net/netmon/interfaces_linux.go b/net/netmon/interfaces_linux.go
@@ -32,11 +32,6 @@ func init() {
 
 var procNetRouteErr atomic.Bool
 
-// errStopReading is a sentinel error value used internally by
-// lineread.File callers to stop reading. It doesn't escape to
-// callers/users.
-var errStopReading = errors.New("stop reading")
-
 /*
 Parse 10.0.0.1 out of:
 
@@ -52,44 +47,42 @@ func likelyHomeRouterIPLinux() (ret netip.Addr, myIP netip.Addr, ok bool) {
 	}
 	lineNum := 0
 	var f []mem.RO
-	err := lineread.File(procNetRoutePath, func(line []byte) error {
+	for lr := range lineread.File(procNetRoutePath) {
+		line, err := lr.Value()
+		if err != nil {
+			procNetRouteErr.Store(true)
+			log.Printf("interfaces: failed to read /proc/net/route: %v", err)
+			return ret, myIP, false
+		}
 		lineNum++
 		if lineNum == 1 {
 			// Skip header line.
-			return nil
+			continue
 		}
 		if lineNum > maxProcNetRouteRead {
-			return errStopReading
+			break
 		}
 		f = mem.AppendFields(f[:0], mem.B(line))
 		if len(f) < 4 {
-			return nil
+			continue
 		}
 		gwHex, flagsHex := f[2], f[3]
 		flags, err := mem.ParseUint(flagsHex, 16, 16)
 		if err != nil {
-			return nil // ignore error, skip line and keep going
+			continue // ignore error, skip line and keep going
 		}
 		if flags&(unix.RTF_UP|unix.RTF_GATEWAY) != unix.RTF_UP|unix.RTF_GATEWAY {
-			return nil
+			continue
 		}
 		ipu32, err := mem.ParseUint(gwHex, 16, 32)
 		if err != nil {
-			return nil // ignore error, skip line and keep going
+			continue // ignore error, skip line and keep going
 		}
 		ip := netaddr.IPv4(byte(ipu32), byte(ipu32>>8), byte(ipu32>>16), byte(ipu32>>24))
 		if ip.IsPrivate() {
 			ret = ip
-			return errStopReading
+			break
 		}
-		return nil
-	})
-	if errors.Is(err, errStopReading) {
-		err = nil
-	}
-	if err != nil {
-		procNetRouteErr.Store(true)
-		log.Printf("interfaces: failed to read /proc/net/route: %v", err)
 	}
 	if ret.IsValid() {
 		// Try to get the local IP of the interface associated with

diff --git a/net/netmon/netmon_linux_test.go b/net/netmon/netmon_linux_test.go
@@ -1,6 +1,8 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
+//go:build linux && !android
+
 package netmon
 
 import (

diff --git a/net/tshttpproxy/tshttpproxy_synology.go b/net/tshttpproxy/tshttpproxy_synology.go
@@ -76,21 +76,22 @@ func synologyProxiesFromConfig() (*url.URL, *url.URL, error) {
 func parseSynologyConfig(r io.Reader) (*url.URL, *url.URL, error) {
 	cfg := map[string]string{}
 
-	if err := lineread.Reader(r, func(line []byte) error {
+	for lr := range lineread.Reader(r) {
+		line, err := lr.Value()
+		if err != nil {
+			return nil, nil, err
+		}
 		// accept and skip over empty lines
 		line = bytes.TrimSpace(line)
 		if len(line) == 0 {
-			return nil
+			continue
 		}
 
 		key, value, ok := strings.Cut(string(line), "=")
 		if !ok {
-			return fmt.Errorf("missing \"=\" in proxy.conf line: %q", line)
+			return nil, nil, fmt.Errorf("missing \"=\" in proxy.conf line: %q", line)
 		}
 		cfg[string(key)] = string(value)
-		return nil
-	}); err != nil {
-		return nil, nil, err
 	}
 
 	if cfg["proxy_enabled"] != "yes" {

diff --git a/util/lineread/lineread.go b/util/lineread/lineread.go
@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// Package lineread reads lines from files. It's not fancy, but it got repetitive.
+// Package lineread splits things into lines.
 package lineread
 
 import (
@@ -15,24 +15,12 @@ import (
 )
 
 // File returns an iterator that reads lines from the named file.
+//
+// The returned substrings don't include the trailing newline.
+// Lines may be empty.
 func File(name string) iter.Seq[result.Of[[]byte]] {
 	f, err := os.Open(name)
-	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))
-		}
-	}
+	return reader(f, f, err)
 }
 
 // Bytes returns an iterator over the lines in bs.
@@ -54,17 +42,31 @@ func Bytes(bs []byte) iter.Seq[[]byte] {
 	}
 }
 
-// Reader calls fn for each line.
-// If fn returns an error, Reader stops reading and returns that error.
-// Reader may also return errors encountered reading and parsing from r.
-// To stop reading early, use a sentinel "stop" error value and ignore
-// it when returned from Reader.
-func Reader(r io.Reader, fn func(line []byte) error) error {
-	bs := bufio.NewScanner(r)
-	for bs.Scan() {
-		if err := fn(bs.Bytes()); err != nil {
-			return err
+// Reader returns an iterator over the lines in r.
+//
+// The returned substrings don't include the trailing newline.
+// Lines may be empty.
+func Reader(r io.Reader) iter.Seq[result.Of[[]byte]] {
+	return reader(r, nil, nil)
+}
+
+func reader(r io.Reader, c io.Closer, err error) iter.Seq[result.Of[[]byte]] {
+	return func(yield func(result.Of[[]byte]) bool) {
+		if err != nil {
+			yield(result.Error[[]byte](err))
+			return
+		}
+		if c != nil {
+			defer c.Close()
+		}
+		bs := bufio.NewScanner(r)
+		for bs.Scan() {
+			if !yield(result.Value(bs.Bytes())) {
+				return
+			}
+		}
+		if err := bs.Err(); err != nil {
+			yield(result.Error[[]byte](err))
 		}
 	}
-	return bs.Err()
 }
diff --git a/util/lineread/lineread_test.go b/util/lineread/lineread_test.go
@@ -1,3 +1,6 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
 package lineread
 
 import (
@@ -19,10 +22,9 @@ func TestBytesLines(t *testing.T) {
 
 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
-	})
+	for line := range Reader(strings.NewReader("foo\n\nbar\nbaz")) {
+		got = append(got, string(line.MustValue()))
+	}
 	want := []string{"foo", "", "bar", "baz"}
 	if !slices.Equal(got, want) {
 		t.Errorf("got %q; want %q", got, want)

diff --git a/util/pidowner/pidowner_linux.go b/util/pidowner/pidowner_linux.go
@@ -13,21 +13,21 @@ import (
 
 func ownerOfPID(pid int) (userID string, err error) {
 	file := fmt.Sprintf("/proc/%d/status", pid)
-	err = lineread.File(file, func(line []byte) error {
+	for lr := range lineread.File(file) {
+		line, err := lr.Value()
+		if err != nil {
+			if os.IsNotExist(err) {
+				return "", ErrProcessNotFound
+			}
+			return "", err
+		}
 		if len(line) < 4 || string(line[:4]) != "Uid:" {
-			return nil
+			continue
 		}
 		f := strings.Fields(string(line))
 		if len(f) >= 2 {
 			userID = f[1] // real userid
 		}
-		return nil
-	})
-	if os.IsNotExist(err) {
-		return "", ErrProcessNotFound
-	}
-	if err != nil {
-		return
 	}
 	if userID == "" {
 		return "", fmt.Errorf("missing Uid line in %s", file)