From 0d40dfa745b176a83d91cf0981bdbd3a92e2e547 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 1 Apr 2016 18:11:26 -0700 Subject: [PATCH] compress/gzip: cleanup gzip package Changes made: * Reader.flg is not used anywhere else other than readHeader and does not need to be stored. * Store Reader.digest and Writer.digest as uint32s rather than as a hash.Hash32 and use the crc32.Update function instead. This simplifies initialization logic since the zero value of uint32 is the initial CRC-32 value. There are no performance detriments to doing this since the hash.Hash32 returned by crc32 simply calls crc32.Update as well. * s/[0:/[:/ Consistently use shorter notation for slicing. * s/RFC1952/RFC 1952/ Consistently use RFC notation. Change-Id: I55416a19f4836cbed943adaa3f672538ea5d166d Reviewed-on: https://go-review.googlesource.com/21429 Reviewed-by: Brad Fitzpatrick Run-TryBot: Joe Tsai TryBot-Result: Gobot Gobot --- src/compress/gzip/gunzip.go | 54 +++++++++++++------------------- src/compress/gzip/gunzip_test.go | 2 +- src/compress/gzip/gzip.go | 26 ++++++--------- 3 files changed, 32 insertions(+), 50 deletions(-) diff --git a/src/compress/gzip/gunzip.go b/src/compress/gzip/gunzip.go index 8ab2b5e5ecf..1bd8769867c 100644 --- a/src/compress/gzip/gunzip.go +++ b/src/compress/gzip/gunzip.go @@ -10,7 +10,6 @@ import ( "bufio" "compress/flate" "errors" - "hash" "hash/crc32" "io" "time" @@ -72,9 +71,8 @@ type Reader struct { Header // valid after NewReader or Reader.Reset r flate.Reader decompressor io.ReadCloser - digest hash.Hash32 - size uint32 - flg byte + digest uint32 // CRC-32, IEEE polynomial (section 8) + size uint32 // Uncompressed size (section 2.3.1) buf [512]byte err error multistream bool @@ -91,7 +89,6 @@ func NewReader(r io.Reader) (*Reader, error) { z := new(Reader) z.r = makeReader(r) z.multistream = true - z.digest = crc32.NewIEEE() if err := z.readHeader(true); err != nil { return nil, err } @@ -103,11 +100,7 @@ func NewReader(r io.Reader) (*Reader, error) { // This permits reusing a Reader rather than allocating a new one. func (z *Reader) Reset(r io.Reader) error { z.r = makeReader(r) - if z.digest == nil { - z.digest = crc32.NewIEEE() - } else { - z.digest.Reset() - } + z.digest = 0 z.size = 0 z.err = nil z.multistream = true @@ -157,18 +150,18 @@ func (z *Reader) readString() (string, error) { // GZIP (RFC 1952) specifies that strings are NUL-terminated ISO 8859-1 (Latin-1). if needconv { s := make([]rune, 0, i) - for _, v := range z.buf[0:i] { + for _, v := range z.buf[:i] { s = append(s, rune(v)) } return string(s), nil } - return string(z.buf[0:i]), nil + return string(z.buf[:i]), nil } } } func (z *Reader) read2() (uint32, error) { - _, err := io.ReadFull(z.r, z.buf[0:2]) + _, err := io.ReadFull(z.r, z.buf[:2]) if err != nil { if err == io.EOF { err = io.ErrUnexpectedEOF @@ -179,9 +172,9 @@ func (z *Reader) read2() (uint32, error) { } func (z *Reader) readHeader(save bool) error { - _, err := io.ReadFull(z.r, z.buf[0:10]) + _, err := io.ReadFull(z.r, z.buf[:10]) if err != nil { - // RFC1952 section 2.2 says the following: + // RFC 1952, section 2.2, says the following: // A gzip file consists of a series of "members" (compressed data sets). // // Other than this, the specification does not clarify whether a @@ -193,16 +186,15 @@ func (z *Reader) readHeader(save bool) error { if z.buf[0] != gzipID1 || z.buf[1] != gzipID2 || z.buf[2] != gzipDeflate { return ErrHeader } - z.flg = z.buf[3] + flg := z.buf[3] if save { z.ModTime = time.Unix(int64(get4(z.buf[4:8])), 0) // z.buf[8] is xfl, ignored z.OS = z.buf[9] } - z.digest.Reset() - z.digest.Write(z.buf[0:10]) + z.digest = crc32.Update(0, crc32.IEEETable, z.buf[:10]) - if z.flg&flagExtra != 0 { + if flg&flagExtra != 0 { n, err := z.read2() if err != nil { return err @@ -220,7 +212,7 @@ func (z *Reader) readHeader(save bool) error { } var s string - if z.flg&flagName != 0 { + if flg&flagName != 0 { if s, err = z.readString(); err != nil { return err } @@ -229,7 +221,7 @@ func (z *Reader) readHeader(save bool) error { } } - if z.flg&flagComment != 0 { + if flg&flagComment != 0 { if s, err = z.readString(); err != nil { return err } @@ -238,18 +230,18 @@ func (z *Reader) readHeader(save bool) error { } } - if z.flg&flagHdrCrc != 0 { + if flg&flagHdrCrc != 0 { n, err := z.read2() if err != nil { return err } - sum := z.digest.Sum32() & 0xFFFF + sum := z.digest & 0xFFFF if n != sum { return ErrHeader } } - z.digest.Reset() + z.digest = 0 if z.decompressor == nil { z.decompressor = flate.NewReader(z.r) } else { @@ -264,29 +256,27 @@ func (z *Reader) Read(p []byte) (n int, err error) { } n, z.err = z.decompressor.Read(p) - z.digest.Write(p[0:n]) + z.digest = crc32.Update(z.digest, crc32.IEEETable, p[:n]) z.size += uint32(n) if z.err != io.EOF { // In the normal case we return here. return n, z.err } - // Finished file; check checksum + size. - if _, err := io.ReadFull(z.r, z.buf[0:8]); err != nil { + // Finished file; check checksum and size. + if _, err := io.ReadFull(z.r, z.buf[:8]); err != nil { if err == io.EOF { err = io.ErrUnexpectedEOF } z.err = err return n, err } - crc32, isize := get4(z.buf[0:4]), get4(z.buf[4:8]) - sum := z.digest.Sum32() - if sum != crc32 || isize != z.size { + digest, size := get4(z.buf[:4]), get4(z.buf[4:8]) + if digest != z.digest || size != z.size { z.err = ErrChecksum return n, z.err } - z.digest.Reset() - z.size = 0 + z.digest, z.size = 0, 0 // File is ok; check if there is another. if !z.multistream { diff --git a/src/compress/gzip/gunzip_test.go b/src/compress/gzip/gunzip_test.go index 007d9585cee..593644ac1b3 100644 --- a/src/compress/gzip/gunzip_test.go +++ b/src/compress/gzip/gunzip_test.go @@ -411,7 +411,7 @@ Found: } func TestNilStream(t *testing.T) { - // Go liberally interprets RFC1952 section 2.2 to mean that a gzip file + // Go liberally interprets RFC 1952 section 2.2 to mean that a gzip file // consist of zero or more members. Thus, we test that a nil stream is okay. _, err := NewReader(bytes.NewReader(nil)) if err != io.EOF { diff --git a/src/compress/gzip/gzip.go b/src/compress/gzip/gzip.go index 4d945e47fe8..46512985858 100644 --- a/src/compress/gzip/gzip.go +++ b/src/compress/gzip/gzip.go @@ -8,7 +8,6 @@ import ( "compress/flate" "errors" "fmt" - "hash" "hash/crc32" "io" ) @@ -30,8 +29,8 @@ type Writer struct { level int wroteHeader bool compressor *flate.Writer - digest hash.Hash32 - size uint32 + digest uint32 // CRC-32, IEEE polynomial (section 8) + size uint32 // Uncompressed size (section 2.3.1) closed bool buf [10]byte err error @@ -66,12 +65,6 @@ func NewWriterLevel(w io.Writer, level int) (*Writer, error) { } func (z *Writer) init(w io.Writer, level int) { - digest := z.digest - if digest != nil { - digest.Reset() - } else { - digest = crc32.NewIEEE() - } compressor := z.compressor if compressor != nil { compressor.Reset(w) @@ -82,7 +75,6 @@ func (z *Writer) init(w io.Writer, level int) { }, w: w, level: level, - digest: digest, compressor: compressor, } } @@ -113,8 +105,8 @@ func (z *Writer) writeBytes(b []byte) error { if len(b) > 0xffff { return errors.New("gzip.Write: Extra data is too large") } - put2(z.buf[0:2], uint16(len(b))) - _, err := z.w.Write(z.buf[0:2]) + put2(z.buf[:2], uint16(len(b))) + _, err := z.w.Write(z.buf[:2]) if err != nil { return err } @@ -149,7 +141,7 @@ func (z *Writer) writeString(s string) (err error) { } // GZIP strings are NUL-terminated. z.buf[0] = 0 - _, err = z.w.Write(z.buf[0:1]) + _, err = z.w.Write(z.buf[:1]) return err } @@ -185,7 +177,7 @@ func (z *Writer) Write(p []byte) (int, error) { z.buf[8] = 0 } z.buf[9] = z.OS - n, z.err = z.w.Write(z.buf[0:10]) + n, z.err = z.w.Write(z.buf[:10]) if z.err != nil { return n, z.err } @@ -212,7 +204,7 @@ func (z *Writer) Write(p []byte) (int, error) { } } z.size += uint32(len(p)) - z.digest.Write(p) + z.digest = crc32.Update(z.digest, crc32.IEEETable, p) n, z.err = z.compressor.Write(p) return n, z.err } @@ -262,8 +254,8 @@ func (z *Writer) Close() error { if z.err != nil { return z.err } - put4(z.buf[0:4], z.digest.Sum32()) + put4(z.buf[:4], z.digest) put4(z.buf[4:8], z.size) - _, z.err = z.w.Write(z.buf[0:8]) + _, z.err = z.w.Write(z.buf[:8]) return z.err }