Skip to content
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

Add unistd::close_range() #2153

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
MacOS, FreeBSD, DragonflyBSD, Android, iOS and Haiku.
([#2074](https://github.com/nix-rust/nix/pull/2074))

- Added `close_range(2)` on Linux
SteveLauC marked this conversation as resolved.
Show resolved Hide resolved
([#2153](https://github.com/nix-rust/nix/pull/2153))

## [0.27.1] - 2023-08-28

### Fixed
Expand Down
56 changes: 56 additions & 0 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,62 @@ pub fn close(fd: RawFd) -> Result<()> {
Errno::result(res).map(drop)
}

#[cfg(target_os = "linux")]
libc_bitflags! {
/// Options for close_range()
pub struct CloseRangeFlags : c_uint {
/// Unshare file descriptors before closing them.
CLOSE_RANGE_UNSHARE;
/// Set close-on-exec instead of closing file descriptors.
CLOSE_RANGE_CLOEXEC;
}
}

/// Close a range of raw file descriptors.
/// [close_range(2)](https://man7.org/linux/man-pages/man2/close_range.2.html).
///
/// # Safety
///
/// Be aware that many Rust types implicitly close-on-drop, including
/// `std::fs::File`. Explicitly closing them with this method too can result in
/// a double-close condition, which can cause confusing `EBADF` errors in
/// seemingly unrelated code. Caveat programmer.
///
/// # Examples
///
/// ```no_run
/// use std::os::unix::io::AsRawFd;
/// use nix::unistd::{close_range, CloseRangeFlags};
///
/// let f = tempfile::tempfile().unwrap();
/// unsafe { close_range(f.as_raw_fd(), f.as_raw_fd(), CloseRangeFlags::empty()).unwrap() }; // Bad! f will also close on drop!
/// ```
///
/// ```rust
/// use std::os::unix::io::IntoRawFd;
/// use nix::unistd::{close_range, CloseRangeFlags};
///
/// let f = tempfile::tempfile().unwrap();
/// let fd = f.into_raw_fd(); // Good. into_raw_fd consumes f
/// unsafe { close_range(fd, fd, CloseRangeFlags::empty()).unwrap(); }
/// ```
#[cfg(target_os = "linux")]
pub unsafe fn close_range(
first: RawFd,
last: RawFd,
flags: CloseRangeFlags,
) -> Result<()> {
let res = unsafe {
libc::syscall(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be using raw syscalls here. They lack type safety, and they aren't portable. Instead, you should add close_range in a PR to libc. Then , once that merges, you can add support to Nix. If you're ambitious, you might consider adding closefrom while you're at it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I was following the precedent of other functions in this file (eg: gettid, execveat, and pivot_root).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For gettid() and execveat(), see this issue #2156, for pivot_root, there seems to be no libc wrapper for it, so we have to use the syscall directly.


close_range() has a wrapper in glibc (now sure about other libcs), can you please add it to the upstream libc crate and then we can add it in nix

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, that makes sense.

For my use case I need to statically link and are therefore using musl which does not yet implement close_range().

libc::SYS_close_range,
first as libc::c_uint,
last as libc::c_uint,
flags.bits(),
)
};
Errno::result(res).map(drop)
}

/// Read from a raw file descriptor.
///
/// See also [read(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html)
Expand Down