code review 7038051: src: Use bytes.Equal instead of bytes.Compare where pos...

src: Use bytes.Equal instead of bytes.Compare where possible.

bytes.Equal is simpler to read and should also be faster because
of short-circuiting and assembly implementations.

Change generated automatically using:
  gofmt -r 'bytes.Compare(a, b) == 0 -> bytes.Equal(a, b)'
  gofmt -r 'bytes.Compare(a, b) != 0 -> !bytes.Equal(a, b)'

[mdempsky, 2013-01-03 22:59:47]

Hello golang-dev@googlegroups.com,

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

[dave_cheney.net, 2013-01-03 23:00:43]

Can you please update the description with benchmarks for the packages
affected.

On Fri, Jan 4, 2013 at 9:59 AM,  <mdempsky@google.com> wrote:
> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> Use bytes.Equal instead of bytes.Compare where possible.
>
> bytes.Equal is simpler to read and should also be faster because
> of short-circuiting and assembly implementations.
>
> Change generated automatically using:
>   gofmt -r 'bytes.Compare(a, b) == 0 -> bytes.Equal(a, b)'
>   gofmt -r 'bytes.Compare(a, b) != 0 -> !bytes.Equal(a, b)'
>
> Please review this at https://codereview.appspot.com/7038051/
>
> Affected files:
>   M src/pkg/bufio/bufio_test.go
>   M src/pkg/crypto/rsa/pkcs1v15_test.go
>   M src/pkg/crypto/rsa/rsa_test.go
>   M src/pkg/encoding/asn1/asn1_test.go
>   M src/pkg/encoding/asn1/marshal_test.go
>   M src/pkg/encoding/hex/hex_test.go
>   M src/pkg/encoding/json/decode_test.go
>   M src/pkg/encoding/json/scanner_test.go
>   M src/pkg/exp/locale/collate/collate_test.go
>   M src/pkg/exp/locale/collate/maketables.go
>   M src/pkg/go/doc/doc_test.go
>   M src/pkg/math/big/int_test.go
>
>
> Index: src/pkg/bufio/bufio_test.go
> ===================================================================
> --- a/src/pkg/bufio/bufio_test.go
> +++ b/src/pkg/bufio/bufio_test.go
> @@ -748,7 +748,7 @@
>         b := NewReaderSize(strings.NewReader(input), minReadBufferSize)
>         for i, e := range expect {
>                 line, isPrefix, err := b.ReadLine()
> -               if bytes.Compare(line, e.line) != 0 {
> +               if !bytes.Equal(line, e.line) {
>                         t.Errorf("%q call %d, line == %q, want %q", input,
> i, line, e.line)
>                         return
>                 }
> Index: src/pkg/crypto/rsa/pkcs1v15_test.go
> ===================================================================
> --- a/src/pkg/crypto/rsa/pkcs1v15_test.go
> +++ b/src/pkg/crypto/rsa/pkcs1v15_test.go
> @@ -57,7 +57,7 @@
>                         t.Errorf("#%d error decrypting", i)
>                 }
>                 want := []byte(test.out)
> -               if bytes.Compare(out, want) != 0 {
> +               if !bytes.Equal(out, want) {
>                         t.Errorf("#%d got:%#v want:%#v", i, out, want)
>                 }
>         }
> @@ -90,7 +90,7 @@
>                         return false
>                 }
>
> -               if bytes.Compare(plaintext, in) != 0 {
> +               if !bytes.Equal(plaintext, in) {
>                         t.Errorf("output mismatch: %#v %#v", plaintext, in)
>                         return false
>                 }
> @@ -132,7 +132,7 @@
>                         t.Errorf("#%d error decrypting", i)
>                 }
>                 want := []byte(test.out)
> -               if bytes.Compare(key, want) != 0 {
> +               if !bytes.Equal(key, want) {
>                         t.Errorf("#%d got:%#v want:%#v", i, key, want)
>                 }
>         }
> @@ -176,7 +176,7 @@
>                 }
>
>                 expected, _ := hex.DecodeString(test.out)
> -               if bytes.Compare(s, expected) != 0 {
> +               if !bytes.Equal(s, expected) {
>                         t.Errorf("#%d got: %x want: %x", i, s, expected)
>                 }
>         }
> Index: src/pkg/crypto/rsa/rsa_test.go
> ===================================================================
> --- a/src/pkg/crypto/rsa/rsa_test.go
> +++ b/src/pkg/crypto/rsa/rsa_test.go
> @@ -179,7 +179,7 @@
>                         if err != nil {
>                                 t.Errorf("#%d,%d error: %s", i, j, err)
>                         }
> -                       if bytes.Compare(out, message.out) != 0 {
> +                       if !bytes.Equal(out, message.out) {
>                                 t.Errorf("#%d,%d bad result: %x (want %x)",
> i, j, out, message.out)
>                         }
>                 }
> @@ -203,7 +203,7 @@
>                         out, err := DecryptOAEP(sha1, nil, private,
> message.out, nil)
>                         if err != nil {
>                                 t.Errorf("#%d,%d error: %s", i, j, err)
> -                       } else if bytes.Compare(out, message.in) != 0 {
> +                       } else if !bytes.Equal(out, message.in) {
>                                 t.Errorf("#%d,%d bad result: %#v (want
> %#v)", i, j, out, message.in)
>                         }
>
> @@ -211,7 +211,7 @@
>                         out, err = DecryptOAEP(sha1, random, private,
> message.out, nil)
>                         if err != nil {
>                                 t.Errorf("#%d,%d (blind) error: %s", i, j,
> err)
> -                       } else if bytes.Compare(out, message.in) != 0 {
> +                       } else if !bytes.Equal(out, message.in) {
>                                 t.Errorf("#%d,%d (blind) bad result: %#v
> (want %#v)", i, j, out, message.in)
>                         }
>                 }
> Index: src/pkg/encoding/asn1/asn1_test.go
> ===================================================================
> --- a/src/pkg/encoding/asn1/asn1_test.go
> +++ b/src/pkg/encoding/asn1/asn1_test.go
> @@ -124,7 +124,7 @@
>                         t.Errorf("#%d: Incorrect error result (did fail? %v,
> expected: %v)", i, err == nil, test.ok)
>                 }
>                 if err == nil {
> -                       if test.bitLength != ret.BitLength ||
> bytes.Compare(ret.Bytes, test.out) != 0 {
> +                       if test.bitLength != ret.BitLength ||
> !bytes.Equal(ret.Bytes, test.out) {
>                                 t.Errorf("#%d: Bad result: %v (expected %v
> %v)", i, ret, test.out, test.bitLength)
>                         }
>                 }
> @@ -166,7 +166,7 @@
>         for i, test := range bitStringRightAlignTests {
>                 bs := BitString{test.in, test.inlen}
>                 out := bs.RightAlign()
> -               if bytes.Compare(out, test.out) != 0 {
> +               if !bytes.Equal(out, test.out) {
>                         t.Errorf("#%d got: %x want: %x", i, out, test.out)
>                 }
>         }
> @@ -477,7 +477,7 @@
>         if s.A != 0x50 {
>                 t.Errorf("bad value for A: got %d want %d", s.A, 0x50)
>         }
> -       if bytes.Compare([]byte(s.Raw), input) != 0 {
> +       if !bytes.Equal([]byte(s.Raw), input) {
>                 t.Errorf("bad value for Raw: got %x want %x", s.Raw, input)
>         }
>  }
> Index: src/pkg/encoding/asn1/marshal_test.go
> ===================================================================
> --- a/src/pkg/encoding/asn1/marshal_test.go
> +++ b/src/pkg/encoding/asn1/marshal_test.go
> @@ -132,7 +132,7 @@
>                         t.Errorf("#%d failed: %s", i, err)
>                 }
>                 out, _ := hex.DecodeString(test.out)
> -               if bytes.Compare(out, data) != 0 {
> +               if !bytes.Equal(out, data) {
>                         t.Errorf("#%d got: %x want %x\n\t%q\n\t%q", i, data,
> out, data, out)
>
>                 }
> Index: src/pkg/encoding/hex/hex_test.go
> ===================================================================
> --- a/src/pkg/encoding/hex/hex_test.go
> +++ b/src/pkg/encoding/hex/hex_test.go
> @@ -65,7 +65,7 @@
>                         t.Errorf("#%d: unexpected err value: %s", i, err)
>                         continue
>                 }
> -               if bytes.Compare(dst, test.dec) != 0 {
> +               if !bytes.Equal(dst, test.dec) {
>                         t.Errorf("#%d: got: %#v want: #%v", i, dst,
> test.dec)
>                 }
>         }
> Index: src/pkg/encoding/json/decode_test.go
> ===================================================================
> --- a/src/pkg/encoding/json/decode_test.go
> +++ b/src/pkg/encoding/json/decode_test.go
> @@ -422,7 +422,7 @@
>         if err != nil {
>                 t.Fatalf("Marshal: %v", err)
>         }
> -       if bytes.Compare(jsonBig, b) != 0 {
> +       if !bytes.Equal(jsonBig, b) {
>                 t.Errorf("Marshal jsonBig")
>                 diff(t, b, jsonBig)
>                 return
> @@ -474,7 +474,7 @@
>         if err := Unmarshal(b, &s1); err != nil {
>                 t.Fatalf("Unmarshal: %v", err)
>         }
> -       if bytes.Compare(s0, s1) != 0 {
> +       if !bytes.Equal(s0, s1) {
>                 t.Errorf("Marshal large byte slice")
>                 diff(t, s0, s1)
>         }
> Index: src/pkg/encoding/json/scanner_test.go
> ===================================================================
> --- a/src/pkg/encoding/json/scanner_test.go
> +++ b/src/pkg/encoding/json/scanner_test.go
> @@ -92,7 +92,7 @@
>                 t.Fatalf("Compact: %v", err)
>         }
>         b := buf.Bytes()
> -       if bytes.Compare(b, jsonBig) != 0 {
> +       if !bytes.Equal(b, jsonBig) {
>                 t.Error("Compact(jsonBig) != jsonBig")
>                 diff(t, b, jsonBig)
>                 return
> @@ -118,7 +118,7 @@
>                 t.Fatalf("Indent2: %v", err)
>         }
>         b1 := buf1.Bytes()
> -       if bytes.Compare(b1, b) != 0 {
> +       if !bytes.Equal(b1, b) {
>                 t.Error("Indent(Indent(jsonBig)) != Indent(jsonBig)")
>                 diff(t, b1, b)
>                 return
> @@ -130,7 +130,7 @@
>                 t.Fatalf("Compact: %v", err)
>         }
>         b1 = buf1.Bytes()
> -       if bytes.Compare(b1, jsonBig) != 0 {
> +       if !bytes.Equal(b1, jsonBig) {
>                 t.Error("Compact(Indent(jsonBig)) != jsonBig")
>                 diff(t, b1, jsonBig)
>                 return
> Index: src/pkg/exp/locale/collate/collate_test.go
> ===================================================================
> --- a/src/pkg/exp/locale/collate/collate_test.go
> +++ b/src/pkg/exp/locale/collate/collate_test.go
> @@ -388,10 +388,10 @@
>         }
>         // Separate generation from testing to ensure buffers are not
> overwritten.
>         for i, tt := range keyTests {
> -               if bytes.Compare(keys1[i], tt.out) != 0 {
> +               if !bytes.Equal(keys1[i], tt.out) {
>                         t.Errorf("%d: Key(%q) = %d; want %d", i, tt.in,
> keys1[i], tt.out)
>                 }
> -               if bytes.Compare(keys2[i], tt.out) != 0 {
> +               if !bytes.Equal(keys2[i], tt.out) {
>                         t.Errorf("%d: KeyFromString(%q) = %d; want %d", i,
> tt.in, keys2[i], tt.out)
>                 }
>         }
> Index: src/pkg/exp/locale/collate/maketables.go
> ===================================================================
> --- a/src/pkg/exp/locale/collate/maketables.go
> +++ b/src/pkg/exp/locale/collate/maketables.go
> @@ -674,7 +674,7 @@
>         for _, str := range testInput.values() {
>                 k0 := c0.KeyFromString(&buf, str)
>                 k := c.KeyFromString(&buf, str)
> -               if bytes.Compare(k0, k) != 0 {
> +               if !bytes.Equal(k0, k) {
>                         failOnError(fmt.Errorf("test:%U: keys differ (%x vs
> %x)", []rune(str), k0, k))
>                 }
>                 buf.Reset()
> Index: src/pkg/go/doc/doc_test.go
> ===================================================================
> --- a/src/pkg/go/doc/doc_test.go
> +++ b/src/pkg/go/doc/doc_test.go
> @@ -123,7 +123,7 @@
>                 }
>
>                 // compare
> -               if bytes.Compare(got, want) != 0 {
> +               if !bytes.Equal(got, want) {
>                         t.Errorf("package %s\n\tgot:\n%s\n\twant:\n%s",
> pkg.Name, got, want)
>                 }
>         }
> Index: src/pkg/math/big/int_test.go
> ===================================================================
> --- a/src/pkg/math/big/int_test.go
> +++ b/src/pkg/math/big/int_test.go
> @@ -643,7 +643,7 @@
>
>  func checkBytes(b []byte) bool {
>         b2 := new(Int).SetBytes(b).Bytes()
> -       return bytes.Compare(b, b2) == 0
> +       return bytes.Equal(b, b2)
>  }
>
>  func TestBytes(t *testing.T) {
>
>

[dave_cheney.net, 2013-01-03 23:03:42]

On Fri, Jan 4, 2013 at 10:00 AM, Dave Cheney <dave@cheney.net> wrote:
> Can you please update the description with benchmarks for the packages
affected.

Oh, it looks like the only affected code was locale/maketables,
everything else is test code. Please ignore my request for benchmark
data, there won't be any.

Maybe this is a cause that go vet should watch for (similar to b :=
b[0:len(b)])

[adg, 2013-01-06 21:37:07]

Please make the CL description begin with this line:

src: use bytes.Equal instead of bytes.Compare where possible

(The src: prefix is important)

I will add you to the CONTRIBUTORS file.

[rsc, 2013-01-06 22:25:08]

You can add him to the CONTRIBUTORS file using 'hg sync'.

[mdempsky, 2013-01-06 22:48:36]

Hello golang-dev@googlegroups.com, dave@cheney.net, adg@golang.org,
rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[adg, 2013-01-06 23:02:56]

LGTM

[adg, 2013-01-06 23:04:00]

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

src: Use bytes.Equal instead of bytes.Compare where possible.

bytes.Equal is simpler to read and should also be faster because
of short-circuiting and assembly implementations.

Change generated automatically using:
  gofmt -r 'bytes.Compare(a, b) == 0 -> bytes.Equal(a, b)'
  gofmt -r 'bytes.Compare(a, b) != 0 -> !bytes.Equal(a, b)'

R=golang-dev, dave, adg, rsc
CC=golang-dev
https://codereview.appspot.com/7038051

Committer: Andrew Gerrand <adg@golang.org>