Skip to content

Commit 0f88286

Browse files
authored
Merge pull request #4470 from kolyshkin/strings-cut
Use strings.Cut and strings.CutPrefix where possible
2 parents 6400bee + 746a5c2 commit 0f88286

File tree

17 files changed

+97
-109
lines changed

17 files changed

+97
-109
lines changed

exec.go

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,27 +117,36 @@ following will output a list of processes running in the container:
117117
SkipArgReorder: true,
118118
}
119119

120+
// getSubCgroupPaths parses --cgroup arguments, which can either be
121+
// - a single "path" argument (for cgroup v2);
122+
// - one or more controller[,controller[,...]]:path arguments (for cgroup v1).
123+
//
124+
// Returns a controller to path map. For cgroup v2, it's a single entity map
125+
// with empty controller value.
120126
func getSubCgroupPaths(args []string) (map[string]string, error) {
121127
if len(args) == 0 {
122128
return nil, nil
123129
}
124130
paths := make(map[string]string, len(args))
125131
for _, c := range args {
126132
// Split into controller:path.
127-
cs := strings.SplitN(c, ":", 3)
128-
if len(cs) > 2 {
129-
return nil, fmt.Errorf("invalid --cgroup argument: %s", c)
130-
}
131-
if len(cs) == 1 { // no controller: prefix
133+
if ctr, path, ok := strings.Cut(c, ":"); ok {
134+
// There may be a few comma-separated controllers.
135+
for _, ctrl := range strings.Split(ctr, ",") {
136+
if ctrl == "" {
137+
return nil, fmt.Errorf("invalid --cgroup argument: %s (empty <controller> prefix)", c)
138+
}
139+
if _, ok := paths[ctrl]; ok {
140+
return nil, fmt.Errorf("invalid --cgroup argument(s): controller %s specified multiple times", ctrl)
141+
}
142+
paths[ctrl] = path
143+
}
144+
} else {
145+
// No "controller:" prefix (cgroup v2, a single path).
132146
if len(args) != 1 {
133147
return nil, fmt.Errorf("invalid --cgroup argument: %s (missing <controller>: prefix)", c)
134148
}
135149
paths[""] = c
136-
} else {
137-
// There may be a few comma-separated controllers.
138-
for _, ctrl := range strings.Split(cs[0], ",") {
139-
paths[ctrl] = cs[1]
140-
}
141150
}
142151
}
143152
return paths, nil

libcontainer/cgroups/devices/systemd.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,7 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) {
224224
continue
225225
}
226226

227-
group := strings.TrimPrefix(line, ruleMajorStr)
228-
if len(group) < len(line) { // got it
227+
if group, ok := strings.CutPrefix(line, ruleMajorStr); ok {
229228
return prefix + group, nil
230229
}
231230
}

libcontainer/cgroups/file.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ func openFile(dir, file string, flags int) (*os.File, error) {
151151
if prepareOpenat2() != nil {
152152
return openFallback(path, flags, mode)
153153
}
154-
relPath := strings.TrimPrefix(path, cgroupfsPrefix)
155-
if len(relPath) == len(path) { // non-standard path, old system?
154+
relPath, ok := strings.CutPrefix(path, cgroupfsPrefix)
155+
if !ok { // Non-standard path, old system?
156156
return openFallback(path, flags, mode)
157157
}
158158

libcontainer/cgroups/fs/cpuacct.go

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,7 @@ import (
1111
)
1212

1313
const (
14-
cgroupCpuacctStat = "cpuacct.stat"
15-
cgroupCpuacctUsageAll = "cpuacct.usage_all"
16-
17-
nanosecondsInSecond = 1000000000
18-
19-
userModeColumn = 1
20-
kernelModeColumn = 2
21-
cuacctUsageAllColumnsNumber = 3
14+
nsInSec = 1000000000
2215

2316
// The value comes from `C.sysconf(C._SC_CLK_TCK)`, and
2417
// on Linux it's a constant which is safe to be hard coded,
@@ -80,7 +73,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
8073
const (
8174
userField = "user"
8275
systemField = "system"
83-
file = cgroupCpuacctStat
76+
file = "cpuacct.stat"
8477
)
8578

8679
// Expected format:
@@ -102,7 +95,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
10295
return 0, 0, &parseError{Path: path, File: file, Err: err}
10396
}
10497

105-
return (userModeUsage * nanosecondsInSecond) / clockTicks, (kernelModeUsage * nanosecondsInSecond) / clockTicks, nil
98+
return (userModeUsage * nsInSec) / clockTicks, (kernelModeUsage * nsInSec) / clockTicks, nil
10699
}
107100

108101
func getPercpuUsage(path string) ([]uint64, error) {
@@ -112,7 +105,6 @@ func getPercpuUsage(path string) ([]uint64, error) {
112105
if err != nil {
113106
return percpuUsage, err
114107
}
115-
// TODO: use strings.SplitN instead.
116108
for _, value := range strings.Fields(data) {
117109
value, err := strconv.ParseUint(value, 10, 64)
118110
if err != nil {
@@ -126,7 +118,7 @@ func getPercpuUsage(path string) ([]uint64, error) {
126118
func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) {
127119
usageKernelMode := []uint64{}
128120
usageUserMode := []uint64{}
129-
const file = cgroupCpuacctUsageAll
121+
const file = "cpuacct.usage_all"
130122

131123
fd, err := cgroups.OpenFile(path, file, os.O_RDONLY)
132124
if os.IsNotExist(err) {
@@ -140,22 +132,23 @@ func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) {
140132
scanner.Scan() // skipping header line
141133

142134
for scanner.Scan() {
143-
lineFields := strings.SplitN(scanner.Text(), " ", cuacctUsageAllColumnsNumber+1)
144-
if len(lineFields) != cuacctUsageAllColumnsNumber {
135+
// Each line is: cpu user system
136+
fields := strings.SplitN(scanner.Text(), " ", 3)
137+
if len(fields) != 3 {
145138
continue
146139
}
147140

148-
usageInKernelMode, err := strconv.ParseUint(lineFields[kernelModeColumn], 10, 64)
141+
user, err := strconv.ParseUint(fields[1], 10, 64)
149142
if err != nil {
150143
return nil, nil, &parseError{Path: path, File: file, Err: err}
151144
}
152-
usageKernelMode = append(usageKernelMode, usageInKernelMode)
145+
usageUserMode = append(usageUserMode, user)
153146

154-
usageInUserMode, err := strconv.ParseUint(lineFields[userModeColumn], 10, 64)
147+
kernel, err := strconv.ParseUint(fields[2], 10, 64)
155148
if err != nil {
156149
return nil, nil, &parseError{Path: path, File: file, Err: err}
157150
}
158-
usageUserMode = append(usageUserMode, usageInUserMode)
151+
usageKernelMode = append(usageKernelMode, kernel)
159152
}
160153
if err := scanner.Err(); err != nil {
161154
return nil, nil, &parseError{Path: path, File: file, Err: err}

libcontainer/cgroups/fs/cpuacct_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ func TestCpuacctStats(t *testing.T) {
5353
962250696038415, 981956408513304, 1002658817529022, 994937703492523,
5454
874843781648690, 872544369885276, 870104915696359, 870202363887496,
5555
},
56-
UsageInKernelmode: (uint64(291429664) * nanosecondsInSecond) / clockTicks,
57-
UsageInUsermode: (uint64(452278264) * nanosecondsInSecond) / clockTicks,
56+
UsageInKernelmode: (uint64(291429664) * nsInSec) / clockTicks,
57+
UsageInUsermode: (uint64(452278264) * nsInSec) / clockTicks,
5858
}
5959

6060
if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) {
@@ -86,8 +86,8 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) {
8686
},
8787
PercpuUsageInKernelmode: []uint64{},
8888
PercpuUsageInUsermode: []uint64{},
89-
UsageInKernelmode: (uint64(291429664) * nanosecondsInSecond) / clockTicks,
90-
UsageInUsermode: (uint64(452278264) * nanosecondsInSecond) / clockTicks,
89+
UsageInKernelmode: (uint64(291429664) * nsInSec) / clockTicks,
90+
UsageInUsermode: (uint64(452278264) * nsInSec) / clockTicks,
9191
}
9292

9393
if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) {

libcontainer/cgroups/fs/cpuset.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,26 +48,23 @@ func getCpusetStat(path string, file string) ([]uint16, error) {
4848
}
4949

5050
for _, s := range strings.Split(fileContent, ",") {
51-
sp := strings.SplitN(s, "-", 3)
52-
switch len(sp) {
53-
case 3:
54-
return extracted, &parseError{Path: path, File: file, Err: errors.New("extra dash")}
55-
case 2:
56-
min, err := strconv.ParseUint(sp[0], 10, 16)
51+
fromStr, toStr, ok := strings.Cut(s, "-")
52+
if ok {
53+
from, err := strconv.ParseUint(fromStr, 10, 16)
5754
if err != nil {
5855
return extracted, &parseError{Path: path, File: file, Err: err}
5956
}
60-
max, err := strconv.ParseUint(sp[1], 10, 16)
57+
to, err := strconv.ParseUint(toStr, 10, 16)
6158
if err != nil {
6259
return extracted, &parseError{Path: path, File: file, Err: err}
6360
}
64-
if min > max {
65-
return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, min > max")}
61+
if from > to {
62+
return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, from > to")}
6663
}
67-
for i := min; i <= max; i++ {
64+
for i := from; i <= to; i++ {
6865
extracted = append(extracted, uint16(i))
6966
}
70-
case 1:
67+
} else {
7168
value, err := strconv.ParseUint(s, 10, 16)
7269
if err != nil {
7370
return extracted, &parseError{Path: path, File: file, Err: err}

libcontainer/cgroups/fs2/defaultpath.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,9 @@ func parseCgroupFile(path string) (string, error) {
8383
func parseCgroupFromReader(r io.Reader) (string, error) {
8484
s := bufio.NewScanner(r)
8585
for s.Scan() {
86-
var (
87-
text = s.Text()
88-
parts = strings.SplitN(text, ":", 3)
89-
)
90-
if len(parts) < 3 {
91-
return "", fmt.Errorf("invalid cgroup entry: %q", text)
92-
}
93-
// text is like "0::/user.slice/user-1001.slice/session-1.scope"
94-
if parts[0] == "0" && parts[1] == "" {
95-
return parts[2], nil
86+
// "0::/user.slice/user-1001.slice/session-1.scope"
87+
if path, ok := strings.CutPrefix(s.Text(), "0::"); ok {
88+
return path, nil
9689
}
9790
}
9891
if err := s.Err(); err != nil {

libcontainer/cgroups/fs2/freezer.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,7 @@ func waitFrozen(dirPath string) (cgroups.FreezerState, error) {
104104
if i == maxIter {
105105
return cgroups.Undefined, fmt.Errorf("timeout of %s reached waiting for the cgroup to freeze", waitTime*maxIter)
106106
}
107-
line := scanner.Text()
108-
val := strings.TrimPrefix(line, "frozen ")
109-
if val != line { // got prefix
107+
if val, ok := strings.CutPrefix(scanner.Text(), "frozen "); ok {
110108
if val[0] == '1' {
111109
return cgroups.Frozen, nil
112110
}

libcontainer/cgroups/fs2/fs2.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,11 +237,10 @@ func (m *Manager) setUnified(res map[string]string) error {
237237
if errors.Is(err, os.ErrPermission) || errors.Is(err, os.ErrNotExist) {
238238
// Check if a controller is available,
239239
// to give more specific error if not.
240-
sk := strings.SplitN(k, ".", 2)
241-
if len(sk) != 2 {
240+
c, _, ok := strings.Cut(k, ".")
241+
if !ok {
242242
return fmt.Errorf("unified resource %q must be in the form CONTROLLER.PARAMETER", k)
243243
}
244-
c := sk[0]
245244
if _, ok := m.controllers[c]; !ok && c != "cgroup" {
246245
return fmt.Errorf("unified resource %q can't be set: controller %q not available", k, c)
247246
}

libcontainer/cgroups/fs2/psi.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,29 +58,29 @@ func statPSI(dirPath string, file string) (*cgroups.PSIStats, error) {
5858
func parsePSIData(psi []string) (cgroups.PSIData, error) {
5959
data := cgroups.PSIData{}
6060
for _, f := range psi {
61-
kv := strings.SplitN(f, "=", 2)
62-
if len(kv) != 2 {
61+
key, val, ok := strings.Cut(f, "=")
62+
if !ok {
6363
return data, fmt.Errorf("invalid psi data: %q", f)
6464
}
6565
var pv *float64
66-
switch kv[0] {
66+
switch key {
6767
case "avg10":
6868
pv = &data.Avg10
6969
case "avg60":
7070
pv = &data.Avg60
7171
case "avg300":
7272
pv = &data.Avg300
7373
case "total":
74-
v, err := strconv.ParseUint(kv[1], 10, 64)
74+
v, err := strconv.ParseUint(val, 10, 64)
7575
if err != nil {
76-
return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err)
76+
return data, fmt.Errorf("invalid %s PSI value: %w", key, err)
7777
}
7878
data.Total = v
7979
}
8080
if pv != nil {
81-
v, err := strconv.ParseFloat(kv[1], 64)
81+
v, err := strconv.ParseFloat(val, 64)
8282
if err != nil {
83-
return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err)
83+
return data, fmt.Errorf("invalid %s PSI value: %w", key, err)
8484
}
8585
*pv = v
8686
}

0 commit comments

Comments
 (0)