refactor: unistd::close() does not need to be unsafe (#2495)

Author: SteveLauC

src/sys/fanotify.rs

@@ -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!");
         };

src/unistd.rs

@@ -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)
 }

test/sys/test_socket.rs

@@ -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
@@ -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(())
 }