code review 86060044: strings: more consistent error messages

bytes, strings: more consistent error messages

[gri, 2014-04-10 23:01:33]

Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2014-04-10 23:07:01]

LGTM but do bytes.Reader too in the same CL



On Thu, Apr 10, 2014 at 4:01 PM, <gri@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> strings: more consistent error messages
>
> Please review this at https://codereview.appspot.com/86060044/
>
> Affected files (+8, -8 lines):
>   M src/pkg/strings/reader.go
>   M src/pkg/strings/reader_test.go
>
>
> Index: src/pkg/strings/reader.go
> ===================================================================
> --- a/src/pkg/strings/reader.go
> +++ b/src/pkg/strings/reader.go
> @@ -44,7 +44,7 @@
>  func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
>         // cannot modify state - see io.ReaderAt
>         if off < 0 {
> -               return 0, errors.New("strings: invalid offset")
> +               return 0, errors.New("strings.Reader.ReadAt: negative
> offset")
>         }
>         if off >= int64(len(r.s)) {
>                 return 0, io.EOF
> @@ -67,11 +67,11 @@
>  }
>
>  func (r *Reader) UnreadByte() error {
> +       r.prevRune = -1
>         if r.i <= 0 {
> -               return errors.New("strings.Reader: at beginning of string")
> +               return errors.New("strings.Reader.UnreadByte: at
> beginning of string")
>         }
>         r.i--
> -       r.prevRune = -1
>         return nil
>  }
>
> @@ -92,7 +92,7 @@
>
>  func (r *Reader) UnreadRune() error {
>         if r.prevRune < 0 {
> -               return errors.New("strings.Reader: previous operation was
> not ReadRune")
> +               return errors.New("strings.Reader.UnreadRune: previous
> operation was not ReadRune")
>         }
>         r.i = int64(r.prevRune)
>         r.prevRune = -1
> @@ -111,10 +111,10 @@
>         case 2:
>                 abs = int64(len(r.s)) + offset
>         default:
> -               return 0, errors.New("strings: invalid whence")
> +               return 0, errors.New("strings.Reader.Seek: invalid
> whence")
>         }
>         if abs < 0 {
> -               return 0, errors.New("strings: negative position")
> +               return 0, errors.New("strings.Reader.Seek: negative
> position")
>         }
>         r.i = abs
>         return abs, nil
> Index: src/pkg/strings/reader_test.go
> ===================================================================
> --- a/src/pkg/strings/reader_test.go
> +++ b/src/pkg/strings/reader_test.go
> @@ -27,7 +27,7 @@
>                 {seek: os.SEEK_SET, off: 0, n: 20, want: "0123456789"},
>                 {seek: os.SEEK_SET, off: 1, n: 1, want: "1"},
>                 {seek: os.SEEK_CUR, off: 1, wantpos: 3, n: 2, want: "34"},
> -               {seek: os.SEEK_SET, off: -1, seekerr: "strings: negative
> position"},
> +               {seek: os.SEEK_SET, off: -1, seekerr:
> "strings.Reader.Seek: negative position"},
>                 {seek: os.SEEK_SET, off: 1 << 33, wantpos: 1 << 33},
>                 {seek: os.SEEK_CUR, off: 1, wantpos: 1<<33 + 1},
>                 {seek: os.SEEK_SET, n: 5, want: "01234"},
> @@ -84,7 +84,7 @@
>                 {1, 9, "123456789", nil},
>                 {11, 10, "", io.EOF},
>                 {0, 0, "", nil},
> -               {-1, 0, "", "strings: invalid offset"},
> +               {-1, 0, "", "strings.Reader.ReadAt: negative offset"},
>         }
>         for i, tt := range tests {
>                 b := make([]byte, tt.n)
>
>
>

[gri, 2014-04-11 04:42:33]

done


On Thu, Apr 10, 2014 at 4:06 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> LGTM but do bytes.Reader too in the same CL
>
>
>
> On Thu, Apr 10, 2014 at 4:01 PM, <gri@golang.org> wrote:
>
>> Reviewers: bradfitz,
>>
>> Message:
>> Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>>
>> Description:
>> strings: more consistent error messages
>>
>> Please review this at https://codereview.appspot.com/86060044/
>>
>> Affected files (+8, -8 lines):
>>   M src/pkg/strings/reader.go
>>   M src/pkg/strings/reader_test.go
>>
>>
>> Index: src/pkg/strings/reader.go
>> ===================================================================
>> --- a/src/pkg/strings/reader.go
>> +++ b/src/pkg/strings/reader.go
>> @@ -44,7 +44,7 @@
>>  func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
>>         // cannot modify state - see io.ReaderAt
>>         if off < 0 {
>> -               return 0, errors.New("strings: invalid offset")
>> +               return 0, errors.New("strings.Reader.ReadAt: negative
>> offset")
>>         }
>>         if off >= int64(len(r.s)) {
>>                 return 0, io.EOF
>> @@ -67,11 +67,11 @@
>>  }
>>
>>  func (r *Reader) UnreadByte() error {
>> +       r.prevRune = -1
>>         if r.i <= 0 {
>> -               return errors.New("strings.Reader: at beginning of
>> string")
>> +               return errors.New("strings.Reader.UnreadByte: at
>> beginning of string")
>>         }
>>         r.i--
>> -       r.prevRune = -1
>>         return nil
>>  }
>>
>> @@ -92,7 +92,7 @@
>>
>>  func (r *Reader) UnreadRune() error {
>>         if r.prevRune < 0 {
>> -               return errors.New("strings.Reader: previous operation was
>> not ReadRune")
>> +               return errors.New("strings.Reader.UnreadRune: previous
>> operation was not ReadRune")
>>         }
>>         r.i = int64(r.prevRune)
>>         r.prevRune = -1
>> @@ -111,10 +111,10 @@
>>         case 2:
>>                 abs = int64(len(r.s)) + offset
>>         default:
>> -               return 0, errors.New("strings: invalid whence")
>> +               return 0, errors.New("strings.Reader.Seek: invalid
>> whence")
>>         }
>>         if abs < 0 {
>> -               return 0, errors.New("strings: negative position")
>> +               return 0, errors.New("strings.Reader.Seek: negative
>> position")
>>         }
>>         r.i = abs
>>         return abs, nil
>> Index: src/pkg/strings/reader_test.go
>> ===================================================================
>> --- a/src/pkg/strings/reader_test.go
>> +++ b/src/pkg/strings/reader_test.go
>> @@ -27,7 +27,7 @@
>>                 {seek: os.SEEK_SET, off: 0, n: 20, want: "0123456789"},
>>                 {seek: os.SEEK_SET, off: 1, n: 1, want: "1"},
>>                 {seek: os.SEEK_CUR, off: 1, wantpos: 3, n: 2, want: "34"},
>> -               {seek: os.SEEK_SET, off: -1, seekerr: "strings: negative
>> position"},
>> +               {seek: os.SEEK_SET, off: -1, seekerr:
>> "strings.Reader.Seek: negative position"},
>>                 {seek: os.SEEK_SET, off: 1 << 33, wantpos: 1 << 33},
>>                 {seek: os.SEEK_CUR, off: 1, wantpos: 1<<33 + 1},
>>                 {seek: os.SEEK_SET, n: 5, want: "01234"},
>> @@ -84,7 +84,7 @@
>>                 {1, 9, "123456789", nil},
>>                 {11, 10, "", io.EOF},
>>                 {0, 0, "", nil},
>> -               {-1, 0, "", "strings: invalid offset"},
>> +               {-1, 0, "", "strings.Reader.ReadAt: negative offset"},
>>         }
>>         for i, tt := range tests {
>>                 b := make([]byte, tt.n)
>>
>>
>>
>

[bradfitz, 2014-04-11 04:44:02]

LGTM



On Thu, Apr 10, 2014 at 9:42 PM, Robert Griesemer <gri@golang.org> wrote:

> done
>
>
> On Thu, Apr 10, 2014 at 4:06 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>
>> LGTM but do bytes.Reader too in the same CL
>>
>>
>>
>> On Thu, Apr 10, 2014 at 4:01 PM, <gri@golang.org> wrote:
>>
>>> Reviewers: bradfitz,
>>>
>>> Message:
>>> Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),
>>>
>>> I'd like you to review this change to
>>> https://code.google.com/p/go/
>>>
>>>
>>> Description:
>>> strings: more consistent error messages
>>>
>>> Please review this at https://codereview.appspot.com/86060044/
>>>
>>> Affected files (+8, -8 lines):
>>>   M src/pkg/strings/reader.go
>>>   M src/pkg/strings/reader_test.go
>>>
>>>
>>> Index: src/pkg/strings/reader.go
>>> ===================================================================
>>> --- a/src/pkg/strings/reader.go
>>> +++ b/src/pkg/strings/reader.go
>>> @@ -44,7 +44,7 @@
>>>  func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
>>>         // cannot modify state - see io.ReaderAt
>>>         if off < 0 {
>>> -               return 0, errors.New("strings: invalid offset")
>>> +               return 0, errors.New("strings.Reader.ReadAt: negative
>>> offset")
>>>         }
>>>         if off >= int64(len(r.s)) {
>>>                 return 0, io.EOF
>>> @@ -67,11 +67,11 @@
>>>  }
>>>
>>>  func (r *Reader) UnreadByte() error {
>>> +       r.prevRune = -1
>>>         if r.i <= 0 {
>>> -               return errors.New("strings.Reader: at beginning of
>>> string")
>>> +               return errors.New("strings.Reader.UnreadByte: at
>>> beginning of string")
>>>         }
>>>         r.i--
>>> -       r.prevRune = -1
>>>         return nil
>>>  }
>>>
>>> @@ -92,7 +92,7 @@
>>>
>>>  func (r *Reader) UnreadRune() error {
>>>         if r.prevRune < 0 {
>>> -               return errors.New("strings.Reader: previous operation
>>> was not ReadRune")
>>> +               return errors.New("strings.Reader.UnreadRune: previous
>>> operation was not ReadRune")
>>>         }
>>>         r.i = int64(r.prevRune)
>>>         r.prevRune = -1
>>> @@ -111,10 +111,10 @@
>>>         case 2:
>>>                 abs = int64(len(r.s)) + offset
>>>         default:
>>> -               return 0, errors.New("strings: invalid whence")
>>> +               return 0, errors.New("strings.Reader.Seek: invalid
>>> whence")
>>>         }
>>>         if abs < 0 {
>>> -               return 0, errors.New("strings: negative position")
>>> +               return 0, errors.New("strings.Reader.Seek: negative
>>> position")
>>>         }
>>>         r.i = abs
>>>         return abs, nil
>>> Index: src/pkg/strings/reader_test.go
>>> ===================================================================
>>> --- a/src/pkg/strings/reader_test.go
>>> +++ b/src/pkg/strings/reader_test.go
>>> @@ -27,7 +27,7 @@
>>>                 {seek: os.SEEK_SET, off: 0, n: 20, want: "0123456789"},
>>>                 {seek: os.SEEK_SET, off: 1, n: 1, want: "1"},
>>>                 {seek: os.SEEK_CUR, off: 1, wantpos: 3, n: 2, want:
>>> "34"},
>>> -               {seek: os.SEEK_SET, off: -1, seekerr: "strings: negative
>>> position"},
>>> +               {seek: os.SEEK_SET, off: -1, seekerr:
>>> "strings.Reader.Seek: negative position"},
>>>                 {seek: os.SEEK_SET, off: 1 << 33, wantpos: 1 << 33},
>>>                 {seek: os.SEEK_CUR, off: 1, wantpos: 1<<33 + 1},
>>>                 {seek: os.SEEK_SET, n: 5, want: "01234"},
>>> @@ -84,7 +84,7 @@
>>>                 {1, 9, "123456789", nil},
>>>                 {11, 10, "", io.EOF},
>>>                 {0, 0, "", nil},
>>> -               {-1, 0, "", "strings: invalid offset"},
>>> +               {-1, 0, "", "strings.Reader.ReadAt: negative offset"},
>>>         }
>>>         for i, tt := range tests {
>>>                 b := make([]byte, tt.n)
>>>
>>>
>>>
>>
>

[gri, 2014-04-11 04:45:46]

*** Submitted as https://code.google.com/p/go/source/detail?r=4b84ac898256 ***

bytes, strings: more consistent error messages

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://codereview.appspot.com/86060044

[gobot, 2014-04-11 07:58:58]

This CL appears to have broken the netbsd-amd64-bsiegert builder.
See http://build.golang.org/log/5ccefd71e067579a290f7f64027e01e702cbde65