Skip to content

Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND #82417

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 12, 2021

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Feb 22, 2021

fixes #82410

While sendfile() returns EINVAL when the output was opened with O_APPEND, copy_file_range() does not and returns EBADF instead, which – unlike other EBADF causes – is not fatal for this operation since a regular write() will likely succeed.

We now treat EBADF as a non-fatal error for copy_file_range and fall back to a read-write copy as we already did for several other errors.

@rust-highfive
Copy link
Contributor

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2021
@the8472 the8472 force-pushed the fix-copy_file_range-append branch from 680c9fc to 81602fb Compare February 22, 2021 20:41
@the8472 the8472 changed the title Fix copy file range append Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND Feb 22, 2021
@pnkfelix pnkfelix added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Mar 11, 2021
@m-ou-se m-ou-se assigned m-ou-se and unassigned kennytm Mar 11, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 11, 2021

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 11, 2021

📌 Commit 81602fb has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2021
@m-ou-se m-ou-se added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Mar 11, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 11, 2021

Nominating for backport. It's not a critical issue as the copy wil just fail (it doesn't produce incorrect data), but it's a trivial change to backport. (Probably not worth a stable patch release by itself. But if one happens, it'd be good to include this.)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 11, 2021
… r=m-ou-se

Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND

fixes rust-lang#82410

While `sendfile()` returns `EINVAL` when the output was opened with O_APPEND,  `copy_file_range()` does not and returns `EBADF` instead, which – unlike other `EBADF` causes – is not fatal for this operation since a regular `write()` will likely succeed.

We now treat `EBADF` as a non-fatal error for `copy_file_range` and fall back to a read-write copy as we already did for several other errors.
@bors
Copy link
Collaborator

bors commented Mar 11, 2021

⌛ Testing commit 81602fb with merge 03e864f...

@bors
Copy link
Collaborator

bors commented Mar 12, 2021

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 03e864f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 12, 2021
@bors bors merged commit 03e864f into rust-lang:master Mar 12, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 12, 2021
@pnkfelix
Copy link
Member

discussed in T-compiler meeting. beta-backport approved.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 18, 2021
@pnkfelix
Copy link
Member

stable approved too (not that it matters since release is next week so we're unlikely to have a point release)

@pnkfelix pnkfelix added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Mar 18, 2021
@Mark-Simulacrum Mark-Simulacrum removed the stable-accepted Accepted for backporting to the compiler in the stable channel. label Mar 19, 2021
@Mark-Simulacrum Mark-Simulacrum removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Mar 19, 2021
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.52.0, 1.51.0 Mar 22, 2021
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 22, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
…imulacrum

[stable] 1.51.0 release

Also includes backports of the release notes, as well as:

*  SplitInclusive is public API rust-lang#83372
*  std: Fix a bug on the wasm32-wasi target opening files rust-lang#82804
*  Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND rust-lang#82417

r? `@Mark-Simulacrum`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std::io::copy returns Bad File Descriptor with writer files in append mode in 1.50
8 participants