-
Notifications
You must be signed in to change notification settings - Fork 18k
io: document whether io.Reader returning (0, nil) is illegal #5310
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
Labels
Comments
See also https://golang.org/issue/5309 |
I agree. I've seen this asked a few times now. Labels changed: added documentation, removed priority-triage. Owner changed to @robpike. Status changed to Accepted. |
From what I recall from previous discussions, (0, nil) has been considered illegal by people, and it was also asserted that the Go standard library would never return (0, nil) from a Read() implementation. The main problem is (0, nil) causes busy waits, and the only sane alternative behavior is to behave as though it returned (0, io.EOF). That's precisely what bufio.Scanner assumes (0, nil) to be. So there's no point in allowing (0, nil) to be legal. |
The standard library is already inconsistent. io.Copy treats (0, nil) as a no-op. bufio.Scanner treats (0, nil) as EOF. We don't even understand the rules ourselves. The disadvantage to allowing (0, nil) but treating it as a no-op is that things can spin (like io.Copy). An advantage to allowing it is that it allows occasional (0, nil) mid-stream, maybe easing the implementation of Readers (like our crypto/tls.*Reader, which can sometimes return (0, nil)). The disadvantage to treating it like EOF is that we then have THREE WAYS to do EOF, and all callers have to deal with all three ways. The current two is onerous enough. I recommend documenting it as a semantic no-op, but discouraged (especially repeatedly) because it's useless and can only harm performance. Labels changed: added go1.1. |
I think its a mistake to have to consider the the return values together to make a decision. 0 implies no bytes were read and nil implies no error (inlcuding EOF). The only sane interpretation IMHO is that (0, nil) implies (0 bytes available, reader is not in error state). While it is true many people do not handle this case, most people who do handle it treat it as a no-op. Changing (0, nil) to imply EOF is going to break code... |
Well, I was half right. Reading from a terminal does return 0, errno=0 but we turn that into 0, error=io.EOF. So we can straighten this out. I'm not going to resurrect the Go1.1 tag for this because I don't believe it's important to update every Read implementation in the tree to deal with this before we release. |
I have a wish to specify one detail of io.Reader, which AFAIK, is not specified now: 'n, err = r.Read(b))': if n == len(b) then err can be only nil or io.EOF. IOW, it allows for easy 'OK' checking like: 'if n, err = r.Read(b); n != len(b) { ... error handling goes here ... }' because io.EOF is "sticky", i.e. if some read returns {42, io.EOF} (for len(b) == 42), the next read will return {0, io.EOF}, which IIRC is guaranteed already somewhere. Of course it has a problem with len(b) == 0, for which I don't know how to properly specify the behavior. (Never yet intentionally used 'r.Read([]byte(nil))' anywhere) |
Also, zero length messages are valid in lots of protocols. TLS, for instance. And not that POSIX matters to us since we get to make up the semantics for the standard library, but we should keep the following in mind: "If nbyte is zero and the file is not a regular file, the results are unspecified." - POSIX.1-2008 on write "How read() handles zero-byte STREAMS messages is determined by the current read mode setting. [...] In message-nondiscard mode or message-discard mode, a zero-byte message shall return 0 and the message shall be removed from the STREAM." - POSIX.1-2008 on read Using a zero-length read as EOF is a very old and useful convention but that's all it should be. |
Minux, Ality, Regardless of what plan9 or TLS or X does, io.Reader doesn't define message boundaries. If you're using an io.Reader against a message-oriented input device, and yourReader.Reader(buf) = 5, nil, you can't tell the difference between 5 2 + 3 2 + 3 + 0 Likewise, things like io.LimitReader or bufio.Reader have no problems moving "boundaries" around. It's possible that a future interface (or your own) might define how a Read-like method should behave with messages and preserve boundaries (e.g. chan []byte) but io.Reader is not that interface. "Preserving" zero "messages" is not a valid argument in this discussion. In any case, per the mailing list, it's come around to being defined as a no-op. I'm writing this for background. |
Pipe behaves the same: you can do a 5 byte write to a PipeWriter, and then do 3 reads to it. The reads don't have to be on any "message" boundaries. See: http://play.golang.org/p/havfmiwUyA |
This clearly shows the preservation of message boundaries using io.Pipe: http://play.golang.org/p/YwR5a2rjWd Reads return when either the read buffer is full or the end of a message boundary is encountered. Perhaps we're using different terminology? |
That program does not contain any use of io.Reader. io.Pipe does what it does, but connecting it to an arbitrary destination using the io.Reader interface eliminates all promise of message boundaries being preserved across the boundary. You're confusing the specific properties of a particular implementation of a Read method with the general properties that a client of the io.Reader interface can assume. |
Some of you seem to be trying to define all possible properties that all possible Read methods might provide, and then arguing that io.Reader must preserve them. It won't and it can't and it shouldn't. For example, if there's bufio in between, all structure of the messages will be destroyed. The intent here is to document what the clients of the Reader interface need to know so that the properties of the interface - a byte stream, no more, no less - hold. The only open issue was the meaning of (0, nil) not being defined in the documents. Since it's a byte stream, the meaning should be no-op, and that is being documented. |
As much as I would have loved to see (0, nil) being made illegal, I do understand the impracticality, and documenting it as a no-op is far better than the current situation. But could we add some non-normative text that says that io.Reader implementations are discouraged from returning (0, nil)? |
This issue was closed by revision 5fbb54e. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: