Skip to content

Fall back to fcntl if ioctl(FIOCLEX, ..) is unsupported#41462

Closed
tbu- wants to merge 1 commit into
rust-lang:masterfrom
tbu-:pr_cloexec_fallback_fcntl
Closed

Fall back to fcntl if ioctl(FIOCLEX, ..) is unsupported#41462
tbu- wants to merge 1 commit into
rust-lang:masterfrom
tbu-:pr_cloexec_fallback_fcntl

Conversation

@tbu-

@tbu- tbu- commented Apr 22, 2017

Copy link
Copy Markdown
Contributor

Fixes #41347.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @BurntSushi

(rust_highfive has picked a reviewer for you, use r? to override)

@eddyb

eddyb commented Apr 22, 2017

Copy link
Copy Markdown
Contributor

cc @rust-lang/libs

@Mark-Simulacrum

Copy link
Copy Markdown
Member

Thanks for the PR! We’ll periodically check in on it to make sure that @BurntSushi or someone else from the team reviews it soon.

@BurntSushi

Copy link
Copy Markdown
Member

@tbu- Sorry, I'm missing a lot of context here. I don't have any idea what this change actually does or what problem it solves. Could you perhaps say more about what's happening here?

@eddyb

eddyb commented Apr 23, 2017

Copy link
Copy Markdown
Contributor

@BurntSushi If you see the linked issue, this change is trying to provide support for faux-linux systems that don't support the ioctl (I think it's similar to Linux 2.4 but I'm not sure).

@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 23, 2017
@alexcrichton

Copy link
Copy Markdown
Member

Could you expand the description in the PR to describe the problem? And can you also leave a comment as to why the code is structured the way it is?

I'd personally prefer to not go too far down the road of supporting pre-2.6.18 Linux kernels, so is this literally intended to support Linux 2.4 or is it portability to Esxi?

@alexcrichton alexcrichton added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team labels Apr 24, 2017
@arielb1

arielb1 commented May 2, 2017

Copy link
Copy Markdown
Contributor

@tbu-

Could you describe which kernels are you trying to support with this PR?

@tbu-

tbu- commented May 4, 2017

Copy link
Copy Markdown
Contributor Author

This was just trying to fix the issue that was encountered in #41437. I don't know if it needs other stuff as well.

I'm not affected by this, so I'm not too attached to this patch.

@alexcrichton

Copy link
Copy Markdown
Member

Ok, in that case I'm personally inclined to hold off for now. We don't know if the target in #41347 actually warrants an entirely new compiler target or if this change is enough to get the program working. In light of that it seems like it may be premature to do this?

@tbu-

tbu- commented May 4, 2017

Copy link
Copy Markdown
Contributor Author

OK.

@tbu- tbu- closed this May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants