Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io.ReadAtLeast - expect EOF return when buffer is empty in Read function #391

Closed
ChinW opened this issue Aug 4, 2022 · 1 comment
Closed
Assignees
Labels
help wanted Extra attention is needed needs fix pending development Requested PR owner to improve code and waiting for the result question Further information is requested

Comments

@ChinW
Copy link

ChinW commented Aug 4, 2022

Please fill out the following system information before opening an issue:

  • OS (e.g. Ubuntu 18.04): macOS 12.5
  • Go version (e.g. Go 1.13): Go 1.18
  • gnet version (e.g. v1.0.0): v2.1.1

What is your question about gnet?
Please describe your question meticulously.

hi Andy, thanks for your work. I met below issue in io.ReadAtLeast function, wonder if my thought is correct?

  1. io.ReadAtLeast
 # from go/src/io/io.go
func ReadAtLeast(r Reader, buf []byte, min int) (n int, err error) {
	if len(buf) < min {
		return 0, ErrShortBuffer
	}
	for n < min && err == nil {
		var nn int
		nn, err = r.Read(buf[n:]) // !! here, if buf is empty, we expect err should be EOF
		n += nn
	}
	if n >= min {
		err = nil
	} else if n > 0 && err == EOF {
		err = ErrUnexpectedEOF
	}
	return
}

  1. gnet/connection.go
func (c *conn) Read(p []byte) (n int, err error) {
	if c.inboundBuffer.IsEmpty() {
		n = copy(p, c.buffer)
		c.buffer = c.buffer[n:]
		return n, nil // should this be io.EOF? 
	}
	n, _ = c.inboundBuffer.Read(p)
	if n == len(p) {
		return
	}
	m := copy(p[n:], c.buffer)
	n += m
	c.buffer = c.buffer[m:]
	return
}

Comparing to bytes/buffer.go, it returns EOF in a case where input bytes also empty

func (b *Buffer) Read(p []byte) (n int, err error) {
	b.lastRead = opInvalid
	if b.empty() {
		// Buffer is empty, reset to recover space.
		b.Reset()
		if len(p) == 0 {
			return 0, nil
		} 
		return 0, io.EOF // here
	}
	n = copy(p, b.buf[b.off:])
	b.off += n
	if n > 0 {
		b.lastRead = opRead
	}
	return n, nil
}
@ChinW ChinW added help wanted Extra attention is needed question Further information is requested labels Aug 4, 2022
@panjf2000 panjf2000 added pending development Requested PR owner to improve code and waiting for the result needs fix labels Aug 9, 2022
@panjf2000
Copy link
Owner

Sorry for the delay, I've fixed this issue and c.Read([]byte) now is behaving like bytes.Buffer.Read([]byte), please get the latest code and try it on.

0-haha pushed a commit to 0-haha/gnet that referenced this issue Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed needs fix pending development Requested PR owner to improve code and waiting for the result question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants