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

Fix File.writev and File.flush in cases where the IO vector exceeds IOV_MAX. #2771

Merged
merged 1 commit into from
Jun 9, 2018

Conversation

jemc
Copy link
Member

@jemc jemc commented Jun 9, 2018

Currently, when the _pending_writes IO vector inside Fileexceeds IOV_MAX, we enter a loop of iterating over chunks of the IO vector of size less than or equal to IOV_MAX in count. This loop was invalid, causing out-of-bounds array accesses that bubbled up an error.

This PR fixes the loop logic, and updates the tests to catch such issues.

It also cleans up some of the code and comment formatting issues in File._write_to_disk.

@jemc
Copy link
Member Author

jemc commented Jun 9, 2018

Note that this is just one of several issues I encountered with the File class, so there are a few more PRs to come.

#2526 is a separate issue.

Copy link
Contributor

@dipinhora dipinhora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding and fixing my error (and cleaning up the code).

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jun 9, 2018
@SeanTAllen SeanTAllen merged commit 2ae305d into master Jun 9, 2018
@SeanTAllen SeanTAllen deleted the fix/file-writev-flush branch June 9, 2018 17:41
ponylang-main added a commit that referenced this pull request Jun 9, 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.

3 participants