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: 29dc0cb9c9c63d8a008960b4527d6aa6798c1c17
GitHub-Pull-Request: golang/go#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>

diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go
index 1871814c6e00d5c953444772d17516ab4ccd438b..23edd5a4be638a8c593830fdcd4335c687f5a8b1 100644 (file)
--- 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
index 5e8cf85fd90ab9cf3b1ce90066f49984e3aa8c26..200a357bc0433fdc3359d77b20f53b803741402e 100644 (file)
--- 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
index 11c558c4c3b28fa5d1db16bcf5e4cfe2bdc3d619..d8cc09a24e165382fe85e887e8514218066ff86a 100644 (file)
--- 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
index ac493c7dcd43b0d96149641129c2f06d3c62ab19..4bd3a3c2029621bec29b96fa787fc30e44609a46 100644 (file)
--- 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 {