From 90e3c3a1ef9a62a0294bb61de4fcd54731a898bf Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sat, 30 Sep 2023 00:00:37 -0400 Subject: [PATCH] unistd: make getpeereid take AsFd rather than a raw FD (#2137) * unistd: make getpeereid take AsFd rather than a raw FD `AsFd` returns a `BorrowedFd` that is correct by construction, meaning that this API rejects more error states at the type level (such as passing in `fd = -1`). * CHANGELOG: record changes * unistd: pass AsFd impl by value * unistd: more AsFd usage * CHANGELOG: update changes --- CHANGELOG.md | 19 +++++++++++++++++-- src/sys/socket/sockopt.rs | 12 ++++++++++-- src/unistd.rs | 20 ++++++++++---------- test/test_unistd.rs | 39 ++++++++------------------------------- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a171afd68..a1a99728c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,21 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] - ReleaseDate + +### Changed + +- The following APIs now take an implementation of `AsFd` rather than a + `RawFd`: + + - `unistd::tcgetpgrp` + - `unistd::tcsetpgrp` + - `unistd::fpathconf` + - `unistd::ttyname` + - `unistd::getpeereid` + + ([#2137](https://github.com/nix-rust/nix/pull/2137)) + ## [0.27.1] - 2023-08-28 ### Fixed @@ -99,7 +114,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## [0.26.3] - 2023-08-27 ### Fixed -- Fix: send `ETH_P_ALL` in htons format +- Fix: send `ETH_P_ALL` in htons format ([#1925](https://github.com/nix-rust/nix/pull/1925)) - Fix: `recvmsg` now sets the length of the received `sockaddr_un` field correctly on Linux platforms. ([#2041](https://github.com/nix-rust/nix/pull/2041)) @@ -187,7 +202,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ([#1824](https://github.com/nix-rust/nix/pull/1824)) - Workaround XNU bug causing netmasks returned by `getifaddrs` to misbehave. ([#1788](https://github.com/nix-rust/nix/pull/1788)) - + ### Removed - Removed deprecated error constants and conversions. diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs index 44f3ebbc1d..3192df1fa6 100644 --- a/src/sys/socket/sockopt.rs +++ b/src/sys/socket/sockopt.rs @@ -550,7 +550,11 @@ cfg_if! { TcpMaxSeg, GetOnly, libc::IPPROTO_TCP, libc::TCP_MAXSEG, u32); } } -#[cfg(not(any(target_os = "openbsd", target_os = "haiku", target_os = "redox")))] +#[cfg(not(any( + target_os = "openbsd", + target_os = "haiku", + target_os = "redox" +)))] #[cfg(feature = "net")] sockopt_impl!( #[cfg_attr(docsrs, doc(cfg(feature = "net")))] @@ -572,7 +576,11 @@ sockopt_impl!( libc::TCP_REPAIR, u32 ); -#[cfg(not(any(target_os = "openbsd", target_os = "haiku", target_os = "redox")))] +#[cfg(not(any( + target_os = "openbsd", + target_os = "haiku", + target_os = "redox" +)))] #[cfg(feature = "net")] sockopt_impl!( #[cfg_attr(docsrs, doc(cfg(feature = "net")))] diff --git a/src/unistd.rs b/src/unistd.rs index bb9f1c1f67..d950ad6ee1 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -366,8 +366,8 @@ feature! { /// Get the group process id (GPID) of the foreground process group on the /// terminal associated to file descriptor (FD). #[inline] -pub fn tcgetpgrp(fd: c_int) -> Result { - let res = unsafe { libc::tcgetpgrp(fd) }; +pub fn tcgetpgrp(fd: F) -> Result { + let res = unsafe { libc::tcgetpgrp(fd.as_fd().as_raw_fd()) }; Errno::result(res).map(Pid) } /// Set the terminal foreground process group (see @@ -376,8 +376,8 @@ pub fn tcgetpgrp(fd: c_int) -> Result { /// Get the group process id (PGID) to the foreground process group on the /// terminal associated to file descriptor (FD). #[inline] -pub fn tcsetpgrp(fd: c_int, pgrp: Pid) -> Result<()> { - let res = unsafe { libc::tcsetpgrp(fd, pgrp.into()) }; +pub fn tcsetpgrp(fd: F, pgrp: Pid) -> Result<()> { + let res = unsafe { libc::tcsetpgrp(fd.as_fd().as_raw_fd(), pgrp.into()) }; Errno::result(res).map(drop) } } @@ -2192,10 +2192,10 @@ pub enum PathconfVar { /// - `Ok(None)`: the variable has no limit (for limit variables) or is /// unsupported (for option variables) /// - `Err(x)`: an error occurred -pub fn fpathconf(fd: RawFd, var: PathconfVar) -> Result> { +pub fn fpathconf(fd: F, var: PathconfVar) -> Result> { let raw = unsafe { Errno::clear(); - libc::fpathconf(fd, var as c_int) + libc::fpathconf(fd.as_fd().as_raw_fd(), var as c_int) }; if raw == -1 { if errno::errno() == 0 { @@ -3913,12 +3913,12 @@ feature! { /// Get the name of the terminal device that is open on file descriptor fd /// (see [`ttyname(3)`](https://man7.org/linux/man-pages/man3/ttyname.3.html)). #[cfg(not(target_os = "fuchsia"))] -pub fn ttyname(fd: RawFd) -> Result { +pub fn ttyname(fd: F) -> Result { const PATH_MAX: usize = libc::PATH_MAX as usize; let mut buf = vec![0_u8; PATH_MAX]; let c_buf = buf.as_mut_ptr() as *mut libc::c_char; - let ret = unsafe { libc::ttyname_r(fd, c_buf, buf.len()) }; + let ret = unsafe { libc::ttyname_r(fd.as_fd().as_raw_fd(), c_buf, buf.len()) }; if ret != 0 { return Err(Errno::from_i32(ret)); } @@ -3943,11 +3943,11 @@ feature! { target_os = "netbsd", target_os = "dragonfly", ))] -pub fn getpeereid(fd: RawFd) -> Result<(Uid, Gid)> { +pub fn getpeereid(fd: F) -> Result<(Uid, Gid)> { let mut uid = 1; let mut gid = 1; - let ret = unsafe { libc::getpeereid(fd, &mut uid, &mut gid) }; + let ret = unsafe { libc::getpeereid(fd.as_fd().as_raw_fd(), &mut uid, &mut gid) }; Errno::result(ret).map(|_| (Uid(uid), Gid(gid))) } diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 10284e4127..80abdc436d 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -635,7 +635,7 @@ fn test_acct() { fn test_fpathconf_limited() { let f = tempfile().unwrap(); // AFAIK, PATH_MAX is limited on all platforms, so it makes a good test - let path_max = fpathconf(f.as_raw_fd(), PathconfVar::PATH_MAX); + let path_max = fpathconf(f, PathconfVar::PATH_MAX); assert!( path_max .expect("fpathconf failed") @@ -1230,9 +1230,11 @@ fn test_ttyname() { grantpt(&fd).expect("grantpt failed"); unlockpt(&fd).expect("unlockpt failed"); let sname = unsafe { ptsname(&fd) }.expect("ptsname failed"); - let fds = open(Path::new(&sname), OFlag::O_RDWR, stat::Mode::empty()) + let fds = fs::OpenOptions::new() + .read(true) + .write(true) + .open(Path::new(&sname)) .expect("open failed"); - assert!(fds > 0); let name = ttyname(fds).expect("ttyname failed"); assert!(name.starts_with("/dev")); @@ -1242,18 +1244,7 @@ fn test_ttyname() { #[cfg(not(any(target_os = "redox", target_os = "fuchsia")))] fn test_ttyname_not_pty() { let fd = File::open("/dev/zero").unwrap(); - assert!(fd.as_raw_fd() > 0); - assert_eq!(ttyname(fd.as_raw_fd()), Err(Errno::ENOTTY)); -} - -#[test] -#[cfg(not(any( - target_os = "redox", - target_os = "fuchsia", - target_os = "haiku" -)))] -fn test_ttyname_invalid_fd() { - assert_eq!(ttyname(-1), Err(Errno::EBADF)); + assert_eq!(ttyname(fd), Err(Errno::ENOTTY)); } #[test] @@ -1269,8 +1260,8 @@ fn test_getpeereid() { use std::os::unix::net::UnixStream; let (sock_a, sock_b) = UnixStream::pair().unwrap(); - let (uid_a, gid_a) = getpeereid(sock_a.as_raw_fd()).unwrap(); - let (uid_b, gid_b) = getpeereid(sock_b.as_raw_fd()).unwrap(); + let (uid_a, gid_a) = getpeereid(sock_a).unwrap(); + let (uid_b, gid_b) = getpeereid(sock_b).unwrap(); let uid = geteuid(); let gid = getegid(); @@ -1281,20 +1272,6 @@ fn test_getpeereid() { assert_eq!(gid_a, gid_b); } -#[test] -#[cfg(any( - target_os = "macos", - target_os = "ios", - target_os = "freebsd", - target_os = "openbsd", - target_os = "netbsd", - target_os = "dragonfly", -))] -fn test_getpeereid_invalid_fd() { - // getpeereid is not POSIX, so error codes are inconsistent between different Unices. - getpeereid(-1).expect_err("assertion failed"); -} - #[test] #[cfg(not(target_os = "redox"))] fn test_faccessat_none_not_existing() {