Merge pull request #4470 from kolyshkin/strings-cut

Use strings.Cut and strings.CutPrefix where possible

exec.go

@@ -117,6 +117,12 @@ following will output a list of processes running in the container:
 	SkipArgReorder: true,
 }
 
+// getSubCgroupPaths parses --cgroup arguments, which can either be
+//   - a single "path" argument (for cgroup v2);
+//   - one or more controller[,controller[,...]]:path arguments (for cgroup v1).
+//
+// Returns a controller to path map. For cgroup v2, it's a single entity map
+// with empty controller value.
 func getSubCgroupPaths(args []string) (map[string]string, error) {
 	if len(args) == 0 {
 		return nil, nil
@@ -124,20 +130,23 @@ func getSubCgroupPaths(args []string) (map[string]string, error) {
 	paths := make(map[string]string, len(args))
 	for _, c := range args {
 		// Split into controller:path.
-		cs := strings.SplitN(c, ":", 3)
-		if len(cs) > 2 {
-			return nil, fmt.Errorf("invalid --cgroup argument: %s", c)
-		}
-		if len(cs) == 1 { // no controller: prefix
+		if ctr, path, ok := strings.Cut(c, ":"); ok {
+			// There may be a few comma-separated controllers.
+			for _, ctrl := range strings.Split(ctr, ",") {
+				if ctrl == "" {
+					return nil, fmt.Errorf("invalid --cgroup argument: %s (empty <controller> prefix)", c)
+				}
+				if _, ok := paths[ctrl]; ok {
+					return nil, fmt.Errorf("invalid --cgroup argument(s): controller %s specified multiple times", ctrl)
+				}
+				paths[ctrl] = path
+			}
+		} else {
+			// No "controller:" prefix (cgroup v2, a single path).
 			if len(args) != 1 {
 				return nil, fmt.Errorf("invalid --cgroup argument: %s (missing <controller>: prefix)", c)
 			}
 			paths[""] = c
-		} else {
-			// There may be a few comma-separated controllers.
-			for _, ctrl := range strings.Split(cs[0], ",") {
-				paths[ctrl] = cs[1]
-			}
 		}
 	}
 	return paths, nil

libcontainer/cgroups/devices/systemd.go

@@ -224,8 +224,7 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) {
 			continue
 		}
 
-		group := strings.TrimPrefix(line, ruleMajorStr)
-		if len(group) < len(line) { // got it
+		if group, ok := strings.CutPrefix(line, ruleMajorStr); ok {
 			return prefix + group, nil
 		}
 	}

libcontainer/cgroups/file.go

@@ -151,8 +151,8 @@ func openFile(dir, file string, flags int) (*os.File, error) {
 	if prepareOpenat2() != nil {
 		return openFallback(path, flags, mode)
 	}
-	relPath := strings.TrimPrefix(path, cgroupfsPrefix)
-	if len(relPath) == len(path) { // non-standard path, old system?
+	relPath, ok := strings.CutPrefix(path, cgroupfsPrefix)
+	if !ok { // Non-standard path, old system?
 		return openFallback(path, flags, mode)
 	}
 

libcontainer/cgroups/fs/cpuacct.go

@@ -11,14 +11,7 @@ import (
 )
 
 const (
-	cgroupCpuacctStat     = "cpuacct.stat"
-	cgroupCpuacctUsageAll = "cpuacct.usage_all"
-
-	nanosecondsInSecond = 1000000000
-
-	userModeColumn              = 1
-	kernelModeColumn            = 2
-	cuacctUsageAllColumnsNumber = 3
+	nsInSec = 1000000000
 
 	// The value comes from `C.sysconf(C._SC_CLK_TCK)`, and
 	// on Linux it's a constant which is safe to be hard coded,
@@ -80,7 +73,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
 	const (
 		userField   = "user"
 		systemField = "system"
-		file        = cgroupCpuacctStat
+		file        = "cpuacct.stat"
 	)
 
 	// Expected format:
@@ -102,7 +95,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
 		return 0, 0, &parseError{Path: path, File: file, Err: err}
 	}
 
-	return (userModeUsage * nanosecondsInSecond) / clockTicks, (kernelModeUsage * nanosecondsInSecond) / clockTicks, nil
+	return (userModeUsage * nsInSec) / clockTicks, (kernelModeUsage * nsInSec) / clockTicks, nil
 }
 
 func getPercpuUsage(path string) ([]uint64, error) {
@@ -112,7 +105,6 @@ func getPercpuUsage(path string) ([]uint64, error) {
 	if err != nil {
 		return percpuUsage, err
 	}
-	// TODO: use strings.SplitN instead.
 	for _, value := range strings.Fields(data) {
 		value, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
@@ -126,7 +118,7 @@ func getPercpuUsage(path string) ([]uint64, error) {
 func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) {
 	usageKernelMode := []uint64{}
 	usageUserMode := []uint64{}
-	const file = cgroupCpuacctUsageAll
+	const file = "cpuacct.usage_all"
 
 	fd, err := cgroups.OpenFile(path, file, os.O_RDONLY)
 	if os.IsNotExist(err) {
@@ -140,22 +132,23 @@ func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) {
 	scanner.Scan() // skipping header line
 
 	for scanner.Scan() {
-		lineFields := strings.SplitN(scanner.Text(), " ", cuacctUsageAllColumnsNumber+1)
-		if len(lineFields) != cuacctUsageAllColumnsNumber {
+		// Each line is: cpu user system
+		fields := strings.SplitN(scanner.Text(), " ", 3)
+		if len(fields) != 3 {
 			continue
 		}
 
-		usageInKernelMode, err := strconv.ParseUint(lineFields[kernelModeColumn], 10, 64)
+		user, err := strconv.ParseUint(fields[1], 10, 64)
 		if err != nil {
 			return nil, nil, &parseError{Path: path, File: file, Err: err}
 		}
-		usageKernelMode = append(usageKernelMode, usageInKernelMode)
+		usageUserMode = append(usageUserMode, user)
 
-		usageInUserMode, err := strconv.ParseUint(lineFields[userModeColumn], 10, 64)
+		kernel, err := strconv.ParseUint(fields[2], 10, 64)
 		if err != nil {
 			return nil, nil, &parseError{Path: path, File: file, Err: err}
 		}
-		usageUserMode = append(usageUserMode, usageInUserMode)
+		usageKernelMode = append(usageKernelMode, kernel)
 	}
 	if err := scanner.Err(); err != nil {
 		return nil, nil, &parseError{Path: path, File: file, Err: err}

libcontainer/cgroups/fs/cpuacct_test.go

@@ -53,8 +53,8 @@ func TestCpuacctStats(t *testing.T) {
 			962250696038415, 981956408513304, 1002658817529022, 994937703492523,
 			874843781648690, 872544369885276, 870104915696359, 870202363887496,
 		},
-		UsageInKernelmode: (uint64(291429664) * nanosecondsInSecond) / clockTicks,
-		UsageInUsermode:   (uint64(452278264) * nanosecondsInSecond) / clockTicks,
+		UsageInKernelmode: (uint64(291429664) * nsInSec) / clockTicks,
+		UsageInUsermode:   (uint64(452278264) * nsInSec) / clockTicks,
 	}
 
 	if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) {
@@ -86,8 +86,8 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) {
 		},
 		PercpuUsageInKernelmode: []uint64{},
 		PercpuUsageInUsermode:   []uint64{},
-		UsageInKernelmode:       (uint64(291429664) * nanosecondsInSecond) / clockTicks,
-		UsageInUsermode:         (uint64(452278264) * nanosecondsInSecond) / clockTicks,
+		UsageInKernelmode:       (uint64(291429664) * nsInSec) / clockTicks,
+		UsageInUsermode:         (uint64(452278264) * nsInSec) / clockTicks,
 	}
 
 	if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) {

libcontainer/cgroups/fs/cpuset.go

@@ -48,26 +48,23 @@ func getCpusetStat(path string, file string) ([]uint16, error) {
 	}
 
 	for _, s := range strings.Split(fileContent, ",") {
-		sp := strings.SplitN(s, "-", 3)
-		switch len(sp) {
-		case 3:
-			return extracted, &parseError{Path: path, File: file, Err: errors.New("extra dash")}
-		case 2:
-			min, err := strconv.ParseUint(sp[0], 10, 16)
+		fromStr, toStr, ok := strings.Cut(s, "-")
+		if ok {
+			from, err := strconv.ParseUint(fromStr, 10, 16)
 			if err != nil {
 				return extracted, &parseError{Path: path, File: file, Err: err}
 			}
-			max, err := strconv.ParseUint(sp[1], 10, 16)
+			to, err := strconv.ParseUint(toStr, 10, 16)
 			if err != nil {
 				return extracted, &parseError{Path: path, File: file, Err: err}
 			}
-			if min > max {
-				return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, min > max")}
+			if from > to {
+				return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, from > to")}
 			}
-			for i := min; i <= max; i++ {
+			for i := from; i <= to; i++ {
 				extracted = append(extracted, uint16(i))
 			}
-		case 1:
+		} else {
 			value, err := strconv.ParseUint(s, 10, 16)
 			if err != nil {
 				return extracted, &parseError{Path: path, File: file, Err: err}

libcontainer/cgroups/fs2/defaultpath.go

@@ -83,16 +83,9 @@ func parseCgroupFile(path string) (string, error) {
 func parseCgroupFromReader(r io.Reader) (string, error) {
 	s := bufio.NewScanner(r)
 	for s.Scan() {
-		var (
-			text  = s.Text()
-			parts = strings.SplitN(text, ":", 3)
-		)
-		if len(parts) < 3 {
-			return "", fmt.Errorf("invalid cgroup entry: %q", text)
-		}
-		// text is like "0::/user.slice/user-1001.slice/session-1.scope"
-		if parts[0] == "0" && parts[1] == "" {
-			return parts[2], nil
+		// "0::/user.slice/user-1001.slice/session-1.scope"
+		if path, ok := strings.CutPrefix(s.Text(), "0::"); ok {
+			return path, nil
 		}
 	}
 	if err := s.Err(); err != nil {

libcontainer/cgroups/fs2/freezer.go

@@ -104,9 +104,7 @@ func waitFrozen(dirPath string) (cgroups.FreezerState, error) {
 		if i == maxIter {
 			return cgroups.Undefined, fmt.Errorf("timeout of %s reached waiting for the cgroup to freeze", waitTime*maxIter)
 		}
-		line := scanner.Text()
-		val := strings.TrimPrefix(line, "frozen ")
-		if val != line { // got prefix
+		if val, ok := strings.CutPrefix(scanner.Text(), "frozen "); ok {
 			if val[0] == '1' {
 				return cgroups.Frozen, nil
 			}

libcontainer/cgroups/fs2/fs2.go

@@ -237,11 +237,10 @@ func (m *Manager) setUnified(res map[string]string) error {
 			if errors.Is(err, os.ErrPermission) || errors.Is(err, os.ErrNotExist) {
 				// Check if a controller is available,
 				// to give more specific error if not.
-				sk := strings.SplitN(k, ".", 2)
-				if len(sk) != 2 {
+				c, _, ok := strings.Cut(k, ".")
+				if !ok {
 					return fmt.Errorf("unified resource %q must be in the form CONTROLLER.PARAMETER", k)
 				}
-				c := sk[0]
 				if _, ok := m.controllers[c]; !ok && c != "cgroup" {
 					return fmt.Errorf("unified resource %q can't be set: controller %q not available", k, c)
 				}

libcontainer/cgroups/fs2/psi.go

@@ -58,12 +58,12 @@ func statPSI(dirPath string, file string) (*cgroups.PSIStats, error) {
 func parsePSIData(psi []string) (cgroups.PSIData, error) {
 	data := cgroups.PSIData{}
 	for _, f := range psi {
-		kv := strings.SplitN(f, "=", 2)
-		if len(kv) != 2 {
+		key, val, ok := strings.Cut(f, "=")
+		if !ok {
 			return data, fmt.Errorf("invalid psi data: %q", f)
 		}
 		var pv *float64
-		switch kv[0] {
+		switch key {
 		case "avg10":
 			pv = &data.Avg10
 		case "avg60":
@@ -71,16 +71,16 @@ func parsePSIData(psi []string) (cgroups.PSIData, error) {
 		case "avg300":
 			pv = &data.Avg300
 		case "total":
-			v, err := strconv.ParseUint(kv[1], 10, 64)
+			v, err := strconv.ParseUint(val, 10, 64)
 			if err != nil {
-				return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err)
+				return data, fmt.Errorf("invalid %s PSI value: %w", key, err)
 			}
 			data.Total = v
 		}
 		if pv != nil {
-			v, err := strconv.ParseFloat(kv[1], 64)
+			v, err := strconv.ParseFloat(val, 64)
 			if err != nil {
-				return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err)
+				return data, fmt.Errorf("invalid %s PSI value: %w", key, err)
 			}
 			*pv = v
 		}

libcontainer/cgroups/fscommon/rdma.go

@@ -17,14 +17,12 @@ import (
 func parseRdmaKV(raw string, entry *cgroups.RdmaEntry) error {
 	var value uint32
 
-	parts := strings.SplitN(raw, "=", 3)
+	k, v, ok := strings.Cut(raw, "=")
 
-	if len(parts) != 2 {
+	if !ok {
 		return errors.New("Unable to parse RDMA entry")
 	}
 
-	k, v := parts[0], parts[1]
-
 	if v == "max" {
 		value = math.MaxUint32
 	} else {
@@ -34,9 +32,10 @@ func parseRdmaKV(raw string, entry *cgroups.RdmaEntry) error {
 		}
 		value = uint32(val64)
 	}
-	if k == "hca_handle" {
+	switch k {
+	case "hca_handle":
 		entry.HcaHandles = value
-	} else if k == "hca_object" {
+	case "hca_object":
 		entry.HcaObjects = value
 	}
 

libcontainer/cgroups/fscommon/utils.go

@@ -54,38 +54,39 @@ func ParseUint(s string, base, bitSize int) (uint64, error) {
 	return value, nil
 }
 
-// ParseKeyValue parses a space-separated "name value" kind of cgroup
+// ParseKeyValue parses a space-separated "key value" kind of cgroup
 // parameter and returns its key as a string, and its value as uint64
-// (ParseUint is used to convert the value). For example,
+// (using [ParseUint] to convert the value). For example,
 // "io_service_bytes 1234" will be returned as "io_service_bytes", 1234.
 func ParseKeyValue(t string) (string, uint64, error) {
-	parts := strings.SplitN(t, " ", 3)
-	if len(parts) != 2 {
-		return "", 0, fmt.Errorf("line %q is not in key value format", t)
+	key, val, ok := strings.Cut(t, " ")
+	if !ok || key == "" || val == "" {
+		return "", 0, fmt.Errorf(`line %q is not in "key value" format`, t)
 	}
 
-	value, err := ParseUint(parts[1], 10, 64)
+	value, err := ParseUint(val, 10, 64)
 	if err != nil {
 		return "", 0, err
 	}
 
-	return parts[0], value, nil
+	return key, value, nil
 }
 
-// GetValueByKey reads a key-value pairs from the specified cgroup file,
-// and returns a value of the specified key. ParseUint is used for value
-// conversion.
+// GetValueByKey reads space-separated "key value" pairs from the specified
+// cgroup file, looking for a specified key, and returns its value as uint64,
+// using [ParseUint] for conversion. If the value is not found, 0 is returned.
 func GetValueByKey(path, file, key string) (uint64, error) {
 	content, err := cgroups.ReadFile(path, file)
 	if err != nil {
 		return 0, err
 	}
 
+	key += " "
 	lines := strings.Split(content, "\n")
 	for _, line := range lines {
-		arr := strings.Split(line, " ")
-		if len(arr) == 2 && arr[0] == key {
-			val, err := ParseUint(arr[1], 10, 64)
+		v, ok := strings.CutPrefix(line, key)
+		if ok {
+			val, err := ParseUint(v, 10, 64)
 			if err != nil {
 				err = &ParseError{Path: path, File: file, Err: err}
 			}
@@ -103,7 +104,6 @@ func GetCgroupParamUint(path, file string) (uint64, error) {
 	if err != nil {
 		return 0, err
 	}
-	contents = strings.TrimSpace(contents)
 	if contents == "max" {
 		return math.MaxUint64, nil
 	}
@@ -118,11 +118,10 @@ func GetCgroupParamUint(path, file string) (uint64, error) {
 // GetCgroupParamInt reads a single int64 value from specified cgroup file.
 // If the value read is "max", the math.MaxInt64 is returned.
 func GetCgroupParamInt(path, file string) (int64, error) {
-	contents, err := cgroups.ReadFile(path, file)
+	contents, err := GetCgroupParamString(path, file)
 	if err != nil {
 		return 0, err
 	}
-	contents = strings.TrimSpace(contents)
 	if contents == "max" {
 		return math.MaxInt64, nil
 	}

libcontainer/cgroups/systemd/user.go

@@ -61,8 +61,7 @@ func DetectUID() (int, error) {
 	scanner := bufio.NewScanner(bytes.NewReader(b))
 	for scanner.Scan() {
 		s := strings.TrimSpace(scanner.Text())
-		if strings.HasPrefix(s, "OwnerUID=") {
-			uidStr := strings.TrimPrefix(s, "OwnerUID=")
+		if uidStr, ok := strings.CutPrefix(s, "OwnerUID="); ok {
 			i, err := strconv.Atoi(uidStr)
 			if err != nil {
 				return -1, fmt.Errorf("could not detect the OwnerUID: %w", err)

libcontainer/cgroups/utils.go

@@ -332,8 +332,8 @@ func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) {
 
 	for _, file := range fileNames {
 		// example: hugepages-1048576kB
-		val := strings.TrimPrefix(file, "hugepages-")
-		if len(val) == len(file) {
+		val, ok := strings.CutPrefix(file, "hugepages-")
+		if !ok {
 			// Unexpected file name: no prefix found, ignore it.
 			continue
 		}

libcontainer/configs/validate/rootless.go

@@ -52,29 +52,32 @@ func rootlessEUIDMount(config *configs.Config) error {
 	//      convinced that's a good idea. The kernel is the best arbiter of
 	//      access control.
 
+	// Check that the options list doesn't contain any uid= or gid= entries
+	// that don't resolve to root.
 	for _, mount := range config.Mounts {
-		// Check that the options list doesn't contain any uid= or gid= entries
-		// that don't resolve to root.
+		// Look for a common substring; skip further processing
+		// if there can't be any uid= or gid= options.
+		if !strings.Contains(mount.Data, "id=") {
+			continue
+		}
 		for _, opt := range strings.Split(mount.Data, ",") {
-			if str := strings.TrimPrefix(opt, "uid="); len(str) < len(opt) {
+			if str, ok := strings.CutPrefix(opt, "uid="); ok {
 				uid, err := strconv.Atoi(str)
 				if err != nil {
 					// Ignore unknown mount options.
 					continue
 				}
 				if _, err := config.HostUID(uid); err != nil {
-					return fmt.Errorf("cannot specify uid=%d mount option for rootless container: %w", uid, err)
+					return fmt.Errorf("cannot specify %s mount option for rootless container: %w", opt, err)
 				}
-			}
-
-			if str := strings.TrimPrefix(opt, "gid="); len(str) < len(opt) {
+			} else if str, ok := strings.CutPrefix(opt, "gid="); ok {
 				gid, err := strconv.Atoi(str)
 				if err != nil {
 					// Ignore unknown mount options.
 					continue
 				}
 				if _, err := config.HostGID(gid); err != nil {
-					return fmt.Errorf("cannot specify gid=%d mount option for rootless container: %w", gid, err)
+					return fmt.Errorf("cannot specify %s mount option for rootless container: %w", opt, err)
 				}
 			}
 		}

libcontainer/specconv/spec_linux.go

@@ -685,8 +685,8 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
 	var sp []systemdDbus.Property
 
 	for k, v := range spec.Annotations {
-		name := strings.TrimPrefix(k, keyPrefix)
-		if len(name) == len(k) { // prefix not there
+		name, ok := strings.CutPrefix(k, keyPrefix)
+		if !ok { // prefix not there
 			continue
 		}
 		if err := checkPropertyName(name); err != nil {

libcontainer/utils/utils.go

@@ -77,7 +77,7 @@ func stripRoot(root, path string) string {
 		path = "/"
 	case root == "/":
 		// do nothing
-	case strings.HasPrefix(path, root+"/"):
+	default:
 		path = strings.TrimPrefix(path, root+"/")
 	}
 	return CleanPath("/" + path)
@@ -88,8 +88,8 @@ func stripRoot(root, path string) string {
 func SearchLabels(labels []string, key string) (string, bool) {
 	key += "="
 	for _, s := range labels {
-		if strings.HasPrefix(s, key) {
-			return s[len(key):], true
+		if val, ok := strings.CutPrefix(s, key); ok {
+			return val, true
 		}
 	}
 	return "", false