Skip to content

Commit

Permalink
refactor: unistd::close() does not need to be unsafe (#2495)
Browse files Browse the repository at this point in the history
  • Loading branch information
SteveLauC authored Sep 9, 2024
1 parent 81acce2 commit 65d90b9
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 43 deletions.
6 changes: 1 addition & 5 deletions src/sys/fanotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,7 @@ impl Drop for FanotifyEvent {
if self.0.fd == libc::FAN_NOFD {
return;
}
// SAFETY:
//
// If this fd is not `FAN_NOFD`, then it should be a valid, owned file
// descriptor, which means we can safely close it.
let e = unsafe { close(self.0.fd) };
let e = close(self.0.fd);
if !std::thread::panicking() && e == Err(Errno::EBADF) {
panic!("Closing an invalid file descriptor!");
};
Expand Down
35 changes: 3 additions & 32 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1340,38 +1340,9 @@ pub fn gethostname() -> Result<OsString> {

/// Close a file descriptor.
///
/// # Safety
///
/// If you pass a `RawFd` to this function, ensure that this `close()` won't
/// trigger a double close.
///
/// ```no_run
/// use std::os::unix::io::AsRawFd;
/// use nix::unistd::close;
///
/// let f = tempfile::tempfile().unwrap();
/// // SAFETY:
/// //
/// // NOT safe! f will also close on drop!
/// unsafe { close(f.as_raw_fd()).unwrap() };
/// ```
///
/// We should pass `f` by value:
///
/// In the following case, it is generally preferred to call `drop(f)` rather
/// than `close()`.
///
/// ```rust
/// use std::os::unix::io::IntoRawFd;
/// use nix::unistd::close;
///
/// let f = tempfile::tempfile().unwrap();
/// // SAFETY:
/// //
/// // We are safe! `into_raw_fd()` consumes f
/// unsafe { close(f).unwrap() };
/// ```
pub unsafe fn close<Fd: std::os::fd::IntoRawFd>(fd: Fd) -> Result<()> {
/// If `fd` is an owned file descriptor, it is generally preferred to call
/// `drop(fd)` rather than `close(fd)`.
pub fn close<Fd: std::os::fd::IntoRawFd>(fd: Fd) -> Result<()> {
let res = unsafe { libc::close(fd.into_raw_fd()) };
Errno::result(res).map(drop)
}
Expand Down
8 changes: 2 additions & 6 deletions test/sys/test_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,9 +914,7 @@ pub fn test_scm_rights() {
unsafe { std::os::fd::BorrowedFd::borrow_raw(received_r) };
read(borrowed_received_r, &mut buf).unwrap();
assert_eq!(&buf[..], b"world");
// SAFETY:
// there shouldn't be double close
unsafe { close(received_r).unwrap() };
close(received_r).unwrap();
}

// Disable the test on emulated platforms due to not enabled support of AF_ALG in QEMU from rust cross
Expand Down Expand Up @@ -1645,9 +1643,7 @@ fn test_impl_scm_credentials_and_rights(
unsafe { std::os::fd::BorrowedFd::borrow_raw(received_r) };
read(received_r_borrowed, &mut buf).unwrap();
assert_eq!(&buf[..], b"world");
// SAFETY:
// double-close won't happen
unsafe { close(received_r).unwrap() };
close(received_r).unwrap();

Ok(())
}
Expand Down

0 comments on commit 65d90b9

Please sign in to comment.