-
Notifications
You must be signed in to change notification settings - Fork 666
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
refactor: I/O safety for unistd.rs #2440
refactor: I/O safety for unistd.rs #2440
Conversation
f7e7382
to
46a79c5
Compare
46a79c5
to
edcb0dd
Compare
))] | ||
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int { | ||
#[cfg(all(feature = "fanotify", target_os = "linux"))] | ||
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> RawFd { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be removed when we add I/O safety to module fanotify
.
@@ -73,7 +73,7 @@ impl IntoRawFd for PtyMaster { | |||
|
|||
impl io::Read for PtyMaster { | |||
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { | |||
unistd::read(self.0.as_raw_fd(), buf).map_err(io::Error::from) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Read
implementation (and Write
) feels a little bit weird to me, I kinda think we should not implement std Read/Write traits for Nix wrapper types, the I/O syscalls should be used instead.
But this implementation would definitely make our interface more rusty, so I am not quite against it, though I think we should be consistent, i.e., if one wrapper type has this trait implemented, we should do this to all the wrapper types.
src/unistd.rs
Outdated
@@ -1295,19 +1504,21 @@ impl LinkatFlags { | |||
/// # References | |||
/// See also [linkat(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/linkat.html) | |||
#[cfg(not(target_os = "redox"))] // RedoxFS does not support symlinks yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if hard link is supported by RedoxFS, but linkat() is for hard link rather than symlink
What does this PR do
Add I/O safety to module unistd.
File descriptor duplication
The safe interface for
dup2()
proposed in this PR looks like this:We take
newfd
by mutable reference so that we can ensure we have exclusive access. This interface stops users from specifyingnewfd
with arbitrary fd values,unsafe fn dup2_raw()
comes as a remedy:dup2_raw()
is unsafe because when specifying a fd that is open, one has to ensure the returnedOwnedFd
is the ONLY owner of this fd, or a double close will happen.Using the above
dup2()
interface for stdin/stdout/stderr redirection is troublesome, one has to construct anOwnedFd
from stdin/stdout/stderr, thenmem::forget()
it. We provide 3 helper functions to make this easier:Since the only flag that can be used with
dup3()
isO_CLOEXEC
, which does not make much sense to stdin/stdout/stderr, I didn't provide the dup3 versions of these utilities.Checklist:
CONTRIBUTING.md