code review 4670051: crypto/openpgp: use new(bytes.Buffer) where possible

crypto/openpgp: use new(bytes.Buffer) where possible

[agl1, 2011-07-08 13:57:06]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/

[bradfitz, 2011-07-08 14:54:57]

LGTM On Fri, Jul 8, 2011 at 6:57 AM, <agl@golang.org> wrote: > Reviewers: bradfitz, > > Message: > Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), > > I'd like you to review this change to > https://go.googlecode.com/hg/ > > > Description: > crypto/openpgp: use new(bytes.Buffer) where possible > > Please review this at http://codereview.appspot.com/**4670051/<http://codereview.appspot.com/4670051/> > > Affected files: > M src/pkg/crypto/openpgp/**canonical_text_test.go > M src/pkg/crypto/openpgp/packet/**packet_test.go > M src/pkg/crypto/openpgp/packet/**private_key.go > M src/pkg/crypto/openpgp/packet/**public_key_test.go > M src/pkg/crypto/openpgp/packet/**symmetrically_encrypted_test.**go > M src/pkg/crypto/openpgp/write_**test.go > > > Index: src/pkg/crypto/openpgp/**canonical_text_test.go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**canonical_text_test.go > +++ b/src/pkg/crypto/openpgp/**canonical_text_test.go > @@ -32,7 +32,7 @@ > > > func testCanonicalText(t *testing.T, input, expected string) { > - r := recordingHash{bytes.NewBuffer(**nil)} > + r := recordingHash{new(bytes.**Buffer)} > c := NewCanonicalTextHash(r) > c.Write([]byte(input)) > result := c.Sum() > Index: src/pkg/crypto/openpgp/packet/**packet_test.go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**packet/packet_test.go > +++ b/src/pkg/crypto/openpgp/**packet/packet_test.go > @@ -30,7 +30,7 @@ > t.Errorf("partial read failed n:%d err:%s", n, err) > } > > - b = bytes.NewBuffer(nil) > + b = new(bytes.Buffer) > n, err = readFull(b, out[:3]) > if n != 0 || err != io.ErrUnexpectedEOF { > t.Errorf("empty read failed n:%d err:%s", n, err) > @@ -196,7 +196,7 @@ > lengths := []int{0, 1, 2, 64, 192, 193, 8000, 8384, 8385, 10000} > > for _, length := range lengths { > - buf := bytes.NewBuffer(nil) > + buf := new(bytes.Buffer) > serializeHeader(buf, tag, length) > tag2, length2, _, err := readHeader(buf) > if err != nil { > @@ -212,7 +212,7 @@ > } > > func TestPartialLengths(t *testing.T) { > - buf := bytes.NewBuffer(nil) > + buf := new(bytes.Buffer) > w := new(partialLengthWriter) > w.w = noOpCloser{buf} > > @@ -236,7 +236,7 @@ > w.Close() > > want := (maxChunkSize * (maxChunkSize + 1)) / 2 > - copyBuf := bytes.NewBuffer(nil) > + copyBuf := new(bytes.Buffer) > r := &partialLengthReader{buf, 0, true} > m, err := io.Copy(copyBuf, r) > if m != int64(want) { > Index: src/pkg/crypto/openpgp/packet/**private_key.go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**packet/private_key.go > +++ b/src/pkg/crypto/openpgp/**packet/private_key.go > @@ -113,14 +113,14 @@ > > func (pk *PrivateKey) Serialize(w io.Writer) (err os.Error) { > // TODO(agl): support encrypted private keys > - buf := bytes.NewBuffer(nil) > + buf := new(bytes.Buffer) > err = pk.PublicKey.**serializeWithoutHeaders(buf) > if err != nil { > return > } > buf.WriteByte(0 /* no encryption */ ) > > - privateKeyBuf := bytes.NewBuffer(nil) > + privateKeyBuf := new(bytes.Buffer) > > switch priv := pk.PrivateKey.(type) { > case *rsa.PrivateKey: > Index: src/pkg/crypto/openpgp/packet/**public_key_test.go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**packet/public_key_test.go > +++ b/src/pkg/crypto/openpgp/**packet/public_key_test.go > @@ -69,7 +69,7 @@ > t.Errorf("#%d: failed to parse, got: %#v", i, > packet) > continue > } > - serializeBuf := bytes.NewBuffer(nil) > + serializeBuf := new(bytes.Buffer) > err = pk.Serialize(serializeBuf) > if err != nil { > t.Errorf("#%d: failed to serialize: %s", i, err) > Index: src/pkg/crypto/openpgp/packet/**symmetrically_encrypted_test.**go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**packet/symmetrically_**encrypted_test.go > +++ b/src/pkg/crypto/openpgp/**packet/symmetrically_**encrypted_test.go > @@ -79,7 +79,7 @@ > const mdcPlaintextHex = "**a302789c3b2d93c4e0eb9aba222835** > 39b3203335af44a134afb800c849cb**4c4de10200aff40b45d31432c80cb3** > 84299a0655966d6939dfdeed1dddf9**80" > > func TestSerialize(t *testing.T) { > - buf := bytes.NewBuffer(nil) > + buf := new(bytes.Buffer) > c := CipherAES128 > key := make([]byte, c.KeySize()) > > @@ -112,7 +112,7 @@ > return > } > > - contentsCopy := bytes.NewBuffer(nil) > + contentsCopy := new(bytes.Buffer) > _, err = io.Copy(contentsCopy, r) > if err != nil { > t.Errorf("error from io.Copy: %s", err) > Index: src/pkg/crypto/openpgp/write_**test.go > ==============================**==============================**======= > --- a/src/pkg/crypto/openpgp/**write_test.go > +++ b/src/pkg/crypto/openpgp/**write_test.go > @@ -16,7 +16,7 @@ > > func TestSignDetached(t *testing.T) { > kring, _ := ReadKeyRing(readerFromHex(**testKeys1And2PrivateHex)) > - out := bytes.NewBuffer(nil) > + out := new(bytes.Buffer) > message := bytes.NewBufferString(**signedInput) > err := DetachSign(out, kring[0], message) > if err != nil { > @@ -28,7 +28,7 @@ > > func TestSignTextDetached(t *testing.T) { > kring, _ := ReadKeyRing(readerFromHex(**testKeys1And2PrivateHex)) > - out := bytes.NewBuffer(nil) > + out := new(bytes.Buffer) > message := bytes.NewBufferString(**signedInput) > err := DetachSignText(out, kring[0], message) > if err != nil { > @@ -40,7 +40,7 @@ > > func TestSignDetachedDSA(t *testing.T) { > kring, _ := ReadKeyRing(readerFromHex(**dsaTestKeyPrivateHex)) > - out := bytes.NewBuffer(nil) > + out := new(bytes.Buffer) > message := bytes.NewBufferString(**signedInput) > err := DetachSign(out, kring[0], message) > if err != nil { > @@ -61,7 +61,7 @@ > return > } > > - w := bytes.NewBuffer(nil) > + w := new(bytes.Buffer) > if err := e.SerializePrivate(w); err != nil { > t.Errorf("failed to serialize entity: %s", err) > return > @@ -78,7 +78,7 @@ > t.Errorf("wrong number of entities found, got %d, want 1", > len(el)) > } > > - w = bytes.NewBuffer(nil) > + w = new(bytes.Buffer) > if err := e.SerializePrivate(w); err != nil { > t.Errorf("failed to serialize entity second time: %s", err) > return > @@ -112,7 +112,7 @@ > if err != nil { > t.Errorf("error rereading message: %s", err) > } > - messageBuf := bytes.NewBuffer(nil) > + messageBuf := new(bytes.Buffer) > _, err = io.Copy(messageBuf, md.UnverifiedBody) > if err != nil { > t.Errorf("error rereading message: %s", err) > > > 

