Fix tcgetpgrp and tcsetpgrp on Linux. (#910)

On Linux, fix how the pid argument is passed to the `TIOCSPGRP` ioctl used
by `tcsetgrp`.

And, on Linux, it appears `tcgetpgrp` can return a pid of 0 when the fd is
a pseudo-terminal device. This isn't documented behavior, and isn't
compatible with rustix's current signaure for `tcgetpgrp` since it returns
a `Pid` which can't be zero, so handle this situation by having it return
`Errno::OPNOTSUPP` for now.

Author: sunfishcode

src/backend/libc/termios/syscalls.rs

@@ -115,6 +115,15 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result<Termios> {
 pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
     unsafe {
         let pid = ret_pid_t(c::tcgetpgrp(borrowed_fd(fd)))?;
+
+        // This doesn't appear to be documented, but on Linux, it appears
+        // `tcsetpgrp` can succceed and set the pid to 0 if we pass it a
+        // pseudo-terminal device fd. For now, translate it into `OPNOTSUPP`.
+        #[cfg(linux_kernel)]
+        if pid == 0 {
+            return Err(io::Errno::OPNOTSUPP);
+        }
+
         Ok(Pid::from_raw_unchecked(pid))
     }
 }

src/backend/linux_raw/termios/syscalls.rs

@@ -86,6 +86,14 @@ pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
         let mut result = MaybeUninit::<c::pid_t>::uninit();
         ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGPGRP), &mut result))?;
         let pid = result.assume_init();
+
+        // This doesn't appear to be documented, but it appears `tcsetpgrp` can
+        // succceed and set the pid to 0 if we pass it a pseudo-terminal device
+        // fd. For now, fail with `OPNOTSUPP`.
+        if pid == 0 {
+            return Err(io::Errno::OPNOTSUPP);
+        }
+
         Ok(Pid::from_raw_unchecked(pid))
     }
 }
