code review 4426066: ioutil: add NopWriter, update tree. (issue4426066)

[brad...@golang.org]

Reviewers: r,

Message:
Hello r (cc: golan...@googlegroups.com),

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

Description:
ioutil: add NopWriter, update tree.

Notably, this removes an unnecessary allocation in
http/transfer.go.

Please review this at http://codereview.appspot.com/4426066/

Affected files:
M src/pkg/compress/lzw/reader_test.go
M src/pkg/compress/lzw/writer_test.go
M src/pkg/html/parse_test.go
M src/pkg/http/transfer.go
M src/pkg/io/ioutil/ioutil.go
M src/pkg/mime/multipart/multipart.go

Index: src/pkg/compress/lzw/reader_test.go
===================================================================
--- a/src/pkg/compress/lzw/reader_test.go
+++ b/src/pkg/compress/lzw/reader_test.go
@@ -112,12 +112,6 @@
}
}

-type devNull struct{}
-
-func (devNull) Write(p []byte) (int, os.Error) {
- return len(p), nil
-}
-
func benchmarkDecoder(b *testing.B, n int) {
b.StopTimer()
b.SetBytes(int64(n))
@@ -134,7 +128,7 @@
runtime.GC()
b.StartTimer()
for i := 0; i < b.N; i++ {
- io.Copy(devNull{}, NewReader(bytes.NewBuffer(buf1), LSB, 8))
+ io.Copy(ioutil.NopWriter, NewReader(bytes.NewBuffer(buf1), LSB, 8))
}
}

Index: src/pkg/compress/lzw/writer_test.go
===================================================================
--- a/src/pkg/compress/lzw/writer_test.go
+++ b/src/pkg/compress/lzw/writer_test.go
@@ -113,7 +113,7 @@
runtime.GC()
b.StartTimer()
for i := 0; i < b.N; i++ {
- w := NewWriter(devNull{}, LSB, 8)
+ w := NewWriter(ioutil.NopWriter, LSB, 8)
w.Write(buf1)
w.Close()
}
Index: src/pkg/html/parse_test.go
===================================================================
--- a/src/pkg/html/parse_test.go
+++ b/src/pkg/html/parse_test.go
@@ -15,12 +15,6 @@
"testing"
)

-type devNull struct{}
-
-func (devNull) Write(p []byte) (int, os.Error) {
- return len(p), nil
-}
-
func pipeErr(err os.Error) io.Reader {
pr, pw := io.Pipe()
pw.CloseWithError(err)
@@ -141,7 +135,7 @@
t.Fatal(err)
}
// Skip the #error section.
- if _, err := io.Copy(devNull{}, <-rc); err != nil {
+ if _, err := io.Copy(ioutil.NopWriter, <-rc); err != nil {
t.Fatal(err)
}
// Compare the parsed tree to the #document section.
Index: src/pkg/http/transfer.go
===================================================================
--- a/src/pkg/http/transfer.go
+++ b/src/pkg/http/transfer.go
@@ -7,6 +7,7 @@
import (
"bufio"
"io"
+ "io/ioutil"
"os"
"strconv"
"strings"
@@ -447,17 +448,10 @@
return nil
}

- trashBuf := make([]byte, 1024) // local for thread safety
- for {
- _, err := b.Read(trashBuf)
- if err == nil {
- continue
- }
- if err == os.EOF {
- break
- }
+ if _, err := io.Copy(ioutil.NopWriter, b); err != nil {
return err
}
+
if b.hdr == nil { // not reading trailer
return nil
}
Index: src/pkg/io/ioutil/ioutil.go
===================================================================
--- a/src/pkg/io/ioutil/ioutil.go
+++ b/src/pkg/io/ioutil/ioutil.go
@@ -101,3 +101,13 @@
func NopCloser(r io.Reader) io.ReadCloser {
return nopCloser{r}
}
+
+type devNull int
+
+func (devNull) Write(p []byte) (int, os.Error) {
+ return len(p), nil
+}
+
+// NopWriter is an io.Writer on which all Write calls succeed
+// without doing anything.
+var NopWriter io.Writer = devNull(0)
Index: src/pkg/mime/multipart/multipart.go
===================================================================
--- a/src/pkg/mime/multipart/multipart.go
+++ b/src/pkg/mime/multipart/multipart.go
@@ -16,6 +16,7 @@
"bufio"
"bytes"
"io"
+ "io/ioutil"
"mime"
"net/textproto"
"os"
@@ -76,14 +77,6 @@

// Implementation ....

-type devNullWriter bool
-
-func (*devNullWriter) Write(p []byte) (n int, err os.Error) {
- return len(p), nil
-}
-
-var devNull = devNullWriter(false)
-
func newPart(mr *multiReader) (bp *Part, err os.Error) {
bp = new(Part)
bp.Header = make(map[string][]string)
@@ -158,7 +151,7 @@
}

func (bp *Part) Close() os.Error {
- io.Copy(&devNull, bp)
+ io.Copy(ioutil.NopWriter, bp)
return nil
}

[r...@golang.org]

http://codereview.appspot.com/4426066/diff/5001/src/pkg/io/ioutil/ioutil.go
File src/pkg/io/ioutil/ioutil.go (right):

http://codereview.appspot.com/4426066/diff/5001/src/pkg/io/ioutil/ioutil.go#newcode111
src/pkg/io/ioutil/ioutil.go:111: // NopWriter is an io.Writer on which
all Write calls succeed
Not thrilled with the name. What about just Discard?

http://codereview.appspot.com/4426066/

[brad...@golang.org]

http://codereview.appspot.com/4426066/diff/5001/src/pkg/io/ioutil/ioutil.go
File src/pkg/io/ioutil/ioutil.go (right):

http://codereview.appspot.com/4426066/diff/5001/src/pkg/io/ioutil/ioutil.go#newcode111
src/pkg/io/ioutil/ioutil.go:111: // NopWriter is an io.Writer on which
all Write calls succeed

On 2011/04/27 22:02:00, r wrote:
> Not thrilled with the name. What about just Discard?

I don't care about the name. I picked NopWriter because it matched
NopCloser also in this package.

Your call.

http://codereview.appspot.com/4426066/

[Russ Cox]

fmt.Fprintf(ioutil.Discard, "hello world")

looks good to me

[Rob 'Commander' Pike]

On Apr 27, 2011, at 3:29 PM, Russ Cox wrote:

> fmt.Fprintf(ioutil.Discard, "hello world")
>
> looks good to me

i think the niceness of reading trumps the parallel with NopCloser (another name i'm not thrilled with, although in that case i don't have a countersuggestion).

rename to Discard please.

-rob