[captn, 2011-07-09 09:23:14]

LGTM gofix? gofmt -s? git grep "NewBuffer(nil)" pkg/asn1/marshal.go: return &forkableWriter{bytes.NewBuffer(nil), nil, nil} pkg/bufio/bufio_test.go: l, _ := NewReaderSize(bytes.NewBuffer(nil), 10) pkg/compress/flate/deflate_test.go: buffer := bytes.NewBuffer(nil) pkg/compress/lzw/reader_test.go: b := bytes.NewBuffer(nil) pkg/compress/lzw/reader_test.go: compressed := bytes.NewBuffer(nil) pkg/compress/zlib/writer_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/armor/armor_test.go: buf = bytes.NewBuffer(nil) pkg/crypto/openpgp/canonical_text_test.go: r := recordingHash{bytes.NewBuffer(nil)} pkg/crypto/openpgp/packet/packet_test.go: b = bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/packet_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/packet_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/packet_test.go: copyBuf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/private_key.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/private_key.go: privateKeyBuf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/public_key_test.go: serializeBuf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/symmetric_key_encrypted_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/symmetrically_encrypted_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/packet/symmetrically_encrypted_test.go: contentsCopy := bytes.NewBuffer(nil) pkg/crypto/openpgp/s2k/s2k_test.go: buf := bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: out := bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: out := bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: out := bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: w := bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: w = bytes.NewBuffer(nil) pkg/crypto/openpgp/write_test.go: messageBuf := bytes.NewBuffer(nil) pkg/encoding/hex/hex.go: buf := bytes.NewBuffer(nil) pkg/encoding/hex/hex_test.go: out := bytes.NewBuffer(nil) pkg/encoding/pem/pem.go: buf := bytes.NewBuffer(nil) pkg/encoding/pem/pem_test.go: buf := bytes.NewBuffer(nil) pkg/encoding/pem/pem_test.go: buf := bytes.NewBuffer(nil) pkg/html/escape.go: buf := bytes.NewBuffer(nil) pkg/html/parse_test.go: b := bytes.NewBuffer(nil) pkg/html/token.go: buf := bytes.NewBuffer(nil) pkg/html/token_test.go: result := bytes.NewBuffer(nil) pkg/image/jpeg/writer_test.go: buf := bytes.NewBuffer(nil) pkg/image/png/writer_test.go: b := bytes.NewBuffer(nil) pkg/template/template_test.go: buf := bytes.NewBuffer(nil) pkg/xml/marshal_test.go: buf := bytes.NewBuffer(nil) pkg/xml/marshal_test.go: buf := bytes.NewBuffer(nil) pkg/xml/marshal_test.go: buf := bytes.NewBuffer(nil)

[rsc, 2011-07-11 06:35:55]

Almost all the buf := bytes.NewBuffer(nil) are better as var buf bytes.Buffer and then pass &buf in the one or two places it matters. Avoids thinking about new entirely. 

[captn, 2011-07-11 14:44:05]

Thinking is good. I'd prefer new where it's shorter.

[rsc, 2012-06-03 04:52:50]

openpgp is gone from this repository, thank god

[remyoudompheng, 2013-07-21 10:13:19]

R=close