Use less syscalls in anon_pipe()
#39386
Conversation
Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return.
|
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
| @@ -29,34 +30,33 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { | |||
| // CLOEXEC flag is to use the `pipe2` syscall on Linux. This was added in | |||
| // 2.6.27, however, and because we support 2.6.18 we must detect this | |||
| // support dynamically. | |||
| if cfg!(target_os = "linux") { | |||
| static TRY_PIPE2: AtomicBool = AtomicBool::new(cfg!(target_os = "linux")); | |||
There was a problem hiding this comment.
Preexisting:
pipe2 is also supported on freebsd (>= 10.0), netbsd (>= 6.0), dragonflybsd (appeared ~4.0-ish?), openbsd (>= 5.7). These probably should also appear in that cfg! above.
|
Looking back on this, I'm not actually sure why handling ENOSYS is here at all. We're not doing a syscall manually, but instead calling a glibc function. That means if it exists we won't get ENOSYS from what I understand. In that case can we just remove that branch? |
|
True. |
We're not calling the raw syscall but a libc function, the libc will have a compatibility layer.
|
@alexcrichton Removed the branch. |
|
@bors: r+ |
|
📌 Commit 4b46d2a has been approved by |
|
⌛ Testing commit 4b46d2a with merge 1f7bafc... |
|
💔 Test failed - status-travis |
|
… On Tue, Jan 31, 2017 at 6:32 AM bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/196950142>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39386 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95JBIP5yPoli7-Tc2FqMOyS6PIQmeks5rX0XogaJpZM4LwwFj>
.
|
|
⌛ Testing commit 4b46d2a with merge 3b88122... |
|
💔 Test failed - status-appveyor |
|
Some |
|
@bors: retry
…On Wed, Feb 1, 2017 at 3:40 AM, tbu- ***@***.***> wrote:
Some curl failed, looks unrelated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39386 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95GceoiA1bLhus7KySDS65ccGPpQ-ks5rYG85gaJpZM4LwwFj>
.
|
Use less syscalls in `anon_pipe()` Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return.
|
☀️ Test successful - status-appveyor, status-travis |
This is not necessarily true. Glibc has had the |
|
Ack sorry about that @cuviper! The fix here would be to just reinstate |
|
Yeah, that should do it! |
|
Ok I'll send a PR for this. Looks like this made it into 1.17.0 but we can at least backport and get it fixed for 1.19.0 |
Should help fix an accidental regression from rust-lang#39386.
std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from rust-lang#39386.
std: Handle ENOSYS when calling `pipe2` Should help fix an accidental regression from #39386.
Should help fix an accidental regression from rust-lang#39386.
Save a
ENOSYSfailure frompipe2and don't try again.Use
cvtinstead ofcvt_rforpipe2-EINTRis not an errorpipe2can return.