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

unistd: adding linux(glibc)/freebsd close_range. #2525

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 46 additions & 0 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3992,3 +3992,49 @@ 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 {
Copy link
Member

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. 🤔

#[cfg(all(target_os = "linux", target_env = "gnu"))]
/// Unshare the file descriptors range before closing them
Copy link
Member

Choose a reason for hiding this comment

The 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
/// Unshare the file descriptors range before closing them
/// Unshare the file descriptor table, then close the file descriptors specified in the range.

CLOSE_RANGE_UNSHARE;
/// Set the close-on-exec flag on the file descriptors range
Copy link
Member

Choose a reason for hiding this comment

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

Let's be explicit:

Suggested change
/// Set the close-on-exec flag on the file descriptors range
/// Set the close-on-exec flag on the file descriptors range instead of closing them.

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>> {
Copy link
Member

Choose a reason for hiding this comment

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

I am curious why it returns a Result<Option<c_int>>, and the reason behind the code related to Errno. From the man page, it returns -1 on error, and 0 on success, looks like it is just a normal syscall👀

use std::os::fd::AsRawFd;

let raw = unsafe {
Errno::clear();
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 {
Ok(Some(raw))
}

}
}
15 changes: 15 additions & 0 deletions test/test_unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,3 +1391,18 @@ fn test_group_from() {
assert_eq!(group.gid, group_id);
assert_eq!(group.name, "wheel");
}

#[test]
#[cfg(any(all(target_os = "linux", target_env = "gnu"), target_os = "freebsd"))]
fn test_close_range() {
use tempfile::NamedTempFile;
const CONTENTS: &[u8] = b"abcdef123456";
let mut tempfile1 = NamedTempFile::new().unwrap();
let mut tempfile2 = NamedTempFile::new().unwrap();
let mut tempfile3 = NamedTempFile::new().unwrap();
tempfile3.write_all(CONTENTS).unwrap();
tempfile2.write_all(CONTENTS).unwrap();
tempfile1.write_all(CONTENTS).unwrap();
let areclosed = close_range(tempfile1, tempfile3, CloseRangeFlags::CLOSE_RANGE_CLOEXEC);
Copy link
Member

Choose a reason for hiding this comment

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

In a multithreaded program, which this is, there is no guarantee that tempfile3's file descriptor will be higher than tempfile1's.

assert_eq!(areclosed.expect("close_range failed").expect("invalid flag"), 0);
}