From 662ff5421287e3738587a9eb01fa50e080e48582 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 31 Jan 2013 13:46:12 -0800 Subject: [PATCH] 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 --- src/pkg/io/io.go | 12 ++++++------ src/pkg/io/io_test.go | 27 ++++++++++++++++++++------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/pkg/io/io.go b/src/pkg/io/io.go index 859adaf1b71..5b24f062efc 100644 --- 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 f3ec050fad9..307066825ba 100644 --- 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)