code review 5637043: all: avoid bytes.NewBuffer(nil)

all: avoid bytes.NewBuffer(nil)
The practice encourages people to think this is the way to
create a bytes.Buffer when new(bytes.Buffer) or
just var buf bytes.Buffer work fine.
(html/token.go was missing the point altogether.)

[r, 2012-02-06 00:53:52]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2012-02-06 01:08:33]

LGTM

I prefer "var buf bytes.Buffer", though.  Same number of allocations
currently, but I think reading new(bytes.Buffer) says "going to be keeping
this" vs. the meaning of a local variable.


On Sun, Feb 5, 2012 at 4:53 PM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> all: avoid bytes.NewBuffer(nil)
> The practice encourages people to think this is the way to
> create a bytes.Buffer when new(bytes.Buffer) is fine.
> (html/token.go was missing the point altogether.)
>
> Please review this at
http://codereview.appspot.com/**5637043/<http://codereview.appspot.com/5637043/>
>
> Affected files:
>  M src/pkg/bufio/bufio_test.go
>  M src/pkg/compress/flate/**deflate_test.go
>  M src/pkg/compress/lzw/reader_**test.go
>  M src/pkg/compress/zlib/writer_**test.go
>  M src/pkg/encoding/asn1/marshal.**go
>  M src/pkg/encoding/gob/encoder_**test.go
>  M src/pkg/encoding/hex/hex.go
>  M src/pkg/encoding/hex/hex_test.**go
>  M src/pkg/encoding/pem/pem.go
>  M src/pkg/encoding/pem/pem_test.**go
>  M src/pkg/exp/html/escape.go
>  M src/pkg/exp/html/parse_test.go
>  M src/pkg/exp/html/token.go
>  M src/pkg/exp/html/token_test.go
>  M src/pkg/html/escape.go
>  M src/pkg/image/jpeg/writer_**test.go
>  M src/pkg/image/png/writer_test.**go
>  M src/pkg/old/template/template_**test.go
>
>
> Index: src/pkg/bufio/bufio_test.go
> ==============================**==============================**=======
> --- a/src/pkg/bufio/bufio_test.go
> +++ b/src/pkg/bufio/bufio_test.go
> @@ -688,7 +688,7 @@
>  }
>
>  func TestReadEmptyBuffer(t *testing.T) {
> -       l, _ := NewReaderSize(bytes.NewBuffer(**nil), minReadBufferSize)
> +       l, _ := NewReaderSize(new(bytes.**Buffer), minReadBufferSize)
>        line, isPrefix, err := l.ReadLine()
>        if err != io.EOF {
>                t.Errorf("expected EOF from ReadLine, got '%s' %t %s",
> line, isPrefix, err)
> Index: src/pkg/compress/flate/**deflate_test.go
> ==============================**==============================**=======
> --- a/src/pkg/compress/flate/**deflate_test.go
> +++ b/src/pkg/compress/flate/**deflate_test.go
> @@ -229,7 +229,7 @@
>  }
>
>  func testToFromWithLevelAndLimit(t *testing.T, level int, input []byte,
> name string, limit int) error {
> -       buffer := bytes.NewBuffer(nil)
> +       buffer := new(bytes.Buffer)
>        w := NewWriter(buffer, level)
>        w.Write(input)
>        w.Close()
> Index: src/pkg/compress/lzw/reader_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/compress/lzw/reader_**test.go
> +++ b/src/pkg/compress/lzw/reader_**test.go
> @@ -81,7 +81,7 @@
>  }
>
>  func TestReader(t *testing.T) {
> -       b := bytes.NewBuffer(nil)
> +       b := new(bytes.Buffer)
>        for _, tt := range lzwTests {
>                d := strings.Split(tt.desc, ";")
>                var order Order
> @@ -116,7 +116,7 @@
>        b.SetBytes(int64(n))
>        buf0, _ := ioutil.ReadFile("../testdata/**e.txt")
>        buf0 = buf0[:10000]
> -       compressed := bytes.NewBuffer(nil)
> +       compressed := new(bytes.Buffer)
>        w := NewWriter(compressed, LSB, 8)
>        for i := 0; i < n; i += len(buf0) {
>                io.Copy(w, bytes.NewBuffer(buf0))
> Index: src/pkg/compress/zlib/writer_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/compress/zlib/**writer_test.go
> +++ b/src/pkg/compress/zlib/**writer_test.go
> @@ -124,7 +124,7 @@
>
>  func TestWriterDictIsUsed(t *testing.T) {
>        var input = []byte("Lorem ipsum dolor sit amet, consectetur
> adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore
> magna aliqua.")
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        compressor, err := NewWriterDict(buf, BestCompression, input)
>        if err != nil {
>                t.Errorf("error in NewWriterDict: %s", err)
> Index: src/pkg/encoding/asn1/marshal.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/asn1/**marshal.go
> +++ b/src/pkg/encoding/asn1/**marshal.go
> @@ -24,7 +24,7 @@
>  }
>
>  func newForkableWriter() *forkableWriter {
> -       return &forkableWriter{bytes.**NewBuffer(nil), nil, nil}
> +       return &forkableWriter{new(bytes.**Buffer), nil, nil}
>  }
>
>  func (f *forkableWriter) fork() (pre, post *forkableWriter) {
> Index: src/pkg/encoding/gob/encoder_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/gob/**encoder_test.go
> +++ b/src/pkg/encoding/gob/**encoder_test.go
> @@ -570,7 +570,7 @@
>                "bo": []bool{false},
>                "st": []string{"s"},
>        }
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        enc := NewEncoder(buf)
>        err := enc.Encode(m)
>        if err != nil {
> @@ -579,7 +579,7 @@
>  }
>
>  func TestSliceReusesMemory(t *testing.T) {
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        // Bytes
>        {
>                x := []byte("abcd")
> Index: src/pkg/encoding/hex/hex.go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/hex/hex.go
> +++ b/src/pkg/encoding/hex/hex.go
> @@ -103,7 +103,7 @@
>  // Dump returns a string that contains a hex dump of the given data. The
> format
>  // of the hex dump matches the output of `hexdump -C` on the command line.
>  func Dump(data []byte) string {
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        dumper := Dumper(buf)
>        dumper.Write(data)
>        dumper.Close()
> Index: src/pkg/encoding/hex/hex_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/hex/hex_**test.go
> +++ b/src/pkg/encoding/hex/hex_**test.go
> @@ -155,7 +155,7 @@
>        }
>
>        for stride := 1; stride < len(in); stride++ {
> -               out := bytes.NewBuffer(nil)
> +               out := new(bytes.Buffer)
>                dumper := Dumper(out)
>                done := 0
>                for done < len(in) {
> Index: src/pkg/encoding/pem/pem.go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/pem/pem.go
> +++ b/src/pkg/encoding/pem/pem.go
> @@ -251,7 +251,7 @@
>  }
>
>  func EncodeToMemory(b *Block) []byte {
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        Encode(buf, b)
>        return buf.Bytes()
>  }
> Index: src/pkg/encoding/pem/pem_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/pem/pem_**test.go
> +++ b/src/pkg/encoding/pem/pem_**test.go
> @@ -73,7 +73,7 @@
>
>  func TestLineBreaker(t *testing.T) {
>        for i, test := range lineBreakerTests {
> -               buf := bytes.NewBuffer(nil)
> +               buf := new(bytes.Buffer)
>                var breaker lineBreaker
>                breaker.out = buf
>                _, err := breaker.Write([]byte(test.in))
> @@ -93,7 +93,7 @@
>        }
>
>        for i, test := range lineBreakerTests {
> -               buf := bytes.NewBuffer(nil)
> +               buf := new(bytes.Buffer)
>                var breaker lineBreaker
>                breaker.out = buf
>
> Index: src/pkg/exp/html/escape.go
> ==============================**==============================**=======
> --- a/src/pkg/exp/html/escape.go
> +++ b/src/pkg/exp/html/escape.go
> @@ -233,7 +233,7 @@
>        if strings.IndexAny(s, escapedChars) == -1 {
>                return s
>        }
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        escape(buf, s)
>        return buf.String()
>  }
> Index: src/pkg/exp/html/parse_test.go
> ==============================**==============================**=======
> --- a/src/pkg/exp/html/parse_test.**go
> +++ b/src/pkg/exp/html/parse_test.**go
> @@ -159,7 +159,7 @@
>        if n == nil || len(n.Child) == 0 {
>                return "", nil
>        }
> -       b := bytes.NewBuffer(nil)
> +       b := new(bytes.Buffer)
>        for _, child := range n.Child {
>                if err := dumpLevel(b, child, 0); err != nil {
>                        return "", err
> Index: src/pkg/exp/html/token.go
> ==============================**==============================**=======
> --- a/src/pkg/exp/html/token.go
> +++ b/src/pkg/exp/html/token.go
> @@ -77,8 +77,7 @@
>        if len(t.Attr) == 0 {
>                return t.Data
>        }
> -       buf := bytes.NewBuffer(nil)
> -       buf.WriteString(t.Data)
> +       buf := bytes.NewBufferString(t.Data)
>        for _, a := range t.Attr {
>                buf.WriteByte(' ')
>                buf.WriteString(a.Key)
> Index: src/pkg/exp/html/token_test.go
> ==============================**==============================**=======
> --- a/src/pkg/exp/html/token_test.**go
> +++ b/src/pkg/exp/html/token_test.**go
> @@ -556,7 +556,7 @@
>  func TestBufAPI(t *testing.T) {
>        s := "0<a>1</a>2<b>3<a>4<a>5</a>6</**b>7</a>8<a/>9"
>        z := NewTokenizer(bytes.NewBuffer([**]byte(s)))
> -       result := bytes.NewBuffer(nil)
> +       result := new(bytes.Buffer)
>        depth := 0
>  loop:
>        for {
> Index: src/pkg/html/escape.go
> ==============================**==============================**=======
> --- a/src/pkg/html/escape.go
> +++ b/src/pkg/html/escape.go
> @@ -237,7 +237,7 @@
>        if strings.IndexAny(s, escapedChars) == -1 {
>                return s
>        }
> -       buf := bytes.NewBuffer(nil)
> +       buf := new(bytes.Buffer)
>        escape(buf, s)
>        return buf.String()
>  }
> Index: src/pkg/image/jpeg/writer_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/image/jpeg/writer_**test.go
> +++ b/src/pkg/image/jpeg/writer_**test.go
> @@ -54,7 +54,7 @@
>                        continue
>                }
>                // Encode that image as JPEG.
> -               buf := bytes.NewBuffer(nil)
> +               buf := new(bytes.Buffer)
>                err = Encode(buf, m0, &Options{Quality: tc.quality})
>                if err != nil {
>                        t.Error(tc.filename, err)
> Index: src/pkg/image/png/writer_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/image/png/writer_**test.go
> +++ b/src/pkg/image/png/writer_**test.go
> @@ -35,12 +35,12 @@
>  }
>
>  func encodeDecode(m image.Image) (image.Image, error) {
> -       b := bytes.NewBuffer(nil)
> -       err := Encode(b, m)
> +       buf := new(bytes.Buffer)
> +       err := Encode(buf, m)
>        if err != nil {
>                return nil, err
>        }
> -       m, err = Decode(b)
> +       m, err = Decode(buf)
>        if err != nil {
>                return nil, err
>        }
> Index: src/pkg/old/template/template_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/old/template/**template_test.go
> +++ b/src/pkg/old/template/**template_test.go
> @@ -795,7 +795,7 @@
>                                t.Errorf("For %q, expected error, got
> none.", c.in)
>                                continue
>                        }
> -                       buf := bytes.NewBuffer(nil)
> +                       buf := new(bytes.Buffer)
>                        err = tmpl.Execute(buf, data)
>                        if err != nil {
>                                t.Error("unexpected Execute error: ", err)
>
>
>

[r2, 2012-02-06 01:33:34]

Yes, although then you need the ugly & sometimes. I'll play some more.

-rob

[r, 2012-02-06 01:50:19]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, r@google.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2012-02-06 02:06:20]

LGTM

On Sun, Feb 5, 2012 at 5:50 PM, <r@golang.org> wrote:

> Hello golang-dev@googlegroups.com, bradfitz@golang.org, r@google.com
> (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
http://codereview.appspot.com/**5637043/<http://codereview.appspot.com/5637043/>
>

[r, 2012-02-06 03:09:06]

*** Submitted as http://code.google.com/p/go/source/detail?r=5900a93988af ***

all: avoid bytes.NewBuffer(nil)
The practice encourages people to think this is the way to
create a bytes.Buffer when new(bytes.Buffer) or
just var buf bytes.Buffer work fine.
(html/token.go was missing the point altogether.)

R=golang-dev, bradfitz, r
CC=golang-dev
http://codereview.appspot.com/5637043