@@ -177,7 +185,15 @@ pub(crate) fn tcsetwinsize(fd: BorrowedFd<'_>, winsize: Winsize) -> io::Result<(
 
 #[inline]
 pub(crate) fn tcsetpgrp(fd: BorrowedFd<'_>, pid: Pid) -> io::Result<()> {
-    unsafe { ret(syscall!(__NR_ioctl, fd, c_uint(TIOCSPGRP), pid)) }
+    let raw_pid: c::c_int = pid.as_raw_nonzero().get();
+    unsafe {
+        ret(syscall_readonly!(
+            __NR_ioctl,
+            fd,
+            c_uint(TIOCSPGRP),
+            by_ref(&raw_pid)
+        ))
+    }
 }
 
 /// A wrapper around a conceptual `cfsetspeed` which handles an arbitrary

src/termios/tc.rs

@@ -1,9 +1,10 @@
 use crate::fd::AsFd;
-use crate::pid::Pid;
 #[cfg(not(target_os = "espidf"))]
 use crate::termios::{Action, OptionalActions, QueueSelector, Termios, Winsize};
 use crate::{backend, io};
 
+pub use crate::pid::Pid;
+
 /// `tcgetattr(fd)`—Get terminal attributes.
 ///
 /// Also known as the `TCGETS` (or `TCGETS2` on Linux) operation with `ioctl`.
@@ -44,6 +45,11 @@ pub fn tcgetwinsize<Fd: AsFd>(fd: Fd) -> io::Result<Winsize> {
 ///
 /// Also known as the `TIOCGPGRP` operation with `ioctl`.
 ///
+/// On Linux, if `fd` is a pseudo-terminal, the underlying system call here can
+/// return a pid of 0, which rustix's `Pid` type doesn't support. So rustix
+/// instead handles this case by failing with [`io::Errno::OPNOTSUPP`] if the
+/// pid is 0.
+///
 /// # References
 ///  - [POSIX]
 ///  - [Linux]

tests/termios/main.rs

@@ -4,6 +4,10 @@
 
 #[cfg(not(windows))]
 mod isatty;
+#[cfg(not(any(windows, target_os = "wasi")))]
+mod pgrp;
+#[cfg(not(any(windows, target_os = "wasi")))]
+mod sid;
 #[cfg(all(not(windows), feature = "pty"))]
 mod termios;
 #[cfg(not(any(windows, target_os = "fuchsia")))]

tests/termios/pgrp.rs

@@ -0,0 +1,55 @@
+use rustix::io::Errno;
+use rustix::termios::{tcgetpgrp, tcsetpgrp, Pid};
+use tempfile::tempdir;
+
+#[cfg(feature = "fs")]
+#[test]
+fn pgrp_notty() {
+    let tmpdir = tempdir().unwrap();
+    let fd = rustix::fs::open(
+        tmpdir.path(),
+        rustix::fs::OFlags::RDONLY,
+        rustix::fs::Mode::empty(),
+    )
+    .unwrap();
+
+    // A file is not a tty.
+    assert_eq!(tcgetpgrp(&fd), Err(Errno::NOTTY));
+    assert_eq!(tcsetpgrp(&fd, Pid::INIT), Err(Errno::NOTTY));
+}
+
+// Disable on illumos where `tcgetattr` doesn't appear to support
+// pseudoterminals.
+#[cfg(not(target_os = "illumos"))]
+#[cfg(feature = "pty")]
+#[test]
+fn pgrp_pseudoterminal() {
+    use rustix::pty::*;
+    use rustix::termios::*;
+
+    let pty = match openpt(OpenptFlags::NOCTTY) {
+        Ok(pty) => pty,
+        Err(rustix::io::Errno::NOSYS) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+
+    // Linux's `tcgetpgrp` returns 0 here, which is not documented, so rustix
+    // translates it into `OPNOTSUPP`.
+    #[cfg(linux_kernel)]
+    assert_eq!(tcgetpgrp(&pty), Err(rustix::io::Errno::OPNOTSUPP));
+
+    // FreeBSD's `tcgetpgrp` returns 100000 here, or presumably some other
+    // number if that number is already taken, which is documented behavior,
+    // but impossible to test for reliably.
+    #[cfg(not(linux_kernel))]
+    assert!(matches!(tcgetpgrp(&pty), Ok(_)));
+
+    // We shouldn't be able to set the process group to pid 1.
+    match tcsetpgrp(&pty, rustix::termios::Pid::INIT).unwrap_err() {
+        #[cfg(freebsdlike)]
+        rustix::io::Errno::PERM => {}
+        #[cfg(any(apple, linux_kernel))]
+        rustix::io::Errno::NOTTY => {}
+        err => Err(err).unwrap(),
+    }
+}

tests/termios/sid.rs

@@ -0,0 +1,27 @@
+use rustix::io::Errno;
+use rustix::termios::tcgetsid;
+use tempfile::tempdir;
+
+#[cfg(feature = "fs")]
+#[test]
+fn sid_notty() {
+    let tmpdir = tempdir().unwrap();
+    let fd = rustix::fs::open(
+        tmpdir.path(),
+        rustix::fs::OFlags::RDONLY,
+        rustix::fs::Mode::empty(),
+    )
+    .unwrap();
+
+    // A file is not a tty.
+    assert_eq!(tcgetsid(&fd), Err(Errno::NOTTY));
+}
+
+#[cfg(all(feature = "stdio", feature = "process"))]
+#[test]
+fn sid_match() {
+    match tcgetsid(rustix::stdio::stdin()) {
+        Ok(sid) => assert_eq!(sid, rustix::process::getsid(None).unwrap()),
+        Err(_err) => {}
+    }
+}

tests/termios/termios.rs

@@ -1,3 +1,86 @@
+#[test]
+fn test_termios_flush() {
+    use rustix::pty::*;
+    use rustix::termios::*;
+
+    let pty = match openpt(OpenptFlags::empty()) {
+        Ok(pty) => pty,
+        Err(rustix::io::Errno::NOSYS) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+    let tio = match tcgetattr(&pty) {
+        Ok(tio) => tio,
+        Err(rustix::io::Errno::NOSYS) => return,
+        #[cfg(apple)]
+        Err(rustix::io::Errno::NOTTY) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+    tcsetattr(&pty, OptionalActions::Now, &tio).unwrap();
+
+    tcflush(&pty, QueueSelector::IOFlush).unwrap();
+}
+
+#[test]
+fn test_termios_drain() {
+    use rustix::pty::*;
+    use rustix::termios::*;
+
+    let pty = match openpt(OpenptFlags::empty()) {
+        Ok(pty) => pty,
+        Err(rustix::io::Errno::NOSYS) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+    let tio = match tcgetattr(&pty) {
+        Ok(tio) => tio,
+        Err(rustix::io::Errno::NOSYS) => return,
+        #[cfg(apple)]
+        Err(rustix::io::Errno::NOTTY) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+    tcsetattr(&pty, OptionalActions::Now, &tio).unwrap();
+
+    tcdrain(&pty).unwrap();
+}
+
+#[test]
+fn test_termios_winsize() {
+    use rustix::pty::*;
+    use rustix::termios::*;
+
+    let pty = match openpt(OpenptFlags::empty()) {
+        Ok(pty) => pty,
+        Err(rustix::io::Errno::NOSYS) => return,
+        Err(e) => Err(e).unwrap(),
+    };
+
+    // Sizes for a pseudoterminal start out 0.
+    let mut sizes = match tcgetwinsize(&pty) {
+        Ok(sizes) => sizes,
+        // Apple doesn't appear to support `tcgetwinsize` on a pty.
+        #[cfg(apple)]
+        Err(rustix::io::Errno::NOTTY) => return,
+        Err(err) => Err(err).unwrap(),
+    };
+    assert_eq!(sizes.ws_row, 0);
+    assert_eq!(sizes.ws_col, 0);
+    assert_eq!(sizes.ws_xpixel, 0);
+    assert_eq!(sizes.ws_ypixel, 0);
+
+    // Set some arbitrary sizes.
+    sizes.ws_row = 28;
+    sizes.ws_col = 82;
+    sizes.ws_xpixel = 365;
+    sizes.ws_ypixel = 794;
+    tcsetwinsize(&pty, sizes).unwrap();
+
+    // Check that the sizes roundtripped.
+    let check_sizes = tcgetwinsize(&pty).unwrap();
+    assert_eq!(check_sizes.ws_row, sizes.ws_row);
+    assert_eq!(check_sizes.ws_col, sizes.ws_col);
+    assert_eq!(check_sizes.ws_xpixel, sizes.ws_xpixel);
+    assert_eq!(check_sizes.ws_ypixel, sizes.ws_ypixel);
+}
+
 // Disable on illumos where `tcgetattr` doesn't appear to support
 // pseudoterminals.
 #[cfg(not(target_os = "illumos"))]
@@ -86,3 +169,12 @@ fn test_termios_speeds() {
         assert_eq!(new_tio.output_speed(), speed::B110);
     }
 }
+
+#[test]
+fn test_termios_tcgetattr_not_tty() {
+    let file = tempfile::tempfile().unwrap();
+    assert_eq!(
+        rustix::termios::tcgetattr(&file).unwrap_err(),
+        rustix::io::Errno::NOTTY
+    );
+}