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

os: improve file.Read method description #6639

Closed
btracey opened this issue Oct 21, 2013 · 7 comments
Closed

os: improve file.Read method description #6639

btracey opened this issue Oct 21, 2013 · 7 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@btracey
Copy link
Contributor

btracey commented Oct 21, 2013

The current description for file.Read is 

"Read reads up to len(b) bytes from the File. It returns the number of bytes read
and an error, if any. EOF is signaled by a zero count with err set to io.EOF."

I found the second sentence confusing. I read it to mean "If the end of the file is
reached, n will be set to zero and err will be set to io.EOF" which is not the
actual behavior. Could you change the wording to be something like 

"Read reads up to len(b) bytes from the File. It returns the number of bytes read
and an error, if any occurred. If the end of the file is reached, the number of bytes
read will be less than the length of the buffer. The buffer will not be truncated. If no
bytes are read, err is set to io.EOF, otherwise no error will be signaled"
@bradfitz
Copy link
Contributor

Comment 1:

This could be clarified, but your sentence "The buffer will not be truncated." doesn't
make much since, since the caller owns the slice.  The File.Read method doesn't get a
pointer to the slice, so it wouldn't even be possible for the Read method to truncate it.
For reference, the io.Reader docs are more clear: http://tip.golang.org/pkg/io/#Reader
What's not clear is whether the File.Read docs are trying to promise a tighter contract
than io.Reader.Read itself.

Labels changed: added documentation.

@btracey
Copy link
Contributor Author

btracey commented Oct 21, 2013

Comment 2:

Good point on the buffer. You are right, that clarification is unnecessary.
The io.Reader description is better. Perhaps file could link to that description and
explicitly state the fact that no io.EOF error is returned when n > 0 (the io
description specifies that either may happen).
Perhaps this should be a different issue, but I'm confused by " and callers should treat
that situation as a no-op" that is the last sentence of the io.Reader description. Is
that just saying there should be a check like
if bytesLeftInReader == 0{
    return 0, io.EOF
}
?

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 3:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-main.

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@gopherbot
Copy link

CL https://golang.org/cl/22220 mentions this issue.

@bradfitz bradfitz modified the milestones: Go1.7, Unplanned Apr 19, 2016
@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@gopherbot
Copy link

CL https://golang.org/cl/32102 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants