strings,bytes: improve Repeat panic messages

The Repeat("-", maxInt) call should produce panic: runtime error: makeslice: len out of range instead of panic: strings: Repeat output length overflow This PR is only for theory perfection. Change-Id: If67d87b147d666fbbb7238656f2a0cb6cf1dbb5b GitHub-Last-Rev: 29dc0cb GitHub-Pull-Request: #67068 Reviewed-on: https://go-review.googlesource.com/c/go/+/581936 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
golang · May 3, 2024 · 44b54b9 · 44b54b9
1 parent 2f5b420
commit 44b54b9
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 34 deletions.
diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go
@@ -583,7 +583,7 @@ func Repeat(b []byte, count int) []byte {
 	if count < 0 {
 		panic("bytes: negative Repeat count")
 	}
-	if len(b) >= maxInt/count {
+	if len(b) > maxInt/count {
 		panic("bytes: Repeat output length overflow")
 	}
 	n := len(b) * count

diff --git a/src/bytes/bytes_test.go b/src/bytes/bytes_test.go
@@ -1242,33 +1242,48 @@ func repeat(b []byte, count int) (err error) {
 
 // See Issue golang.org/issue/16237
 func TestRepeatCatchesOverflow(t *testing.T) {
-	tests := [...]struct {
+	type testCase struct {
 		s      string
 		count  int
 		errStr string
-	}{
+	}
+
+	runTestCases := func(prefix string, tests []testCase) {
+		for i, tt := range tests {
+			err := repeat([]byte(tt.s), tt.count)
+			if tt.errStr == "" {
+				if err != nil {
+					t.Errorf("#%d panicked %v", i, err)
+				}
+				continue
+			}
+
+			if err == nil || !strings.Contains(err.Error(), tt.errStr) {
+				t.Errorf("%s#%d got %q want %q", prefix, i, err, tt.errStr)
+			}
+		}
+	}
+
+	const maxInt = int(^uint(0) >> 1)
+
+	runTestCases("", []testCase{
 		0: {"--", -2147483647, "negative"},
-		1: {"", int(^uint(0) >> 1), ""},
+		1: {"", maxInt, ""},
 		2: {"-", 10, ""},
 		3: {"gopher", 0, ""},
 		4: {"-", -1, "negative"},
 		5: {"--", -102, "negative"},
 		6: {string(make([]byte, 255)), int((^uint(0))/255 + 1), "overflow"},
-	}
-
-	for i, tt := range tests {
-		err := repeat([]byte(tt.s), tt.count)
-		if tt.errStr == "" {
-			if err != nil {
-				t.Errorf("#%d panicked %v", i, err)
-			}
-			continue
-		}
+	})
 
-		if err == nil || !strings.Contains(err.Error(), tt.errStr) {
-			t.Errorf("#%d expected %q got %q", i, tt.errStr, err)
-		}
+	const is64Bit = 1<<(^uintptr(0)>>63)/2 != 0
+	if !is64Bit {
+		return
 	}
+
+	runTestCases("64-bit", []testCase{
+		0: {"-", maxInt, "out of range"},
+	})
 }
 
 func runesEqual(a, b []rune) bool {

diff --git a/src/strings/strings.go b/src/strings/strings.go
@@ -570,7 +570,7 @@ func Repeat(s string, count int) string {
 	if count < 0 {
 		panic("strings: negative Repeat count")
 	}
-	if len(s) >= maxInt/count {
+	if len(s) > maxInt/count {
 		panic("strings: Repeat output length overflow")
 	}
 	n := len(s) * count

diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go
@@ -1170,33 +1170,48 @@ func repeat(s string, count int) (err error) {
 
 // See Issue golang.org/issue/16237
 func TestRepeatCatchesOverflow(t *testing.T) {
-	tests := [...]struct {
+	type testCase struct {
 		s      string
 		count  int
 		errStr string
-	}{
+	}
+
+	runTestCases := func(prefix string, tests []testCase) {
+		for i, tt := range tests {
+			err := repeat(tt.s, tt.count)
+			if tt.errStr == "" {
+				if err != nil {
+					t.Errorf("#%d panicked %v", i, err)
+				}
+				continue
+			}
+
+			if err == nil || !Contains(err.Error(), tt.errStr) {
+				t.Errorf("%s#%d got %q want %q", prefix, i, err, tt.errStr)
+			}
+		}
+	}
+
+	const maxInt = int(^uint(0) >> 1)
+
+	runTestCases("", []testCase{
 		0: {"--", -2147483647, "negative"},
-		1: {"", int(^uint(0) >> 1), ""},
+		1: {"", maxInt, ""},
 		2: {"-", 10, ""},
 		3: {"gopher", 0, ""},
 		4: {"-", -1, "negative"},
 		5: {"--", -102, "negative"},
 		6: {string(make([]byte, 255)), int((^uint(0))/255 + 1), "overflow"},
-	}
-
-	for i, tt := range tests {
-		err := repeat(tt.s, tt.count)
-		if tt.errStr == "" {
-			if err != nil {
-				t.Errorf("#%d panicked %v", i, err)
-			}
-			continue
-		}
+	})
 
-		if err == nil || !Contains(err.Error(), tt.errStr) {
-			t.Errorf("#%d expected %q got %q", i, tt.errStr, err)
-		}
+	const is64Bit = 1<<(^uintptr(0)>>63)/2 != 0
+	if !is64Bit {
+		return
 	}
+
+	runTestCases("64-bit", []testCase{
+		0: {"-", maxInt, "out of range"},
+	})
 }
 
 func runesEqual(a, b []rune) bool {