Replace uses of `strings.Split(N)` with `strings.Cut()` by thaJeztah · Pull Request #44381 · moby/moby

thaJeztah · 2022-11-01T15:45:23Z

strings.Cut() was added in go1.18, so now that go1.17 is EOL, it should be ok to use. For some of these I was initially a bit on the fence, but strings.Cut also is more performant, so switching made sense to do;

func BenchmarkSplit(b *testing.B) {
	b.ReportAllocs()
	data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
	for i := 0; i < b.N; i++ {
		for _, s := range data {
			_ = strings.SplitN(s, "=", 2)[0]
		}
	}
}

func BenchmarkCut(b *testing.B) {
	b.ReportAllocs()
	data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
	for i := 0; i < b.N; i++ {
		for _, s := range data {
			_, _, _ = strings.Cut(s, "=")
		}
	}
}

BenchmarkSplit
BenchmarkSplit-10    	 8244206	       128.0 ns/op	     128 B/op	       4 allocs/op
BenchmarkCut
BenchmarkCut-10      	54411998	        21.80 ns/op	       0 B/op	       0 allocs/op

I split it up per package, but see individual commits for details

tianon · 2022-11-10T20:43:35Z

api/types/types.go

-			if kv[0] == "name" {
-				secopt.Name = kv[1]
+			if k == "name" {
+				secopt.Name = k


Typo 👀

Suggested change

secopt.Name = k

secopt.Name = v

whoops; fixed!

tianon · 2022-11-10T20:51:23Z

daemon/graphdriver/zfs/zfs_freebsd.go

-	if len(suffix) > 1 {
-		return id[:maxlen] + "-" + suffix[1]
+	id, suffix, _ := strings.Cut(id, "-")
+	if len(id) > 12 {


Suggested change

if len(id) > 12 {

if len(id) > maxlen {

? 👀

(I think the previous id[:maxlen] approach is reasonably harmless, right?)

Well, it panics if id would be shorter than 12; https://go.dev/play/p/IHd0vxxaYh2

Details

// You can edit this code! // Click here and start typing. package main import ( "fmt" "strings" ) func main() { fmt.Println(getMountpoint("123456789012-")) fmt.Println(getMountpoint("12345678901-")) } const maxlen = 12 func getMountpoint(id string) string { id, suffix, _ := strings.Cut(id, "-") id = id[:maxlen] if suffix != "" { // preserve filesystem suffix. id += "-" + suffix } return id }

tianon · 2022-11-10T20:52:01Z

daemon/graphdriver/zfs/zfs_freebsd.go

+	id, suffix, _ := strings.Cut(id, "-")
+	if len(id) > 12 {
+		id = id[:maxlen]
 	}


ie, this could instead be:

Suggested change

id, suffix, _ := strings.Cut(id, "-")

if len(id) > 12 {

id = id[:maxlen]

}

id, suffix, _ := strings.Cut(id, "-")

id = id[:maxlen]

See above 😅

thaJeztah

@tianon updated; PTAL

thaJeztah · 2022-11-24T09:16:50Z

api/types/types.go

-			if kv[0] == "name" {
-				secopt.Name = kv[1]
+			if k == "name" {
+				secopt.Name = k


whoops; fixed!

thaJeztah · 2022-11-24T09:29:11Z

daemon/graphdriver/zfs/zfs_freebsd.go

-	if len(suffix) > 1 {
-		return id[:maxlen] + "-" + suffix[1]
+	id, suffix, _ := strings.Cut(id, "-")
+	if len(id) > 12 {


Well, it panics if id would be shorter than 12; https://go.dev/play/p/IHd0vxxaYh2

Details

// You can edit this code! // Click here and start typing. package main import ( "fmt" "strings" ) func main() { fmt.Println(getMountpoint("123456789012-")) fmt.Println(getMountpoint("12345678901-")) } const maxlen = 12 func getMountpoint(id string) string { id, suffix, _ := strings.Cut(id, "-") id = id[:maxlen] if suffix != "" { // preserve filesystem suffix. id += "-" + suffix } return id }

thaJeztah · 2022-11-24T09:29:36Z

daemon/graphdriver/zfs/zfs_freebsd.go

+	id, suffix, _ := strings.Cut(id, "-")
+	if len(id) > 12 {
+		id = id[:maxlen]
 	}


See above 😅

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

We're looking for a specific prefix, so remove the prefix instead. Also remove redundant error-wrapping, as `os.Open()` already provides details in the error returned; open /no/such/file: no such file or directory open /etc/os-release: permission denied Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Fixes a (theoretical?) panic if ID would be shorter than 12 characters. Also trim the ID _after_ cutting off the suffix. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2023-01-15T12:13:25Z

cmd/dockerd/daemon.go

+		proto, addr, ok := strings.Cut(protoAddr, "://")
+		if !ok || addr == "" {
 			return nil, fmt.Errorf("bad format %s, expected PROTO://ADDR", protoAddr)


Turned out this one was overly restrictive, as (contrary to the error message) ADDR is optional here (for fd:// scheme); see #44823

thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Nov 1, 2022

thaJeztah added this to the v-next milestone Nov 1, 2022

This comment was marked as outdated.

Sign in to view

thaJeztah mentioned this pull request Nov 1, 2022

api/types/filters: add output to example #44383

Merged

thaJeztah force-pushed the strings_cut branch 2 times, most recently from b2e2f3c to 07702c1 Compare November 1, 2022 19:12

thaJeztah marked this pull request as ready for review November 1, 2022 21:22

thaJeztah requested review from cpuguy83, johnstep and tonistiigi as code owners November 1, 2022 21:22

cpuguy83 approved these changes Nov 1, 2022

View reviewed changes

tianon reviewed Nov 10, 2022

View reviewed changes

thaJeztah force-pushed the strings_cut branch from 07702c1 to 675fb1c Compare November 24, 2022 09:36

thaJeztah commented Nov 24, 2022

View reviewed changes

thaJeztah mentioned this pull request Nov 28, 2022

builder: handle host-gateway with extra hosts #44537

Merged

thaJeztah force-pushed the strings_cut branch from 675fb1c to 1defe9f Compare November 30, 2022 11:02

thaJeztah mentioned this pull request Nov 30, 2022

api: build: improve validation of SecurityOpt #44236

Open

thaJeztah force-pushed the strings_cut branch from 1defe9f to eebfcec Compare December 3, 2022 13:43

thaJeztah added 11 commits December 21, 2022 11:08

daemon/cluster: use strings.Cut()

c545473

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

graphdriver: windows: use strings.Cut()

6db4af7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/types/filters: use strings.Cut()

ba9bcf9

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/types/time: use strings.Cut()

7841ce7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/types: use strings.Cut()

380a2df

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

builder/remotecontext/git: use strings.Cut()

604a2d6

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

builder/dockerfile: use strings.Cut()

59880cb

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

pkg/tarsum: use strings.Cut() and minor refactor

19db33a

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

volume: use strings.Cut() and minor refactor

64adea1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client: use strings.Cut()

19cd5ff

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cmd/dockerd: use strings.Cut()

5008409

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah added 17 commits December 21, 2022 11:09

daemon/events: use strings.Cut()

32c4c15

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/logger: use strings.Cut()

ad7f1a8

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/links: use strings.Cut()

39f59c3

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration/internal: use strings.Cut() and minor refactor

ebda2fa

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration-cli: links: use gotest.tools compare and assert.Check

38b0c47

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration-cli: use strings.Cut()

d3cd746

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

opts: use strings.Cut() and refactor parseDaemonHost()

774cd9a

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

pkg/parsers/kernel: use strings.Cut() and minor refactor

451b857

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libcontainerd/local: use strings.Cut()

01365cb

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

plugin: use strings.Cut()

bffb356

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

runconfig/opts: use strings.Cut()

ff447f4

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libnetwork/datastore: rename var that collided with import

87ca949

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libnetwork: use strings.Cut() and minor refactor

46f7c92

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon: use strings.Cut() and cleanup error messages

f95e9b6

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/graphdriver/zfs: use strings.Cut, and refactor

3f935d0

Fixes a (theoretical?) panic if ID would be shorter than 12 characters. Also trim the ID _after_ cutting off the suffix. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

pkg/parsers: use strings.Cut(), and cleanup error-messages

6a91e09

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the strings_cut branch from eebfcec to 6a91e09 Compare December 21, 2022 10:09

tianon approved these changes Dec 21, 2022

View reviewed changes

tianon merged commit 3a5598a into moby:master Dec 21, 2022

thaJeztah mentioned this pull request Dec 27, 2022

Replace uses of strings.Split(N) with strings.Cut() docker/cli#3936

Merged

thaJeztah deleted the strings_cut branch January 15, 2023 00:20

thaJeztah mentioned this pull request Jan 15, 2023

fix: the logic of the protocol in loadListeners #44823

Closed

thaJeztah commented Jan 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace uses of `strings.Split(N)` with `strings.Cut()`#44381

Replace uses of `strings.Split(N)` with `strings.Cut()`#44381
tianon merged 28 commits intomoby:masterfrom
thaJeztah:strings_cut

thaJeztah commented Nov 1, 2022

Uh oh!

This comment was marked as outdated.

tianon Nov 10, 2022

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

tianon Nov 10, 2022

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

tianon Nov 10, 2022

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

thaJeztah Nov 24, 2022

Uh oh!

thaJeztah Jan 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thaJeztah commented Nov 1, 2022

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants