-
Notifications
You must be signed in to change notification settings - Fork 677
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
unistd: adding linux(glibc)/freebsd close_range. #2525
base: master
Are you sure you want to change the base?
Changes from 5 commits
2d8a9de
a09ca4f
78f4ca2
e67aeaa
7c83399
a3ca8a8
6542200
bc14f95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add `close_range` in unistd for Linux/glibc and FreeBSD |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3992,3 +3992,58 @@ pub fn chflags<P: ?Sized + NixPath>(path: &P, flags: FileFlag) -> Result<()> { | |||||
Errno::result(res).map(drop) | ||||||
} | ||||||
} | ||||||
|
||||||
#[cfg(any( | ||||||
all(target_os = "linux", target_env = "gnu"), | ||||||
target_os = "freebsd" | ||||||
))] | ||||||
#[cfg(feature = "fs")] | ||||||
libc_bitflags! { | ||||||
/// Options for close_range() | ||||||
#[cfg_attr(docsrs, doc(cfg(feature = "fs")))] | ||||||
pub struct CloseRangeFlags : c_uint { | ||||||
#[cfg(all(target_os = "linux", target_env = "gnu"))] | ||||||
/// Unshare the file descriptors range before closing them | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Considering that the doc in the man page is kinda misleading, let's fix it on our side:
Suggested change
|
||||||
CLOSE_RANGE_UNSHARE; | ||||||
/// Set the close-on-exec flag on the file descriptors range | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's be explicit:
Suggested change
|
||||||
CLOSE_RANGE_CLOEXEC; | ||||||
} | ||||||
} | ||||||
|
||||||
feature! { | ||||||
#![feature = "fs"] | ||||||
|
||||||
/// Close all the file descriptor from a given range. | ||||||
/// An optional flag can be applied to modify its behavior. | ||||||
#[cfg(any( | ||||||
all(target_os = "linux", target_env = "gnu"), | ||||||
target_os = "freebsd" | ||||||
))] | ||||||
pub fn close_range<F: std::os::fd::AsFd>(fdbegin: F, fdlast: F, flags: CloseRangeFlags) -> Result<Option<c_int>> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am curious why it returns a |
||||||
use std::os::fd::AsRawFd; | ||||||
|
||||||
let raw = unsafe { | ||||||
Errno::clear(); | ||||||
|
||||||
cfg_if! { | ||||||
if #[cfg(all(target_os = "linux", target_env = "gnu"))] { | ||||||
libc::syscall(libc::SYS_close_range, fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. glibc added the wrapper in 2.34, and the libc crate has it exposed, any reason why we are using a raw syscall here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally I call the wrapper but I think it is due to the CI when there is old glibc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, some CIs are still using Ubuntu 20.04 due to this issue, which has glibc 2.31 😮💨, perhaps we can try bumping them to see if we are lucky now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can do that if we do not need to bother supporting older releases out in the field. I may come back to it later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, if we use glibc wrapper, then using this interface would require glibc 2.34 or above. We are still not lucky regarding that test: #2533, perhaps we should disable the test under QEMU then 😪 |
||||||
} else { | ||||||
libc::close_range(fdbegin.as_fd().as_raw_fd() as u32, fdlast.as_fd().as_raw_fd() as u32, flags.bits() as i32) | ||||||
} | ||||||
} | ||||||
}; | ||||||
if raw == -1 { | ||||||
if Errno::last_raw() == 0 { | ||||||
Ok(None) | ||||||
} else { | ||||||
Err(Errno::last()) | ||||||
} | ||||||
} else { | ||||||
#[cfg(all(target_os = "linux", target_env = "gnu", target_pointer_width = "64"))] | ||||||
let raw = raw as i32; | ||||||
Ok(Some(raw)) | ||||||
} | ||||||
|
||||||
} | ||||||
} |
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 is interesting, libc defines these 2 constants as unsigned integers. 🤔