io: guarantee err == nil for full reads in ReadFull and ReadAtLeast

This is a backwards compatible API change that fixes broken code.

In Go 1.0, ReadFull(r, buf) could return either len(buf), nil or len(buf), non-nil.
Most code expects only the former, so do that and document the guarantee.

Code that was correct before is still correct.
Code that was incorrect before, by assuming the guarantee, is now correct too.

The same applies to ReadAtLeast.

Fixes #4544.

R=golang-dev, bradfitz, minux.ma
CC=golang-dev
https://golang.org/cl/7235074

diff --git a/src/pkg/io/io.go b/src/pkg/io/io.go
index 859adaf1b710a132b2ab07860daad9a98dfcef0b..5b24f062efc4842901f164e8c15b9c4a0cbb5fa2 100644 (file)
--- a/src/pkg/io/io.go
+++ b/src/pkg/io/io.go
@@ -262,6 +262,7 @@ func WriteString(w Writer, s string) (n int, err error) {
 // If an EOF happens after reading fewer than min bytes,
 // ReadAtLeast returns ErrUnexpectedEOF.
 // If min is greater than the length of buf, ReadAtLeast returns ErrShortBuffer.
+// On return, n >= min if and only if err == nil.
 func ReadAtLeast(r Reader, buf []byte, min int) (n int, err error) {
        if len(buf) < min {
                return 0, ErrShortBuffer
@@ -271,12 +272,10 @@ func ReadAtLeast(r Reader, buf []byte, min int) (n int, err error) {
                nn, err = r.Read(buf[n:])
                n += nn
        }
-       if err == EOF {
-               if n >= min {
-                       err = nil
-               } else if n > 0 {
-                       err = ErrUnexpectedEOF
-               }
+       if n >= min {
+               err = nil
+       } else if n > 0 && err == EOF {
+               err = ErrUnexpectedEOF
        }
        return
 }
@@ -286,6 +285,7 @@ func ReadAtLeast(r Reader, buf []byte, min int) (n int, err error) {
 // The error is EOF only if no bytes were read.
 // If an EOF happens after reading some but not all the bytes,
 // ReadFull returns ErrUnexpectedEOF.
+// On return, n == len(buf) if and only if err == nil.
 func ReadFull(r Reader, buf []byte) (n int, err error) {
        return ReadAtLeast(r, buf, len(buf))
 }

diff --git a/src/pkg/io/io_test.go b/src/pkg/io/io_test.go
index f3ec050fad9ed1de7c5b9c60038f4c0186c74d3c..307066825ba3d009109b8dfb7d4552b6a3eb03e1 100644 (file)
--- a/src/pkg/io/io_test.go
+++ b/src/pkg/io/io_test.go
@@ -6,6 +6,7 @@ package io_test
 
 import (
        "bytes"
+       "fmt"
        . "io"
        "strings"
        "testing"
@@ -120,22 +121,30 @@ func TestReadAtLeast(t *testing.T) {
        testReadAtLeast(t, &rb)
 }
 
-// A version of bytes.Buffer that returns n > 0, EOF on Read
+// A version of bytes.Buffer that returns n > 0, err on Read
 // when the input is exhausted.
-type dataAndEOFBuffer struct {
+type dataAndErrorBuffer struct {
+       err error
        bytes.Buffer
 }
 
-func (r *dataAndEOFBuffer) Read(p []byte) (n int, err error) {
+func (r *dataAndErrorBuffer) Read(p []byte) (n int, err error) {
        n, err = r.Buffer.Read(p)
        if n > 0 && r.Buffer.Len() == 0 && err == nil {
-               err = EOF
+               err = r.err
        }
        return
 }
 
 func TestReadAtLeastWithDataAndEOF(t *testing.T) {
-       var rb dataAndEOFBuffer
+       var rb dataAndErrorBuffer
+       rb.err = EOF
+       testReadAtLeast(t, &rb)
+}
+
+func TestReadAtLeastWithDataAndError(t *testing.T) {
+       var rb dataAndErrorBuffer
+       rb.err = fmt.Errorf("fake error")
        testReadAtLeast(t, &rb)
 }
 
@@ -169,8 +178,12 @@ func testReadAtLeast(t *testing.T, rb ReadWriter) {
        }
        rb.Write([]byte("4"))
        n, err = ReadAtLeast(rb, buf, 2)
-       if err != ErrUnexpectedEOF {
-               t.Errorf("expected ErrUnexpectedEOF, got %v", err)
+       want := ErrUnexpectedEOF
+       if rb, ok := rb.(*dataAndErrorBuffer); ok && rb.err != EOF {
+               want = rb.err
+       }
+       if err != want {
+               t.Errorf("expected %v, got %v", want, err)
        }
        if n != 1 {
                t.Errorf("expected to have read 1 bytes, got %v", n)