Observe close(2) errors for std::fs::{copy, write} by tbu- · Pull Request #149834 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits1 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Adds a (private to std) File::close method and uses it in std::fs::write and std::fs::copy.
This is a lighter version of rust-lang/libs-team#705 that can be added even if that ACP isn't accepted, as this is purely a quality of implementation thing that could be reverted at any time. External libraries and applications can use external crates like https://crates.io/crates/close-err to achieve the same with the files they own.
Operating system: Windows
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
| io::copy(&mut reader, &mut writer) |
| let ret = io::copy(&mut reader, &mut writer)?; |
| writer.close()?; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though this can't fail, I copied it for symmetry with the other implementations.
This comment has been minimized.
This comment has been minimized.
Adds a (private to std) File::close method and uses it in
std::fs::write and std::fs::copy.
This is a lighter version of rust-lang/libs-team#705 that can be added even if that ACP isn't accepted, as this is purely a quality of implementation thing that could be reverted at any time. External libraries and applications can use external crates like https://crates.io/crates/close-err to achieve the same with the files they own.
We're just returning the error from close(2) that we're calling anyway. I'm just changing the code from ignoring the error to returning it. Why is that such a problem? The OS is telling us something, and you'd prefer to throw the error away over returning it to the user?
Well, I both do not like the precedent, and I also think that if anything really needs to do this then these are the methods where it's the least necessary, which makes the first point worse "look, even these simple, least-defensive std methods do it!"
Labels
Operating system: Windows
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.