Skip to content

Commit

Permalink
unistd: make getpeereid take AsFd rather than a raw FD (#2137)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
woodruffw authored Sep 30, 2023
1 parent 154a8a4 commit 90e3c3a
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 45 deletions.
19 changes: 17 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 10 additions & 2 deletions src/sys/socket/sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")))]
Expand All @@ -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")))]
Expand Down
20 changes: 10 additions & 10 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pid> {
let res = unsafe { libc::tcgetpgrp(fd) };
pub fn tcgetpgrp<F: AsFd>(fd: F) -> Result<Pid> {
let res = unsafe { libc::tcgetpgrp(fd.as_fd().as_raw_fd()) };
Errno::result(res).map(Pid)
}
/// Set the terminal foreground process group (see
Expand All @@ -376,8 +376,8 @@ pub fn tcgetpgrp(fd: c_int) -> Result<Pid> {
/// 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<F: AsFd>(fd: F, pgrp: Pid) -> Result<()> {
let res = unsafe { libc::tcsetpgrp(fd.as_fd().as_raw_fd(), pgrp.into()) };
Errno::result(res).map(drop)
}
}
Expand Down Expand Up @@ -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<Option<c_long>> {
pub fn fpathconf<F: AsFd>(fd: F, var: PathconfVar) -> Result<Option<c_long>> {
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 {
Expand Down Expand Up @@ -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<PathBuf> {
pub fn ttyname<F: AsFd>(fd: F) -> Result<PathBuf> {
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));
}
Expand All @@ -3943,11 +3943,11 @@ feature! {
target_os = "netbsd",
target_os = "dragonfly",
))]
pub fn getpeereid(fd: RawFd) -> Result<(Uid, Gid)> {
pub fn getpeereid<F: AsFd>(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)))
}
Expand Down
39 changes: 8 additions & 31 deletions test/test_unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"));
Expand All @@ -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]
Expand All @@ -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();
Expand All @@ -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() {
Expand Down

0 comments on commit 90e3c3a

Please sign in to comment.