bytes: Buffer.ReadFrom should not ignore io.EOF

[blackbeans]

Go version

go version go1.22.0 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xx/Library/Caches/go-build'
GOENV='/Users/xxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=' -mod=readonly'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xxxx/go/pkg/mod'
GONOPROXY='git.xxx.com'
GONOSUMDB='git.xxx.com'
GOOS='darwin'
GOPATH='/Users/xxxxx/go'
GOPRIVATE='git.xxxx.com'
GOPROXY='https://goproxy.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/xxxx/workspace/xxxx/opensource/xxxx/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/6y/r10dlmq565d6y6jyy6w12xp80000gn/T/go-build2825296061=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

	
		in, _ := os.Open("../resources/demo.flac")
		defer in.Close()

		buff := bytes.NewBuffer(make([]byte, 0, MB))
		//read by parttition
		for i := 0; i < 1000; i++ {
			buff.Reset()
			length, err := buff.ReadFrom(io.LimitReader(in, MB))
			if nil != err && !errors.Is(err, io.EOF) {
				t.FailNow()
			} else if errors.Is(err, io.EOF) {
                               // this does not happen
				break
			}
			t.Logf("read:%d\n", length)
		}

What did you see happen?

This code block breaks util loop 1000 times ，but we expect this loop break when the readFrom throws io.EOF .

The bytes library shows the readFrom source code as below :

// ReadFrom reads data from r until EOF and appends it to the buffer, growing
// the buffer as needed. The return value n is the number of bytes read. Any
// error except io.EOF encountered during the read is also returned. If the
// buffer becomes too large, ReadFrom will panic with [ErrTooLarge].
func (b *Buffer) ReadFrom(r io.Reader) (n int64, err error) {
	b.lastRead = opInvalid
	for {
		i := b.grow(MinRead)
		b.buf = b.buf[:i]
		m, e := r.Read(b.buf[i:cap(b.buf)])
		if m < 0 {
			panic(errNegativeRead)
		}

		b.buf = b.buf[:i+m]
		n += int64(m)
		if e == io.EOF {
			return n, nil // e is EOF, so return nil explicitly
		}
		if e != nil {
			return n, e
		}
	}
}

The source code throws nil insteadOf io.EOF .

What did you expect to see?

I Suggest readFrom throws io.EOF,so that keep the same as others io operatoions.

[Jorropo]

This behavior is explicitly documented by io.ReaderFrom:

ReadFrom reads data from r until EOF or error. The return value n is the number of bytes read. Any error except EOF encountered during the read is also returned.

I don't see why it should return io.EOF, what would that mean ? io.ReadFrom read everything there is to read (so everything until io.EOF) unless an error happen.
As far as I can tell that means io.ReadFrom would never return a nil, error it would always return io.EOF or some other error.

[panjf2000]

In Go, returning a non-nil error from a function/method indicates there is something wrong. In this case, EOF shouldn't be interpreted as a wrongness, it only manifests that there is no more data that can be read from a data source. Therefore, it makes sense not to return this error. You may find this behavior rational after checking out read(2) which shares the same philosophy (not treating EOF as an error) with io.ReaderFrom.

[thediveo]

This would break a lot of existing code that has been programed to the documented contract.

[ianlancetaylor]

Thanks, but we aren't going to make this change.