code review 13171046: compress/zlib: add Reset method to Writer.

compress/zlib: add Reset method to Writer.

[remyoudompheng, 2013-08-24 11:03:47]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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

[bradfitz, 2013-08-28 18:28:38]

This also needs to preserve the dictionary, right?



On Sat, Aug 24, 2013 at 4:03 AM, <remyoudompheng@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> compress/zlib: add Reset method to Writer.
>
> Please review this at
https://codereview.appspot.**com/13171046/<https://codereview.appspot.com/131...
>
> Affected files:
>   M src/pkg/compress/zlib/writer.**go
>   M src/pkg/compress/zlib/writer_**test.go
>
>
> Index: src/pkg/compress/zlib/writer.**go
> ==============================**==============================**=======
> --- a/src/pkg/compress/zlib/**writer.go
> +++ b/src/pkg/compress/zlib/**writer.go
> @@ -70,6 +70,22 @@
>         }, nil
>  }
>
> +// Reset clears a Writer state so that it is equivalent to the result of
> +// NewWriterLevel(w, level) where level is the Writer's compression level.
> +func (z *Writer) Reset(w io.Writer) {
> +       z.w = w
> +       z.dict = nil
> +       if z.compressor != nil {
> +               z.compressor.Reset(w)
> +       }
> +       if z.digest != nil {
> +               z.digest.Reset()
> +       }
> +       z.err = nil
> +       z.scratch = [4]byte{}
> +       z.wroteHeader = false
> +}
> +
>  // writeHeader writes the ZLIB header.
>  func (z *Writer) writeHeader() (err error) {
>         z.wroteHeader = true
> @@ -111,11 +127,15 @@
>                         return err
>                 }
>         }
> -       z.compressor, err = flate.NewWriterDict(z.w, z.level, z.dict)
> -       if err != nil {
> -               return err
> +       if z.compressor == nil {
> +               // Initialize deflater unless the Writer is being reused
> +               // after a Reset call.
> +               z.compressor, err = flate.NewWriterDict(z.w, z.level,
> z.dict)
> +               if err != nil {
> +                       return err
> +               }
> +               z.digest = adler32.New()
>         }
> -       z.digest = adler32.New()
>         return nil
>  }
>
> Index: src/pkg/compress/zlib/writer_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/compress/zlib/**writer_test.go
> +++ b/src/pkg/compress/zlib/**writer_test.go
> @@ -89,6 +89,51 @@
>         }
>  }
>
> +func testFileLevelReset(t *testing.T, fn string, level int) {
> +       var b0 []byte
> +       var err error
> +       if fn != "" {
> +               b0, err = ioutil.ReadFile(fn)
> +               if err != nil {
> +                       t.Errorf("%s (level=%d): %v", fn, level, err)
> +                       return
> +               }
> +       }
> +
> +       // Compress once.
> +       buf := new(bytes.Buffer)
> +       zlibw, err := NewWriterLevel(buf, level)
> +       if err == nil {
> +               _, err = zlibw.Write(b0)
> +       }
> +       if err == nil {
> +               err = zlibw.Close()
> +       }
> +       if err != nil {
> +               t.Errorf("%s (level=%d): %v", fn, level, err)
> +               return
> +       }
> +       out := buf.String()
> +
> +       // Reset and comprses again.
> +       buf.Reset()
> +       zlibw.Reset(buf)
> +       _, err = zlibw.Write(b0)
> +       if err == nil {
> +               err = zlibw.Close()
> +       }
> +       if err != nil {
> +               t.Errorf("%s (level=%d): %v", fn, level, err)
> +               return
> +       }
> +       out2 := buf.String()
> +
> +       if out2 != out {
> +               t.Errorf("%s (level=%d): different output after reset (got
> %d bytes, expected %d",
> +                       fn, level, len(out2), len(out))
> +       }
> +}
> +
>  func TestWriter(t *testing.T) {
>         for i, s := range data {
>                 b := []byte(s)
> @@ -122,6 +167,18 @@
>         }
>  }
>
> +func TestWriterReset(t *testing.T) {
> +       for _, fn := range filenames {
> +               testFileLevelReset(t, fn, DefaultCompression)
> +               testFileLevelReset(t, fn, NoCompression)
> +               for level := BestSpeed; level <= BestCompression; level++ {
> +                       if !testing.Short() {
> +                               testFileLevelReset(t, fn, level)
> +                       }
> +               }
> +       }
> +}
> +
>  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.")
>         var buf bytes.Buffer
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[remyoudompheng, 2013-08-29 10:31:01]

On 2013/08/28 18:28:38, bradfitz wrote:
> This also needs to preserve the dictionary, right?

Yes, there is essentially nothing to do since this is mainly handled in
compress/flate.

[remyoudompheng, 2013-08-29 10:31:13]

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

Please take another look.

[bradfitz, 2013-08-29 18:52:48]

LGTM

https://codereview.appspot.com/13171046/diff/12001/src/pkg/compress/zlib/writ...
File src/pkg/compress/zlib/writer.go (right):

https://codereview.appspot.com/13171046/diff/12001/src/pkg/compress/zlib/writ...
src/pkg/compress/zlib/writer.go:73: // Reset clears a Writer state so that it is
equivalent to the result of
Maybe:

// Reset clears the state of the Writer z such that it is equivalent to its
// initial state from NewWriterLevel or NewWriterLevelDict,
// but instead writing to w.

[remyoudompheng, 2013-09-05 19:50:56]

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

compress/zlib: add Reset method to Writer.

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/13171046

https://codereview.appspot.com/13171046/diff/12001/src/pkg/compress/zlib/writ...
File src/pkg/compress/zlib/writer.go (right):

https://codereview.appspot.com/13171046/diff/12001/src/pkg/compress/zlib/writ...
src/pkg/compress/zlib/writer.go:73: // Reset clears a Writer state so that it is
equivalent to the result of
On 2013/08/29 18:52:48, bradfitz wrote:
> Maybe:
> 
> // Reset clears the state of the Writer z such that it is equivalent to its
> // initial state from NewWriterLevel or NewWriterLevelDict,
> // but instead writing to w.
> 

Done.