code review 171120043: encoding/binary: check for unsigned integers in intDataSize.

encoding/binary: check for unsigned integers in intDataSize.

intDataSize ignores unsigned integers, forcing read/writes to miss the fast path
Fixes issue 8956.

[sps, 2014-11-06 06:51:11]

Hello golang-codereviews@googlegroups.com,

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

[bradfitz, 2014-11-06 17:50:34]

The tree is currently closed until December. Please ping this thread again
then.
 On Nov 6, 2014 1:40 PM, <stan@schwertly.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> encoding/binary: check for unsigned integers in intDataSize.
>
> intDataSize ignores unsigned integers, forcing read/writes to miss the
> fast path
> Fixes issue 8956.
>
> Please review this at https://codereview.appspot.com/171120043/
>
> Affected files (+4, -4 lines):
>   M src/encoding/binary/binary.go
>
>
> Index: src/encoding/binary/binary.go
> ===================================================================
> --- a/src/encoding/binary/binary.go
> +++ b/src/encoding/binary/binary.go
> @@ -605,25 +605,25 @@
>  // It returns zero if the type cannot be implemented by the fast path in
> Read or Write.
>  func intDataSize(data interface{}) int {
>         switch data := data.(type) {
> -       case int8, *int8, *uint8:
> +       case int8, uint8, *int8, *uint8:
>                 return 1
>         case []int8:
>                 return len(data)
>         case []uint8:
>                 return len(data)
> -       case int16, *int16, *uint16:
> +       case int16, uint16, *int16, *uint16:
>                 return 2
>         case []int16:
>                 return 2 * len(data)
>         case []uint16:
>                 return 2 * len(data)
> -       case int32, *int32, *uint32:
> +       case int32, uint32, *int32, *uint32:
>                 return 4
>         case []int32:
>                 return 4 * len(data)
>         case []uint32:
>                 return 4 * len(data)
> -       case int64, *int64, *uint64:
> +       case int64, uint64, *int64, *uint64:
>                 return 8
>         case []int64:
>                 return 8 * len(data)
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[sps, 2014-12-11 15:56:38]

Hi, pinging this thread with the new release :)! Do I have do any additional
work with the transition to Git, or is this okay to go as-is?

On 2014/11/06 17:50:34, bradfitz wrote:
> The tree is currently closed until December. Please ping this thread again
> then.
>  On Nov 6, 2014 1:40 PM, <mailto:stan@schwertly.com> wrote:
> 
> > Reviewers: golang-codereviews,
> >
> > Message:
> > Hello mailto:golang-codereviews@googlegroups.com,
> >
> > I'd like you to review this change to
> > https://code.google.com/p/go
> >
> >
> > Description:
> > encoding/binary: check for unsigned integers in intDataSize.
> >
> > intDataSize ignores unsigned integers, forcing read/writes to miss the
> > fast path
> > Fixes issue 8956.
> >
> > Please review this at https://codereview.appspot.com/171120043/
> >
> > Affected files (+4, -4 lines):
> >   M src/encoding/binary/binary.go
> >
> >
> > Index: src/encoding/binary/binary.go
> > ===================================================================
> > --- a/src/encoding/binary/binary.go
> > +++ b/src/encoding/binary/binary.go
> > @@ -605,25 +605,25 @@
> >  // It returns zero if the type cannot be implemented by the fast path in
> > Read or Write.
> >  func intDataSize(data interface{}) int {
> >         switch data := data.(type) {
> > -       case int8, *int8, *uint8:
> > +       case int8, uint8, *int8, *uint8:
> >                 return 1
> >         case []int8:
> >                 return len(data)
> >         case []uint8:
> >                 return len(data)
> > -       case int16, *int16, *uint16:
> > +       case int16, uint16, *int16, *uint16:
> >                 return 2
> >         case []int16:
> >                 return 2 * len(data)
> >         case []uint16:
> >                 return 2 * len(data)
> > -       case int32, *int32, *uint32:
> > +       case int32, uint32, *int32, *uint32:
> >                 return 4
> >         case []int32:
> >                 return 4 * len(data)
> >         case []uint32:
> >                 return 4 * len(data)
> > -       case int64, *int64, *uint64:
> > +       case int64, uint64, *int64, *uint64:
> >                 return 8
> >         case []int64:
> >                 return 8 * len(data)
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "golang-codereviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to mailto:golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

[iant, 2014-12-11 16:10:10]

On Thu, Dec 11, 2014 at 7:56 AM,  <stan@schwertly.com> wrote:
> Hi, pinging this thread with the new release :)! Do I have do any
> additional work with the transition to Git, or is this okay to go as-is?

This will need to be transitioned to the new code review system.  But
not yet.  Please wait for an announcement next week.  Thanks.

Ian



> On 2014/11/06 17:50:34, bradfitz wrote:
>>
>> The tree is currently closed until December. Please ping this thread
>
> again
>>
>> then.
>>   On Nov 6, 2014 1:40 PM, <mailto:stan@schwertly.com> wrote:
>
>
>> > Reviewers: golang-codereviews,
>> >
>> > Message:
>> > Hello mailto:golang-codereviews@googlegroups.com,
>> >
>> > I'd like you to review this change to
>> > https://code.google.com/p/go
>> >
>> >
>> > Description:
>> > encoding/binary: check for unsigned integers in intDataSize.
>> >
>> > intDataSize ignores unsigned integers, forcing read/writes to miss
>
> the
>>
>> > fast path
>> > Fixes issue 8956.
>> >
>> > Please review this at https://codereview.appspot.com/171120043/
>> >
>> > Affected files (+4, -4 lines):
>> >   M src/encoding/binary/binary.go
>> >
>> >
>> > Index: src/encoding/binary/binary.go
>> > ===================================================================
>> > --- a/src/encoding/binary/binary.go
>> > +++ b/src/encoding/binary/binary.go
>> > @@ -605,25 +605,25 @@
>> >  // It returns zero if the type cannot be implemented by the fast
>
> path in
>>
>> > Read or Write.
>> >  func intDataSize(data interface{}) int {
>> >         switch data := data.(type) {
>> > -       case int8, *int8, *uint8:
>> > +       case int8, uint8, *int8, *uint8:
>> >                 return 1
>> >         case []int8:
>> >                 return len(data)
>> >         case []uint8:
>> >                 return len(data)
>> > -       case int16, *int16, *uint16:
>> > +       case int16, uint16, *int16, *uint16:
>> >                 return 2
>> >         case []int16:
>> >                 return 2 * len(data)
>> >         case []uint16:
>> >                 return 2 * len(data)
>> > -       case int32, *int32, *uint32:
>> > +       case int32, uint32, *int32, *uint32:
>> >                 return 4
>> >         case []int32:
>> >                 return 4 * len(data)
>> >         case []uint32:
>> >                 return 4 * len(data)
>> > -       case int64, *int64, *uint64:
>> > +       case int64, uint64, *int64, *uint64:
>> >                 return 8
>> >         case []int64:
>> >                 return 8 * len(data)
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>
> Groups
>>
>> > "golang-codereviews" group.
>> > To unsubscribe from this group and stop receiving emails from it,
>
> send an
>>
>> > email to mailto:golang-codereviews+unsubscribe@googlegroups.com.
>> > For more options, visit https://groups.google.com/d/optout.
>> >
>
>
>
>
> https://codereview.appspot.com/171120043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[gobot, 2014-12-19 05:15:38]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/171120043 is best) in the description in your
new CL.

Thanks very much.