Merge pull request #50 from cyphar/mountinfo-improvements

mountinfo: correctness improvements

Author: kolyshkin

mountinfo/mountinfo_filters.go

@@ -6,9 +6,8 @@ import "strings"
 // used to filter out mountinfo entries we're not interested in,
 // and/or stop further processing if we found what we wanted.
 //
-// It takes a pointer to the Info struct (not fully populated,
-// currently only Mountpoint, FSType, Source, and (on Linux)
-// VFSOptions are filled in), and returns two booleans:
+// It takes a pointer to the Info struct (fully populated with all available
+// fields on the GOOS platform), and returns two booleans:
 //
 // skip: true if the entry should be skipped;
 //

mountinfo/mountinfo_linux.go

@@ -18,11 +18,9 @@ import (
 func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
 	s := bufio.NewScanner(r)
 	out := []*Info{}
-	var err error
 	for s.Scan() {
-		if err = s.Err(); err != nil {
-			return nil, err
-		}
+		var err error
+
 		/*
 		   See http://man7.org/linux/man-pages/man5/proc.5.html
 
@@ -74,7 +72,6 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
 
 		p := &Info{}
 
-		// Fill in the fields that a filter might check
 		p.Mountpoint, err = unescape(fields[4])
 		if err != nil {
 			return nil, fmt.Errorf("Parsing '%s' failed: mount point: %w", fields[4], err)
@@ -89,18 +86,6 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
 		}
 		p.VFSOptions = fields[sepIdx+3]
 
-		// Run a filter early so we can skip parsing/adding entries
-		// the caller is not interested in
-		var skip, stop bool
-		if filter != nil {
-			skip, stop = filter(p)
-			if skip {
-				continue
-			}
-		}
-
-		// Fill in the rest of the fields
-
 		// ignore any numbers parsing errors, as there should not be any
 		p.ID, _ = strconv.Atoi(fields[0])
 		p.Parent, _ = strconv.Atoi(fields[1])
@@ -128,11 +113,23 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) {
 			p.Optional = strings.Join(fields[6:sepIdx-1], " ")
 		}
 
+		// Run the filter after parsing all of the fields.
+		var skip, stop bool
+		if filter != nil {
+			skip, stop = filter(p)
+			if skip {
+				continue
+			}
+		}
+
 		out = append(out, p)
 		if stop {
 			break
 		}
 	}
+	if err := s.Err(); err != nil {
+		return nil, err
+	}
 	return out, nil
 }
 

mountinfo/mountinfo_linux_test.go

@@ -477,6 +477,33 @@ func TestParseFedoraMountinfoFields(t *testing.T) {
 	}
 }
 
+func TestParseFedoraMountinfoFilterFields(t *testing.T) {
+	r := bytes.NewBuffer([]byte(fedoraMountinfo))
+	_, err := GetMountsFromReader(r, func(info *Info) (skip bool, stop bool) {
+		mi := Info{
+			ID:         15,
+			Parent:     35,
+			Major:      0,
+			Minor:      3,
+			Root:       "/",
+			Mountpoint: "/proc",
+			Options:    "rw,nosuid,nodev,noexec,relatime",
+			Optional:   "shared:5",
+			FSType:     "proc",
+			Source:     "proc",
+			VFSOptions: "rw",
+		}
+		if *info != mi {
+			t.Fatalf("expected %#v, got %#v", mi, *info)
+		}
+		// Only match the first entry as in TestParseFedoraMountinfoFields.
+		return false, true
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestParseMountinfoWithSpaces(t *testing.T) {
 	r := bytes.NewBuffer([]byte(mountInfoWithSpaces))
 	infos, err := GetMountsFromReader(r, nil)