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

Do not set File._errno when reading less than requested bytes. #2785

Merged
merged 1 commit into from
Jun 19, 2018

Conversation

mfelsche
Copy link
Contributor

While investigating a potential bug in my implementation of #2707 I found this one:

When requesting more bytes than available from a File the _errno was set to
although reading less than available bytes (> 0) is no error condition.
If anything then FileEOF should be set in this case to signal that we reached the end of file.
Nonetheless i kept it as it is right now, as i think it is more reliable to detect FileEOF when reading 0 bytes.

When requesting more bytes than available from a File the _errno was set to
although reading less than available bytes (> 0) is no error condition.
If anything then FileEOF should be set in this case to signal that we reached the end of file.
Nonetheless i kept it as it is right now, as i think it is more reliable to detect FileEOF when reading 0 bytes.
@mfelsche mfelsche added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jun 12, 2018
@mfelsche mfelsche requested a review from jemc June 16, 2018 20:54
@jemc
Copy link
Member

jemc commented Jun 18, 2018

Hmmm.. not sure about this, due to this detail from the manual for read:

POSIX allows a read() that is interrupted after reading some data to return -1 (with errno set to EINTR) or to return the number of bytes already read.

@mfelsche
Copy link
Contributor Author

@jemc i read that as, either it returns -1, which is fine, even with the change in this PR, errno will be fetched and set on File._errno. The other case should also be fine. If it was interrupted but did read some data, it will return the length and the data will be there in the buffer. Do you see this as an error condition we should handle differently?

@jemc
Copy link
Member

jemc commented Jun 19, 2018

Hmm... okay, sounds good.

@jemc jemc merged commit b596b7b into master Jun 19, 2018
@jemc jemc deleted the file-eof-error branch June 19, 2018 16:54
ponylang-main added a commit that referenced this pull request Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants