From 9e7d4d719cfdc8d73aef04c68e2f1f929a65a94a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 3 Sep 2024 04:34:09 -0700 Subject: [PATCH 01/16] Try `TCGETS` before trying `TCGETS2`. Switch `tcgetattr`/`tcsetattr` from using `TCGETS2`/`TCSETS2` first to using `TCGETS`/`TCSETS` first. Have `tcgetattr` fall back to `TCGETS2` if the `TCGETS` flags indicate that a custom speed is in used. glibc and musl have not yet migrated to `TCGETS2`/`TCSETS2`, and as a result, seccomp sandboxes and Linux-like environments such as WSL don't always support them. Also, fix some bugs in QEMU related to the handling of termios syscalls. This eliminates the need for having rustix do extra fixups on PowerPC. This is expected to fix crossterm-rs/crossterm#912. --- ci/tcgets2-tcsets2.patch | 87 ++++++++++- src/backend/libc/c.rs | 19 ++- src/backend/libc/termios/syscalls.rs | 172 +++++++++++++--------- src/backend/linux_raw/c.rs | 18 +-- src/backend/linux_raw/termios/syscalls.rs | 167 +++++++++++++-------- src/termios/types.rs | 16 +- tests/termios/termios.rs | 61 +++++--- 7 files changed, 350 insertions(+), 190 deletions(-) diff --git a/ci/tcgets2-tcsets2.patch b/ci/tcgets2-tcsets2.patch index 0fb89cbb6..11fe910b2 100644 --- a/ci/tcgets2-tcsets2.patch +++ b/ci/tcgets2-tcsets2.patch @@ -6,15 +6,22 @@ This implements the `TCGETS2` and `TCSETS2` ioctls. diff -ur a/linux-user/ioctls.h b/linux-user/ioctls.h --- a/linux-user/ioctls.h +++ b/linux-user/ioctls.h -@@ -2,6 +2,8 @@ +@@ -1,5 +1,15 @@ + /* emulated ioctl list */ - IOCTL(TCGETS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_termios))) - IOCTL(TCSETS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios))) ++ /* ++ * Put `TCGETS2`/`TCGETS2` before `TCGETS`/`TCSETS` so that on targets ++ * where these have the same value, we find the `TCGETS2`/`TCSETS2` ++ * entries first, which ensures that we set the `c_ispeed` and `c_ospeed` ++ * fields if the target needed them. ++ */ + IOCTL(TCGETS2, IOC_R, MK_PTR(MK_STRUCT(STRUCT_termios2))) + IOCTL(TCSETS2, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios2))) ++ IOCTL(TCSETSF2, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios2))) ++ IOCTL(TCSETSW2, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios2))) + IOCTL(TCGETS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_termios))) + IOCTL(TCSETS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios))) IOCTL(TCSETSF, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios))) - IOCTL(TCSETSW, IOC_W, MK_PTR(MK_STRUCT(STRUCT_termios))) - IOCTL(TIOCGWINSZ, IOC_R, MK_PTR(MK_STRUCT(STRUCT_winsize))) diff -ur a/linux-user/ppc/termbits.h b/linux-user/ppc/termbits.h --- a/linux-user/ppc/termbits.h +++ b/linux-user/ppc/termbits.h @@ -53,12 +60,14 @@ diff -ur a/linux-user/ppc/termbits.h b/linux-user/ppc/termbits.h #define TARGET_CSIZE 00001400 #define TARGET_CS5 00000000 -@@ -178,6 +192,8 @@ +@@ -178,6 +192,10 @@ #define TARGET_TCSETS TARGET_IOW('t', 20, struct target_termios) #define TARGET_TCSETSW TARGET_IOW('t', 21, struct target_termios) #define TARGET_TCSETSF TARGET_IOW('t', 22, struct target_termios) +#define TARGET_TCGETS2 TARGET_TCGETS +#define TARGET_TCSETS2 TARGET_TCSETS ++#define TARGET_TCSETSW2 TARGET_TCSETSW ++#define TARGET_TCSETSF2 TARGET_TCSETSF #define TARGET_TCGETA TARGET_IOR('t', 23, struct target_termio) #define TARGET_TCSETA TARGET_IOW('t', 24, struct target_termio) @@ -157,6 +166,15 @@ diff -ur a/linux-user/strace.c b/linux-user/strace.c #undef UNUSED +@@ -4161,7 +4161,7 @@ + int target_size; + + for (ie = ioctl_entries; ie->target_cmd != 0; ie++) { +- if (ie->target_cmd == arg1) { ++ if (ie->target_cmd == (int)arg1) { + break; + } + } diff -ur a/linux-user/syscall.c b/linux-user/syscall.c --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -326,3 +344,60 @@ diff -ur a/linux-user/user-internals.h b/linux-user/user-internals.h /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ #ifdef TARGET_ARM +diff -ur -x build -x roms a/linux-user/mips/termbits.h b/linux-user/mips/termbits.h +--- a/linux-user/mips/termbits.h ++++ b/linux-user/mips/termbits.h +@@ -18,6 +18,17 @@ + target_cc_t c_cc[TARGET_NCCS]; /* control characters */ + }; + ++struct target_termios2 { ++ target_tcflag_t c_iflag; /* input mode flags */ ++ target_tcflag_t c_oflag; /* output mode flags */ ++ target_tcflag_t c_cflag; /* control mode flags */ ++ target_tcflag_t c_lflag; /* local mode flags */ ++ target_cc_t c_line; /* line discipline */ ++ target_cc_t c_cc[TARGET_NCCS]; /* control characters */ ++ target_speed_t c_ispeed; /* input speed */ ++ target_speed_t c_ospeed; /* output speed */ ++}; ++ + /* c_iflag bits */ + #define TARGET_IGNBRK 0000001 + #define TARGET_BRKINT 0000002 +@@ -117,6 +128,7 @@ + #define TARGET_B3500000 0010016 + #define TARGET_B4000000 0010017 + #define TARGET_CIBAUD 002003600000 /* input baud rate (not used) */ ++#define TARGET_IBSHIFT 16 + #define TARGET_CMSPAR 010000000000 /* mark or space (stick) parity */ + #define TARGET_CRTSCTS 020000000000 /* flow control */ + +@@ -217,9 +229,9 @@ + #define TARGET_TIOCSETP 0x7409 + #define TARGET_TIOCSETN 0x740a /* TIOCSETP wo flush */ + +-/* #define TARGET_TIOCSETA TARGET_IOW('t', 20, struct termios) set termios struct */ +-/* #define TARGET_TIOCSETAW TARGET_IOW('t', 21, struct termios) drain output, set */ +-/* #define TARGET_TIOCSETAF TARGET_IOW('t', 22, struct termios) drn out, fls in, set */ ++/* #define TARGET_TIOCSETA TARGET_IOW('t', 20, struct target_termios) set termios struct */ ++/* #define TARGET_TIOCSETAW TARGET_IOW('t', 21, struct target_termios) drain output, set */ ++/* #define TARGET_TIOCSETAF TARGET_IOW('t', 22, struct target_termios) drn out, fls in, set */ + /* #define TARGET_TIOCGETD TARGET_IOR('t', 26, int) get line discipline */ + /* #define TARGET_TIOCSETD TARGET_IOW('t', 27, int) set line discipline */ + /* 127-124 compat */ +@@ -227,10 +239,10 @@ + #define TARGET_TIOCSBRK 0x5427 /* BSD compatibility */ + #define TARGET_TIOCCBRK 0x5428 /* BSD compatibility */ + #define TARGET_TIOCGSID 0x7416 /* Return the session ID of FD */ +-#define TARGET_TCGETS2 TARGET_IOR('T', 0x2A, struct termios2) +-#define TARGET_TCSETS2 TARGET_IOW('T', 0x2B, struct termios2) +-#define TARGET_TCSETSW2 TARGET_IOW('T', 0x2C, struct termios2) +-#define TARGET_TCSETSF2 TARGET_IOW('T', 0x2D, struct termios2) ++#define TARGET_TCGETS2 TARGET_IOR('T', 0x2A, struct target_termios2) ++#define TARGET_TCSETS2 TARGET_IOW('T', 0x2B, struct target_termios2) ++#define TARGET_TCSETSW2 TARGET_IOW('T', 0x2C, struct target_termios2) ++#define TARGET_TCSETSF2 TARGET_IOW('T', 0x2D, struct target_termios2) + #define TARGET_TIOCGRS485 TARGET_IOR('T', 0x2E, struct serial_rs485) + #define TARGET_TIOCSRS485 TARGET_IOWR('T', 0x2F, struct serial_rs485) + #define TARGET_TIOCGPTN TARGET_IOR('T',0x30, unsigned int) /* Get Pty Number (of pty-mux device) */ diff --git a/src/backend/libc/c.rs b/src/backend/libc/c.rs index 1a5dbf79d..ec140a327 100644 --- a/src/backend/libc/c.rs +++ b/src/backend/libc/c.rs @@ -102,19 +102,26 @@ pub(crate) const O_LARGEFILE: c_int = 0x2000; pub(crate) const MAP_DROPPABLE: u32 = 0x8; // On PowerPC, the regular `termios` has the `termios2` fields and there is no -// `termios2`. linux-raw-sys has aliases `termios2` to `termios` to cover this -// difference, but we still need to manually import it since `libc` doesn't -// have this. +// `termios2`, so we define aliases. #[cfg(all( linux_kernel, feature = "termios", any(target_arch = "powerpc", target_arch = "powerpc64") ))] -pub(crate) use { - linux_raw_sys::general::{termios2, CIBAUD}, - linux_raw_sys::ioctl::{TCGETS2, TCSETS2, TCSETSF2, TCSETSW2}, +pub(crate) use libc::{ + termios as termios2, TCGETS as TCGETS2, TCSETS as TCSETS2, TCSETSF as TCSETSF2, + TCSETSW as TCSETSW2, }; +// And PowerPC doesn't define `CIBAUD`, but it does define `IBSHIFT`, so we can +// compute `CIBAUD` ourselves. +#[cfg(all( + linux_kernel, + feature = "termios", + any(target_arch = "powerpc", target_arch = "powerpc64") +))] +pub(crate) const CIBAUD: u32 = libc::CBAUD << libc::IBSHIFT; + // Automatically enable “large file” support (LFS) features. #[cfg(target_os = "vxworks")] diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index 9eba5637f..26662862a 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -30,31 +30,69 @@ use { #[cfg(not(any(target_os = "espidf", target_os = "wasi")))] pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { - // If we have `TCGETS2`, use it, so that we fill in the `c_ispeed` and - // `c_ospeed` fields. + // On Linux, use `TCGETS`, and fall back to `TCGETS2` if we need to get the + // `c_ispeed` and `c_ospeed` field values. + // + // SAFETY: This invokes the `TCGETS` and optionally also `TCGETS2` ioctls. + // `TCGETS` doesn't initialize the `input_speed` and `output_speed` fields + // of the result, so in the case where we don't call `TCGETS2`, we infer + // their values from other fields and then manually initialize them (except + // on PowerPC where `TCGETS` does initialize those fields). #[cfg(linux_kernel)] - { + unsafe { use crate::termios::{ControlModes, InputModes, LocalModes, OutputModes, SpecialCodes}; use crate::utils::default_array; - let termios2 = unsafe { - let mut termios2 = MaybeUninit::::uninit(); + let mut termios2 = MaybeUninit::::uninit(); - // QEMU's `TCGETS2` doesn't currently set `input_speed` or - // `output_speed` on PowerPC, so zero out the fields ourselves. - #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] - { - termios2.write(core::mem::zeroed()); - } + // Use the old `TCGETS`. Linux has supported `TCGETS2` since Linux + // 2.6.40, however glibc and musl haven't migrated to it yet, so it + // tends to get left out of seccomp sandboxes. It's also unsupported + // by WSL, and likely enough other things too, so we use the old + // `TCGETS` for compatibility. + ret(c::ioctl( + borrowed_fd(fd), + c::TCGETS as _, + termios2.as_mut_ptr(), + ))?; - ret(c::ioctl( - borrowed_fd(fd), - c::TCGETS2 as _, - termios2.as_mut_ptr(), - ))?; + // If `BOTHER` is set for either the input or output, that means + // there's a custom speed, which means we really do need to use + // `TCGETS2`. Unless we're on PowerPC, where `TCGETS` is the same + // as `TCGETS2`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] + { + use crate::termios::speed; + use core::ptr::{addr_of, addr_of_mut}; + + // At this point, `termios2` is initialized except for the + // `c_ispeed` and `c_ospeed` fields, so we don't form a reference + // yet. + let ptr = termios2.as_mut_ptr(); + + let control_modes = addr_of!((*ptr).c_cflag).read(); + let encoded_out = control_modes & c::CBAUD; + let encoded_in = (control_modes & c::CIBAUD) >> c::IBSHIFT; + if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + ret(c::ioctl(borrowed_fd(fd), c::TCGETS2 as _, ptr))?; + } else { + // Otherwise, set the speeds from other fields. + let output_speed = speed::decode(encoded_out).unwrap(); + addr_of_mut!((*ptr).c_ospeed).write(output_speed); - termios2.assume_init() - }; + // For input speeds, `B0` is special-cased to mean the input + // speed is the same as the output speed. + let input_speed = if encoded_in == c::B0 { + output_speed + } else { + speed::decode(encoded_in).unwrap() + }; + addr_of_mut!((*ptr).c_ispeed).write(input_speed); + } + } + + // Now all the fields are set. + let termios2 = termios2.assume_init(); // Convert from the Linux `termios2` to our `Termios`. let mut result = Termios { @@ -68,32 +106,6 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { output_speed: termios2.c_ospeed, }; - // QEMU's `TCGETS2` doesn't currently set `input_speed` or - // `output_speed` on PowerPC, so set them manually if we can. - #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] - { - use crate::termios::speed; - - if result.output_speed == 0 && (termios2.c_cflag & c::CBAUD) != c::BOTHER { - if let Some(output_speed) = speed::decode(termios2.c_cflag & c::CBAUD) { - result.output_speed = output_speed; - } - } - if result.input_speed == 0 - && ((termios2.c_cflag & c::CIBAUD) >> c::IBSHIFT) != c::BOTHER - { - // For input speeds, `B0` is special-cased to mean the input - // speed is the same as the output speed. - if ((termios2.c_cflag & c::CIBAUD) >> c::IBSHIFT) == c::B0 { - result.input_speed = result.output_speed; - } else if let Some(input_speed) = - speed::decode((termios2.c_cflag & c::CIBAUD) >> c::IBSHIFT) - { - result.input_speed = input_speed; - } - } - } - result.special_codes.0[..termios2.c_cc.len()].copy_from_slice(&termios2.c_cc); Ok(result) @@ -139,41 +151,50 @@ pub(crate) fn tcsetattr( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { - // If we have `TCSETS2`, use it, so that we use the `c_ispeed` and - // `c_ospeed` fields. #[cfg(linux_kernel)] { use crate::termios::speed; use crate::utils::default_array; - use linux_raw_sys::general::{termios2, BOTHER, CBAUD, IBSHIFT}; - - #[cfg(not(any(target_arch = "sparc", target_arch = "sparc64")))] - use linux_raw_sys::ioctl::{TCSETS, TCSETS2}; - - // linux-raw-sys' ioctl-generation script for sparc isn't working yet, - // so as a temporary workaround, declare these manually. - #[cfg(any(target_arch = "sparc", target_arch = "sparc64"))] - const TCSETS: u32 = 0x8024_5409; - #[cfg(any(target_arch = "sparc", target_arch = "sparc64"))] - const TCSETS2: u32 = 0x802c_540d; - - // Translate from `optional_actions` into an ioctl request code. On - // MIPS, `optional_actions` already has `TCGETS` added to it. - let request = TCSETS2 - + if cfg!(any( + + // Similar to `tcgetattr`, use the old `TCSETS`, unless either input + // speed or output speed is custom, then we really have to use + // `TCSETS2`. + let encoded_out = termios.control_modes.bits() & c::CBAUD; + let encoded_in = (termios.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; + let request = if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + // Translate from `optional_actions` into a `TCGETS2` ioctl request + // code. On MIPS, `optional_actions` already has `TCGETS` added to + // it. + c::TCSETS2 as c::c_ulong + + if cfg!(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6" + )) { + optional_actions as c::c_ulong - c::TCSETS as c::c_ulong + } else { + optional_actions as c::c_ulong + } + } else { + // Translate from `optional_actions` into a `TCGETS` ioctl request + // code. On MIPS, `optional_actions` already has `TCGETS` added to + // it. + if cfg!(any( target_arch = "mips", target_arch = "mips32r6", target_arch = "mips64", target_arch = "mips64r6" )) { - optional_actions as u32 - TCSETS + optional_actions as c::c_ulong } else { - optional_actions as u32 - }; + optional_actions as c::c_ulong + c::TCSETS as c::c_ulong + } + }; - let input_speed = termios.input_speed(); let output_speed = termios.output_speed(); - let mut termios2 = termios2 { + let input_speed = termios.input_speed(); + let mut termios2 = c::termios2 { c_iflag: termios.input_modes.bits(), c_oflag: termios.output_modes.bits(), c_cflag: termios.control_modes.bits(), @@ -183,18 +204,25 @@ pub(crate) fn tcsetattr( c_ispeed: input_speed, c_ospeed: output_speed, }; + // Ensure that our input and output speeds are set, as `libc` // routines don't always support setting these separately. - termios2.c_cflag &= !CBAUD; - termios2.c_cflag |= speed::encode(output_speed).unwrap_or(BOTHER); - termios2.c_cflag &= !(CBAUD << IBSHIFT); - termios2.c_cflag |= speed::encode(input_speed).unwrap_or(BOTHER) << IBSHIFT; + termios2.c_cflag &= !c::CBAUD; + termios2.c_cflag |= speed::encode(output_speed).unwrap_or(c::BOTHER); + termios2.c_cflag &= !c::CIBAUD; + termios2.c_cflag |= speed::encode(input_speed).unwrap_or(c::BOTHER) << c::IBSHIFT; let nccs = termios2.c_cc.len(); termios2 .c_cc .copy_from_slice(&termios.special_codes.0[..nccs]); - unsafe { ret(c::ioctl(borrowed_fd(fd), request as _, &termios2)) } + unsafe { + ret(c::ioctl( + borrowed_fd(fd), + request as _, + crate::utils::as_ptr(&termios2), + )) + } } #[cfg(not(linux_kernel))] diff --git a/src/backend/linux_raw/c.rs b/src/backend/linux_raw/c.rs index 70bc46a8c..4035bf945 100644 --- a/src/backend/linux_raw/c.rs +++ b/src/backend/linux_raw/c.rs @@ -190,22 +190,12 @@ pub(crate) use linux_raw_sys::{ VKILL, VLNEXT, VMIN, VQUIT, VREPRINT, VSTART, VSTOP, VSUSP, VSWTC, VT0, VT1, VTDLY, VTIME, VWERASE, XCASE, XTABS, }, - ioctl::{TCGETS2, TCSETS2, TCSETSF2, TCSETSW2, TIOCEXCL, TIOCNXCL}, + ioctl::{ + TCFLSH, TCGETS, TCGETS2, TCSBRK, TCSETS, TCSETS2, TCSETSF2, TCSETSW2, TCXONC, TIOCEXCL, + TIOCGPGRP, TIOCGSID, TIOCGWINSZ, TIOCNXCL, TIOCSPGRP, TIOCSWINSZ, + }, }; -// On MIPS, `TCSANOW` et al have `TCSETS` added to them, so we need it to -// subtract it out. -#[cfg(all( - feature = "termios", - any( - target_arch = "mips", - target_arch = "mips32r6", - target_arch = "mips64", - target_arch = "mips64r6" - ) -))] -pub(crate) use linux_raw_sys::ioctl::TCSETS; - // Define our own `uid_t` and `gid_t` if the kernel's versions are not 32-bit. #[cfg(any(target_arch = "arm", target_arch = "sparc", target_arch = "x86"))] pub(crate) type uid_t = u32; diff --git a/src/backend/linux_raw/termios/syscalls.rs b/src/backend/linux_raw/termios/syscalls.rs index 75eeb5edf..0b98ca17c 100644 --- a/src/backend/linux_raw/termios/syscalls.rs +++ b/src/backend/linux_raw/termios/syscalls.rs @@ -13,70 +13,77 @@ use crate::pid::Pid; #[cfg(all(feature = "alloc", feature = "procfs"))] use crate::procfs; use crate::termios::{ - Action, ControlModes, InputModes, LocalModes, OptionalActions, OutputModes, QueueSelector, - SpecialCodeIndex, Termios, Winsize, + speed, Action, ControlModes, InputModes, LocalModes, OptionalActions, OutputModes, + QueueSelector, SpecialCodeIndex, Termios, Winsize, }; #[cfg(all(feature = "alloc", feature = "procfs"))] use crate::{ffi::CStr, fs::FileType, path::DecInt}; use core::mem::MaybeUninit; -use linux_raw_sys::general::IBSHIFT; -use linux_raw_sys::ioctl::{ - TCFLSH, TCSBRK, TCXONC, TIOCGPGRP, TIOCGSID, TIOCGWINSZ, TIOCSPGRP, TIOCSWINSZ, -}; #[inline] pub(crate) fn tcgetwinsize(fd: BorrowedFd<'_>) -> io::Result { unsafe { let mut result = MaybeUninit::::uninit(); - ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGWINSZ), &mut result))?; + ret(syscall!(__NR_ioctl, fd, c_uint(c::TIOCGWINSZ), &mut result))?; Ok(result.assume_init()) } } #[inline] pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { + // SAFETY: This invokes the `TCGETS` and optionally also `TCGETS2` ioctls. + // `TCGETS` doesn't initialize the `input_speed` and `output_speed` fields + // of the result, so in the case where we don't call `TCGETS2`, we infer + // their values from other fields and then manually initialize them (except + // on PowerPC where `TCGETS` does initialize those fields). unsafe { let mut result = MaybeUninit::::uninit(); - // QEMU's `TCGETS2` doesn't currently set `input_speed` or - // `output_speed` on PowerPC, so zero out the fields ourselves. - #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] + // Use the old `TCGETS`. Linux has supported `TCGETS2` since Linux + // 2.6.40, however glibc and musl haven't migrated to it yet, so it + // tends to get left out of seccomp sandboxes. It's also unsupported + // by WSL, and likely enough other things too, so we use the old + // `TCGETS` for compatibility. + ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS), &mut result))?; + + // If `BOTHER` is set for either the input or output, that means + // there's a custom speed, which means we really do need to use + // `TCGETS2`. Unless we're on PowerPC, where `TCGETS` is the same + // as `TCGETS2`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] { - result.write(core::mem::zeroed()); - } - - ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS2), &mut result))?; - - let result = result.assume_init(); - - // QEMU's `TCGETS2` doesn't currently set `input_speed` or - // `output_speed` on PowerPC, so set them manually if we can. - #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] - let result = { - use crate::termios::speed; - let mut result = result; - if result.output_speed == 0 && (result.control_modes.bits() & c::CBAUD) != c::BOTHER { - if let Some(output_speed) = speed::decode(result.control_modes.bits() & c::CBAUD) { - result.output_speed = output_speed; - } - } - if result.input_speed == 0 - && ((result.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT) != c::BOTHER - { - // For input speeds, `B0` is special-cased to mean the input - // speed is the same as the output speed. - if ((result.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT) == c::B0 { - result.input_speed = result.output_speed; - } else if let Some(input_speed) = - speed::decode((result.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT) - { - result.input_speed = input_speed; - } + use core::ptr::{addr_of, addr_of_mut}; + + // At this point, `result` is initialized except for the + // `input_speed` and `output_speed` fields, so we don't form a + // reference yet. + let ptr = result.as_mut_ptr(); + + let control_modes = addr_of!((*ptr).control_modes).read(); + let encoded_out = control_modes.bits() & c::CBAUD; + let encoded_in = (control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; + if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + // Do a `TCGETS2` ioctl, which will fill in `input_speed` and + // `output_speed`. + ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS2), ptr))?; + } else { + // Infer `output_speed`. + let output_speed = speed::decode(encoded_out).unwrap(); + addr_of_mut!((*ptr).output_speed).write(output_speed); + + // Infer `input_speed`. For input speeds, `B0` is special-cased + // to mean the input speed is the same as the output speed. + let input_speed = if encoded_in == c::B0 { + output_speed + } else { + speed::decode(encoded_in).unwrap() + }; + addr_of_mut!((*ptr).input_speed).write(input_speed); } - result - }; + } - Ok(result) + // Now all the fields are set. + Ok(result.assume_init()) } } @@ -84,7 +91,7 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result { unsafe { let mut result = MaybeUninit::::uninit(); - ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGPGRP), &mut result))?; + ret(syscall!(__NR_ioctl, fd, c_uint(c::TIOCGPGRP), &mut result))?; let pid = result.assume_init(); // This doesn't appear to be documented, but it appears `tcsetpgrp` can @@ -104,19 +111,39 @@ pub(crate) fn tcsetattr( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { - // Translate from `optional_actions` into an ioctl request code. On MIPS, - // `optional_actions` already has `TCGETS` added to it. - let request = linux_raw_sys::ioctl::TCSETS2 - + if cfg!(any( + // Similar to `tcgetattr`, use the old `TCSETS`, unless either input speed + // or output speed is custom, then we really have to use `TCSETS2`. + let encoded_out = termios.control_modes.bits() & c::CBAUD; + let encoded_in = (termios.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; + let request = if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + // Translate from `optional_actions` into a `TCGETS2` ioctl request + // code. On MIPS, `optional_actions` already has `TCGETS` added to it. + c::TCSETS2 + + if cfg!(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6" + )) { + optional_actions as u32 - c::TCSETS + } else { + optional_actions as u32 + } + } else { + // Translate from `optional_actions` into a `TCGETS` ioctl request + // code. On MIPS, `optional_actions` already has `TCGETS` added to it. + if cfg!(any( target_arch = "mips", target_arch = "mips32r6", target_arch = "mips64", target_arch = "mips64r6" )) { - optional_actions as u32 - linux_raw_sys::ioctl::TCSETS - } else { optional_actions as u32 - }; + } else { + optional_actions as u32 + c::TCSETS + } + }; + unsafe { ret(syscall_readonly!( __NR_ioctl, @@ -129,12 +156,26 @@ pub(crate) fn tcsetattr( #[inline] pub(crate) fn tcsendbreak(fd: BorrowedFd<'_>) -> io::Result<()> { - unsafe { ret(syscall_readonly!(__NR_ioctl, fd, c_uint(TCSBRK), c_uint(0))) } + unsafe { + ret(syscall_readonly!( + __NR_ioctl, + fd, + c_uint(c::TCSBRK), + c_uint(0) + )) + } } #[inline] pub(crate) fn tcdrain(fd: BorrowedFd<'_>) -> io::Result<()> { - unsafe { ret(syscall_readonly!(__NR_ioctl, fd, c_uint(TCSBRK), c_uint(1))) } + unsafe { + ret(syscall_readonly!( + __NR_ioctl, + fd, + c_uint(c::TCSBRK), + c_uint(1) + )) + } } #[inline] @@ -143,7 +184,7 @@ pub(crate) fn tcflush(fd: BorrowedFd<'_>, queue_selector: QueueSelector) -> io:: ret(syscall_readonly!( __NR_ioctl, fd, - c_uint(TCFLSH), + c_uint(c::TCFLSH), c_uint(queue_selector as u32) )) } @@ -155,7 +196,7 @@ pub(crate) fn tcflow(fd: BorrowedFd<'_>, action: Action) -> io::Result<()> { ret(syscall_readonly!( __NR_ioctl, fd, - c_uint(TCXONC), + c_uint(c::TCXONC), c_uint(action as u32) )) } @@ -165,7 +206,7 @@ pub(crate) fn tcflow(fd: BorrowedFd<'_>, action: Action) -> io::Result<()> { pub(crate) fn tcgetsid(fd: BorrowedFd<'_>) -> io::Result { unsafe { let mut result = MaybeUninit::::uninit(); - ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGSID), &mut result))?; + ret(syscall!(__NR_ioctl, fd, c_uint(c::TIOCGSID), &mut result))?; let pid = result.assume_init(); Ok(Pid::from_raw_unchecked(pid)) } @@ -177,7 +218,7 @@ pub(crate) fn tcsetwinsize(fd: BorrowedFd<'_>, winsize: Winsize) -> io::Result<( ret(syscall_readonly!( __NR_ioctl, fd, - c_uint(TIOCSWINSZ), + c_uint(c::TIOCSWINSZ), by_ref(&winsize) )) } @@ -190,7 +231,7 @@ pub(crate) fn tcsetpgrp(fd: BorrowedFd<'_>, pid: Pid) -> io::Result<()> { ret(syscall_readonly!( __NR_ioctl, fd, - c_uint(TIOCSPGRP), + c_uint(c::TIOCSPGRP), by_ref(&raw_pid) )) } @@ -200,13 +241,13 @@ pub(crate) fn tcsetpgrp(fd: BorrowedFd<'_>, pid: Pid) -> io::Result<()> { /// integer speed value. #[inline] pub(crate) fn set_speed(termios: &mut Termios, arbitrary_speed: u32) -> io::Result<()> { - let encoded_speed = crate::termios::speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); + let encoded_speed = speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); debug_assert_eq!(encoded_speed & !c::CBAUD, 0); termios.control_modes -= ControlModes::from_bits_retain(c::CBAUD | c::CIBAUD); termios.control_modes |= - ControlModes::from_bits_retain(encoded_speed | (encoded_speed << IBSHIFT)); + ControlModes::from_bits_retain(encoded_speed | (encoded_speed << c::IBSHIFT)); termios.input_speed = arbitrary_speed; termios.output_speed = arbitrary_speed; @@ -218,7 +259,7 @@ pub(crate) fn set_speed(termios: &mut Termios, arbitrary_speed: u32) -> io::Resu /// integer speed value. #[inline] pub(crate) fn set_output_speed(termios: &mut Termios, arbitrary_speed: u32) -> io::Result<()> { - let encoded_speed = crate::termios::speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); + let encoded_speed = speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); debug_assert_eq!(encoded_speed & !c::CBAUD, 0); @@ -234,12 +275,12 @@ pub(crate) fn set_output_speed(termios: &mut Termios, arbitrary_speed: u32) -> i /// integer speed value. #[inline] pub(crate) fn set_input_speed(termios: &mut Termios, arbitrary_speed: u32) -> io::Result<()> { - let encoded_speed = crate::termios::speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); + let encoded_speed = speed::encode(arbitrary_speed).unwrap_or(c::BOTHER); debug_assert_eq!(encoded_speed & !c::CBAUD, 0); termios.control_modes -= ControlModes::from_bits_retain(c::CIBAUD); - termios.control_modes |= ControlModes::from_bits_retain(encoded_speed << IBSHIFT); + termios.control_modes |= ControlModes::from_bits_retain(encoded_speed << c::IBSHIFT); termios.input_speed = arbitrary_speed; diff --git a/src/termios/types.rs b/src/termios/types.rs index 0a45800f4..0772af297 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -810,15 +810,17 @@ pub mod speed { /// `u32`. /// /// On BSD platforms, integer speed values are already the same as their - /// encoded values, and on Linux platforms, we use `TCGETS2`/`TCSETS2` and - /// the `c_ispeed`/`c_ospeed` fields, except that on Linux on PowerPC on - /// QEMU, `TCGETS2`/`TCSETS2` don't set `c_ispeed`/`c_ospeed`. + /// encoded values. + /// + /// On Linux on PowerPC, `TCGETS`/`TCSETS` support the `c_ispeed` and + /// `c_ospeed` fields. + /// + /// On Linux on architectures other than PowerPC, `TCGETS`/`TCSETS` don't + /// support the `c_ispeed` and `c_ospeed` fields, so we have to fall back + /// to `TCGETS2`/`TCSETS2` to support them. #[cfg(not(any( bsd, - all( - linux_kernel, - not(any(target_arch = "powerpc", target_arch = "powerpc64")) - ) + all(linux_kernel, any(target_arch = "powerpc", target_arch = "powerpc64")) )))] pub(crate) const fn decode(encoded_speed: c::speed_t) -> Option { match encoded_speed { diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index 0e4071640..471a7bbae 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -112,9 +112,7 @@ fn test_termios_speeds() { assert_eq!(tio.output_speed(), speed::B50); tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); - #[allow(unused_variables)] let new_tio = tcgetattr(&pty).unwrap(); - assert_eq!(new_tio.input_speed(), speed::B50); assert_eq!(new_tio.output_speed(), speed::B50); @@ -124,9 +122,7 @@ fn test_termios_speeds() { assert_eq!(tio.output_speed(), speed::B134); tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); - #[allow(unused_variables)] let new_tio = tcgetattr(&pty).unwrap(); - assert_eq!(new_tio.input_speed(), speed::B134); assert_eq!(new_tio.output_speed(), speed::B134); @@ -134,22 +130,16 @@ fn test_termios_speeds() { // speeds. #[cfg(any(bsd, linux_kernel))] { - tio.set_input_speed(51).unwrap(); - tio.set_output_speed(51).unwrap(); - assert_eq!(tio.input_speed(), 51); - assert_eq!(tio.output_speed(), 51); - tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); - - #[allow(unused_variables)] - let new_tio = tcgetattr(&pty).unwrap(); - - // QEMU's `TCGETS2` doesn't currently set `input_speed` or - // `output_speed` on PowerPC, so we can't use them to set - // arbitrary speed values. - #[cfg(not(all(linux_kernel, any(target_arch = "powerpc", target_arch = "powerpc64"))))] - { - assert_eq!(new_tio.input_speed(), 51); - assert_eq!(new_tio.output_speed(), 51); + for custom_speed in [51, 31250, libc::B50 as _] { + tio.set_input_speed(custom_speed).unwrap(); + tio.set_output_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), custom_speed); + assert_eq!(tio.output_speed(), custom_speed); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), custom_speed); + assert_eq!(new_tio.output_speed(), custom_speed); } } @@ -162,12 +152,39 @@ fn test_termios_speeds() { assert_eq!(tio.output_speed(), speed::B110); tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); - #[allow(unused_variables)] let new_tio = tcgetattr(&pty).unwrap(); - assert_eq!(new_tio.input_speed(), speed::B75); assert_eq!(new_tio.output_speed(), speed::B110); } + + // These platforms are known to support arbitrary not-pre-defined-by-POSIX + // speeds that also differ between input and output. + #[cfg(any(bsd, linux_kernel))] + { + tio.set_output_speed(speed::B134).unwrap(); + for custom_speed in [51, 31250, libc::B50 as _] { + tio.set_input_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), custom_speed); + assert_eq!(tio.output_speed(), speed::B134); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), custom_speed); + assert_eq!(new_tio.output_speed(), speed::B134); + } + + tio.set_input_speed(speed::B150).unwrap(); + for custom_speed in [51, 31250, libc::B50 as _] { + tio.set_output_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), speed::B150); + assert_eq!(tio.output_speed(), custom_speed); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), speed::B150); + assert_eq!(new_tio.output_speed(), custom_speed); + } + } } #[test] From 4120cf85667d4aa5ba69b9bb67aed3adc6c5153b Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 3 Sep 2024 17:18:45 -0700 Subject: [PATCH 02/16] Extent the speeds test with more speeds. --- tests/termios/termios.rs | 45 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index 471a7bbae..0d8436d55 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -126,11 +126,50 @@ fn test_termios_speeds() { assert_eq!(new_tio.input_speed(), speed::B134); assert_eq!(new_tio.output_speed(), speed::B134); + // Check various speeds. + for custom_speed in [speed::B50, speed::B19200, speed::B38400] { + tio.set_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), custom_speed); + assert_eq!(tio.output_speed(), custom_speed); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), custom_speed); + assert_eq!(new_tio.output_speed(), custom_speed); + } + + // Similar, but using `set_input_speed` and `set_output_speed` instead + // of `set_speed`. + for custom_speed in [speed::B50, speed::B19200, speed::B38400] { + tio.set_input_speed(custom_speed).unwrap(); + tio.set_output_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), custom_speed); + assert_eq!(tio.output_speed(), custom_speed); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), custom_speed); + assert_eq!(new_tio.output_speed(), custom_speed); + } + // These platforms are known to support arbitrary not-pre-defined-by-POSIX // speeds. #[cfg(any(bsd, linux_kernel))] { - for custom_speed in [51, 31250, libc::B50 as _] { + for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { + tio.set_speed(custom_speed).unwrap(); + assert_eq!(tio.input_speed(), custom_speed); + assert_eq!(tio.output_speed(), custom_speed); + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + assert_eq!(new_tio.input_speed(), custom_speed); + assert_eq!(new_tio.output_speed(), custom_speed); + } + + // Similar, but using `set_input_speed` and `set_output_speed` instead + // of `set_speed`. + for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { tio.set_input_speed(custom_speed).unwrap(); tio.set_output_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), custom_speed); @@ -162,7 +201,7 @@ fn test_termios_speeds() { #[cfg(any(bsd, linux_kernel))] { tio.set_output_speed(speed::B134).unwrap(); - for custom_speed in [51, 31250, libc::B50 as _] { + for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { tio.set_input_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), custom_speed); assert_eq!(tio.output_speed(), speed::B134); @@ -174,7 +213,7 @@ fn test_termios_speeds() { } tio.set_input_speed(speed::B150).unwrap(); - for custom_speed in [51, 31250, libc::B50 as _] { + for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { tio.set_output_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), speed::B150); assert_eq!(tio.output_speed(), custom_speed); From 1e72ad47c0b373d7ab4ceb905551c4666a83fdf3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 3 Sep 2024 17:27:27 -0700 Subject: [PATCH 03/16] Use a slower speed which is hopefully portable. --- tests/termios/termios.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index 0d8436d55..20fb19c8e 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -156,7 +156,7 @@ fn test_termios_speeds() { // speeds. #[cfg(any(bsd, linux_kernel))] { - for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { + for custom_speed in [speed::B50, 51, 31250, 74880, 115199] { tio.set_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), custom_speed); assert_eq!(tio.output_speed(), custom_speed); @@ -169,7 +169,7 @@ fn test_termios_speeds() { // Similar, but using `set_input_speed` and `set_output_speed` instead // of `set_speed`. - for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { + for custom_speed in [speed::B50, 51, 31250, 74880, 115199] { tio.set_input_speed(custom_speed).unwrap(); tio.set_output_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), custom_speed); @@ -201,7 +201,7 @@ fn test_termios_speeds() { #[cfg(any(bsd, linux_kernel))] { tio.set_output_speed(speed::B134).unwrap(); - for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { + for custom_speed in [speed::B50, 51, 31250, 74880, 115199] { tio.set_input_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), custom_speed); assert_eq!(tio.output_speed(), speed::B134); @@ -213,7 +213,7 @@ fn test_termios_speeds() { } tio.set_input_speed(speed::B150).unwrap(); - for custom_speed in [speed::B50, 51, 31250, 74880, 256000] { + for custom_speed in [speed::B50, 51, 31250, 74880, 115199] { tio.set_output_speed(custom_speed).unwrap(); assert_eq!(tio.input_speed(), speed::B150); assert_eq!(tio.output_speed(), custom_speed); From 14632adb40bdca677b6d1e8f6b6b239fab8e5b78 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 11:28:22 -0700 Subject: [PATCH 04/16] Switch back to doing `TCGETS2` first. This allows more of the fallback code to be done in `cold` functions, and makes the code easier to follow. --- src/backend/libc/termios/syscalls.rs | 237 +++++++++++++--------- src/backend/linux_raw/termios/syscalls.rs | 182 ++++++++++------- src/termios/tc.rs | 9 + src/termios/types.rs | 38 ++++ tests/termios/termios.rs | 76 +++++++ 5 files changed, 377 insertions(+), 165 deletions(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index 26662862a..e24a5eec8 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -30,69 +30,35 @@ use { #[cfg(not(any(target_os = "espidf", target_os = "wasi")))] pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { - // On Linux, use `TCGETS`, and fall back to `TCGETS2` if we need to get the - // `c_ispeed` and `c_ospeed` field values. - // - // SAFETY: This invokes the `TCGETS` and optionally also `TCGETS2` ioctls. - // `TCGETS` doesn't initialize the `input_speed` and `output_speed` fields - // of the result, so in the case where we don't call `TCGETS2`, we infer - // their values from other fields and then manually initialize them (except - // on PowerPC where `TCGETS` does initialize those fields). + // On Linux, use `TCGETS2`, and fall back to `TCGETS` if needed. #[cfg(linux_kernel)] - unsafe { + { use crate::termios::{ControlModes, InputModes, LocalModes, OutputModes, SpecialCodes}; use crate::utils::default_array; let mut termios2 = MaybeUninit::::uninit(); + let ptr = termios2.as_mut_ptr(); + + // SAFETY: This invokes the `TCGETS2` ioctl, which initializes the full + // `Termios` structure. + let termios2 = unsafe { + match ret(c::ioctl(borrowed_fd(fd), c::TCGETS2 as _, ptr)) { + Ok(()) => {} + + // A `NOTTY` or `ACCESS` might mean the OS doesn't support + // `TCGETS2`, for example a seccomp environment or WSL that + // only knows about `TCGETS`. Fall back to the old `TCGETS`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] + Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => { + tcgetattr_fallback(fd, &mut termios2)? + } - // Use the old `TCGETS`. Linux has supported `TCGETS2` since Linux - // 2.6.40, however glibc and musl haven't migrated to it yet, so it - // tends to get left out of seccomp sandboxes. It's also unsupported - // by WSL, and likely enough other things too, so we use the old - // `TCGETS` for compatibility. - ret(c::ioctl( - borrowed_fd(fd), - c::TCGETS as _, - termios2.as_mut_ptr(), - ))?; - - // If `BOTHER` is set for either the input or output, that means - // there's a custom speed, which means we really do need to use - // `TCGETS2`. Unless we're on PowerPC, where `TCGETS` is the same - // as `TCGETS2`. - #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] - { - use crate::termios::speed; - use core::ptr::{addr_of, addr_of_mut}; - - // At this point, `termios2` is initialized except for the - // `c_ispeed` and `c_ospeed` fields, so we don't form a reference - // yet. - let ptr = termios2.as_mut_ptr(); - - let control_modes = addr_of!((*ptr).c_cflag).read(); - let encoded_out = control_modes & c::CBAUD; - let encoded_in = (control_modes & c::CIBAUD) >> c::IBSHIFT; - if encoded_out == c::BOTHER || encoded_in == c::BOTHER { - ret(c::ioctl(borrowed_fd(fd), c::TCGETS2 as _, ptr))?; - } else { - // Otherwise, set the speeds from other fields. - let output_speed = speed::decode(encoded_out).unwrap(); - addr_of_mut!((*ptr).c_ospeed).write(output_speed); - - // For input speeds, `B0` is special-cased to mean the input - // speed is the same as the output speed. - let input_speed = if encoded_in == c::B0 { - output_speed - } else { - speed::decode(encoded_in).unwrap() - }; - addr_of_mut!((*ptr).c_ispeed).write(input_speed); + Err(err) => return Err(err), } - } - // Now all the fields are set. - let termios2 = termios2.assume_init(); + // Now all the fields are set. + termios2.assume_init() + }; // Convert from the Linux `termios2` to our `Termios`. let mut result = Termios { @@ -123,6 +89,59 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { } } +/// Implement `tcgetattr` using the old `TCGETS` ioctl. +#[cfg(all( + linux_kernel, + not(any(target_arch = "powerpc", target_arch = "powerpc64")) +))] +#[cold] +fn tcgetattr_fallback( + fd: BorrowedFd<'_>, + termios: &mut MaybeUninit, +) -> io::Result<()> { + use crate::termios::speed; + use core::ptr::{addr_of, addr_of_mut}; + + // SAFETY: This invokes the `TCGETS` ioctl, which, if it succeeds, + // initializes the `Termios` structure except for the `input_speed` and + // `output_speed` fields, which we manually initialize before forming a + // reference to the full `Termios`. + unsafe { + let ptr = termios.as_mut_ptr(); + + // Do the old `TCGETS` call, which doesn't initialize `input_speed` or + // `output_speed`. + ret(c::ioctl(borrowed_fd(fd), c::TCGETS as _, ptr))?; + + // Read the `control_modes` field without forming a reference to the + // `Termios` because it isn't fully initialized yet. + let control_modes = addr_of!((*ptr).c_cflag).read(); + + // Infer `output_speed`. + let encoded_out = control_modes & c::CBAUD; + let output_speed = match speed::decode(encoded_out) { + Some(output_speed) => output_speed, + None => return Err(io::Errno::RANGE), + }; + addr_of_mut!((*ptr).c_ospeed).write(output_speed); + + // Infer `input_speed`. For input speeds, `B0` is special-cased to mean + // the input speed is the same as the output speed. + let encoded_in = (control_modes & c::CIBAUD) >> c::IBSHIFT; + let input_speed = if encoded_in == c::B0 { + output_speed + } else { + match speed::decode(encoded_in) { + Some(input_speed) => input_speed, + None => return Err(io::Errno::RANGE), + } + }; + addr_of_mut!((*ptr).c_ispeed).write(input_speed); + } + + Ok(()) +} + #[cfg(not(target_os = "wasi"))] pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result { unsafe { @@ -151,47 +170,12 @@ pub(crate) fn tcsetattr( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { + // On Linux, use `TCSETS2`, and fall back to `TCSETS` if needed. #[cfg(linux_kernel)] { use crate::termios::speed; use crate::utils::default_array; - // Similar to `tcgetattr`, use the old `TCSETS`, unless either input - // speed or output speed is custom, then we really have to use - // `TCSETS2`. - let encoded_out = termios.control_modes.bits() & c::CBAUD; - let encoded_in = (termios.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; - let request = if encoded_out == c::BOTHER || encoded_in == c::BOTHER { - // Translate from `optional_actions` into a `TCGETS2` ioctl request - // code. On MIPS, `optional_actions` already has `TCGETS` added to - // it. - c::TCSETS2 as c::c_ulong - + if cfg!(any( - target_arch = "mips", - target_arch = "mips32r6", - target_arch = "mips64", - target_arch = "mips64r6" - )) { - optional_actions as c::c_ulong - c::TCSETS as c::c_ulong - } else { - optional_actions as c::c_ulong - } - } else { - // Translate from `optional_actions` into a `TCGETS` ioctl request - // code. On MIPS, `optional_actions` already has `TCGETS` added to - // it. - if cfg!(any( - target_arch = "mips", - target_arch = "mips32r6", - target_arch = "mips64", - target_arch = "mips64r6" - )) { - optional_actions as c::c_ulong - } else { - optional_actions as c::c_ulong + c::TCSETS as c::c_ulong - } - }; - let output_speed = termios.output_speed(); let input_speed = termios.input_speed(); let mut termios2 = c::termios2 { @@ -216,12 +200,35 @@ pub(crate) fn tcsetattr( .c_cc .copy_from_slice(&termios.special_codes.0[..nccs]); + // Translate from `optional_actions` into a `TCGETS2` ioctl request code. + // On MIPS, `optional_actions` has `TCGETS` added to it. + let request = c::TCSETS2 as c::c_ulong + + if cfg!(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6" + )) { + optional_actions as c::c_ulong - c::TCSETS as c::c_ulong + } else { + optional_actions as c::c_ulong + }; + + // SAFETY: This invokes the `TCSETS2` ioctl. unsafe { - ret(c::ioctl( - borrowed_fd(fd), - request as _, - crate::utils::as_ptr(&termios2), - )) + match ret(c::ioctl(borrowed_fd(fd), request, &termios2)) { + Ok(()) => Ok(()), + + // Similar to `tcgetattr_fallback`, `NOTTY` or `ACCESS` might + // mean the OS doesn't support `TCSETS2`. Fall back to the old + // `TCGETS`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] + Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => { + tcsetattr_fallback(fd, optional_actions, &termios2) + } + + Err(err) => Err(err), + } } } @@ -235,6 +242,44 @@ pub(crate) fn tcsetattr( } } +/// Implement `tcsetattr` using the old `TCSETS` ioctl. +#[cfg(all( + linux_kernel, + not(any(target_arch = "powerpc", target_arch = "powerpc64")) +))] +#[cold] +pub(crate) fn tcsetattr_fallback( + fd: BorrowedFd<'_>, + optional_actions: OptionalActions, + termios2: &c::termios2, +) -> io::Result<()> { + // `TCSETS` silently accepts a `BOTHER` in the `c_cflag` even though it + // doesn't read the `c_ispeed` or `c_ospeed` fields, so we detect this + // case and fail as needed. + let encoded_out = termios2.c_cflag & c::CBAUD; + let encoded_in = (termios2.c_cflag & c::CIBAUD) >> c::IBSHIFT; + if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + return Err(io::Errno::RANGE); + } + + // Translate from `optional_actions` into a `TCGETS` ioctl request + // code. On MIPS, `optional_actions` already has `TCGETS` added to + // it. + let request = if cfg!(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6" + )) { + optional_actions as c::c_ulong + } else { + optional_actions as c::c_ulong + c::TCSETS as c::c_ulong + }; + + // SAFETY: This invokes the `TCSETS` ioctl. + unsafe { ret(c::ioctl(borrowed_fd(fd), request as _, termios2)) } +} + #[cfg(not(target_os = "wasi"))] pub(crate) fn tcsendbreak(fd: BorrowedFd<'_>) -> io::Result<()> { unsafe { ret(c::tcsendbreak(borrowed_fd(fd), 0)) } diff --git a/src/backend/linux_raw/termios/syscalls.rs b/src/backend/linux_raw/termios/syscalls.rs index 0b98ca17c..f575edbe5 100644 --- a/src/backend/linux_raw/termios/syscalls.rs +++ b/src/backend/linux_raw/termios/syscalls.rs @@ -31,56 +31,65 @@ pub(crate) fn tcgetwinsize(fd: BorrowedFd<'_>) -> io::Result { #[inline] pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { - // SAFETY: This invokes the `TCGETS` and optionally also `TCGETS2` ioctls. - // `TCGETS` doesn't initialize the `input_speed` and `output_speed` fields - // of the result, so in the case where we don't call `TCGETS2`, we infer - // their values from other fields and then manually initialize them (except - // on PowerPC where `TCGETS` does initialize those fields). + let mut result = MaybeUninit::::uninit(); + + // SAFETY: This invokes the `TCGETS2` ioctl, which initializes the full + // `Termios` structure. unsafe { - let mut result = MaybeUninit::::uninit(); + match ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS2), &mut result)) { + Ok(()) => Ok(result.assume_init()), + + // A `NOTTY` or `ACCESS` might mean the OS doesn't support + // `TCGETS2`, for example a seccomp environment or WSL that only + // knows about `TCGETS`. Fall back to the old `TCGETS`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] + Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => tcgetattr_fallback(fd), - // Use the old `TCGETS`. Linux has supported `TCGETS2` since Linux - // 2.6.40, however glibc and musl haven't migrated to it yet, so it - // tends to get left out of seccomp sandboxes. It's also unsupported - // by WSL, and likely enough other things too, so we use the old - // `TCGETS` for compatibility. + Err(err) => Err(err), + } + } +} + +/// Implement `tcgetattr` using the old `TCGETS` ioctl. +#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] +#[cold] +fn tcgetattr_fallback(fd: BorrowedFd<'_>) -> io::Result { + use core::ptr::{addr_of, addr_of_mut}; + + let mut result = MaybeUninit::::uninit(); + + // SAFETY: This invokes the `TCGETS` ioctl which initializes the `Termios` + // structure except for the `input_speed` and `output_speed` fields, which + // we manually initialize before forming a reference to the full `Termios`. + unsafe { + // Do the old `TCGETS` call. ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS), &mut result))?; - // If `BOTHER` is set for either the input or output, that means - // there's a custom speed, which means we really do need to use - // `TCGETS2`. Unless we're on PowerPC, where `TCGETS` is the same - // as `TCGETS2`. - #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] - { - use core::ptr::{addr_of, addr_of_mut}; - - // At this point, `result` is initialized except for the - // `input_speed` and `output_speed` fields, so we don't form a - // reference yet. - let ptr = result.as_mut_ptr(); - - let control_modes = addr_of!((*ptr).control_modes).read(); - let encoded_out = control_modes.bits() & c::CBAUD; - let encoded_in = (control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; - if encoded_out == c::BOTHER || encoded_in == c::BOTHER { - // Do a `TCGETS2` ioctl, which will fill in `input_speed` and - // `output_speed`. - ret(syscall!(__NR_ioctl, fd, c_uint(c::TCGETS2), ptr))?; - } else { - // Infer `output_speed`. - let output_speed = speed::decode(encoded_out).unwrap(); - addr_of_mut!((*ptr).output_speed).write(output_speed); - - // Infer `input_speed`. For input speeds, `B0` is special-cased - // to mean the input speed is the same as the output speed. - let input_speed = if encoded_in == c::B0 { - output_speed - } else { - speed::decode(encoded_in).unwrap() - }; - addr_of_mut!((*ptr).input_speed).write(input_speed); + // Read the `control_modes` field without forming a reference to the + // `Termios` because it isn't fully initialized yet. + let ptr = result.as_mut_ptr(); + let control_modes = addr_of!((*ptr).control_modes).read(); + + // Infer the output speed and set `output_speed`. + let encoded_out = control_modes.bits() & c::CBAUD; + let output_speed = match speed::decode(encoded_out) { + Some(output_speed) => output_speed, + None => return Err(io::Errno::RANGE), + }; + addr_of_mut!((*ptr).output_speed).write(output_speed); + + // Infer the input speed and set `input_speed`. `B0` is a special-case + // that means the input speed is the same as the output speed. + let encoded_in = (control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; + let input_speed = if encoded_in == c::B0 { + output_speed + } else { + match speed::decode(encoded_in) { + Some(input_speed) => input_speed, + None => return Err(io::Errno::RANGE), } - } + }; + addr_of_mut!((*ptr).input_speed).write(input_speed); // Now all the fields are set. Ok(result.assume_init()) @@ -111,39 +120,74 @@ pub(crate) fn tcsetattr( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { - // Similar to `tcgetattr`, use the old `TCSETS`, unless either input speed - // or output speed is custom, then we really have to use `TCSETS2`. - let encoded_out = termios.control_modes.bits() & c::CBAUD; - let encoded_in = (termios.control_modes.bits() & c::CIBAUD) >> c::IBSHIFT; - let request = if encoded_out == c::BOTHER || encoded_in == c::BOTHER { - // Translate from `optional_actions` into a `TCGETS2` ioctl request - // code. On MIPS, `optional_actions` already has `TCGETS` added to it. - c::TCSETS2 - + if cfg!(any( - target_arch = "mips", - target_arch = "mips32r6", - target_arch = "mips64", - target_arch = "mips64r6" - )) { - optional_actions as u32 - c::TCSETS - } else { - optional_actions as u32 - } - } else { - // Translate from `optional_actions` into a `TCGETS` ioctl request - // code. On MIPS, `optional_actions` already has `TCGETS` added to it. - if cfg!(any( + // Translate from `optional_actions` into a `TCGETS2` ioctl request code. + // On MIPS, `optional_actions` has `TCGETS` added to it. + let request = c::TCSETS2 + + if cfg!(any( target_arch = "mips", target_arch = "mips32r6", target_arch = "mips64", target_arch = "mips64r6" )) { - optional_actions as u32 + optional_actions as u32 - c::TCSETS } else { - optional_actions as u32 + c::TCSETS + optional_actions as u32 + }; + + // SAFETY: This invokes the `TCSETS2` ioctl. + unsafe { + match ret(syscall_readonly!( + __NR_ioctl, + fd, + c_uint(request), + by_ref(termios) + )) { + Ok(()) => Ok(()), + + // Similar to `tcgetattr_fallback`, `NOTTY` or `ACCESS` might mean + // the OS doesn't support `TCSETS2`. Fall back to the old `TCGETS`. + #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] + Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => { + tcsetattr_fallback(fd, optional_actions, termios) + } + + Err(err) => Err(err), } + } +} + +/// Implement `tcsetattr` using the old `TCSETS` ioctl. +#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] +#[cold] +fn tcsetattr_fallback( + fd: BorrowedFd<'_>, + optional_actions: OptionalActions, + termios: &Termios, +) -> io::Result<()> { + // `TCSETS` silently accepts a `BOTHER` in the `c_cflag` even though it + // doesn't read the `c_ispeed` or `c_ospeed` fields, so we detect this + // case and fail as needed. + let control_modes_bits = termios.control_modes.bits(); + let encoded_out = control_modes_bits & c::CBAUD; + let encoded_in = (control_modes_bits & c::CIBAUD) >> c::IBSHIFT; + if encoded_out == c::BOTHER || encoded_in == c::BOTHER { + return Err(io::Errno::RANGE); + } + + // Translate from `optional_actions` into a `TCSETS` ioctl request + // code. On MIPS, `optional_actions` already has `TCSETS` added to it. + let request = if cfg!(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6" + )) { + optional_actions as u32 + } else { + optional_actions as u32 + c::TCSETS }; + // SAFETY: This invokes the `TCSETS` ioctl. unsafe { ret(syscall_readonly!( __NR_ioctl, diff --git a/src/termios/tc.rs b/src/termios/tc.rs index 0f828448d..2ef171f2a 100644 --- a/src/termios/tc.rs +++ b/src/termios/tc.rs @@ -9,6 +9,11 @@ pub use crate::pid::Pid; /// /// Also known as the `TCGETS` (or `TCGETS2` on Linux) operation with `ioctl`. /// +/// On Linux, this uses `TCGETS2`. If that fails in a way that indicates that +/// the host doesn't support it, this falls back to the old `TCGETS`, manually +/// initializes the fields that `TCGETS` doesn't initialize, and fails with +/// `io::Errno::RANGE` if the input or output speeds cannot be supported. +/// /// # References /// - [POSIX `tcgetattr`] /// - [Linux `ioctl_tty`] @@ -84,6 +89,10 @@ pub fn tcsetpgrp(fd: Fd, pid: Pid) -> io::Result<()> { /// /// Also known as the `TCSETS` (or `TCSETS2` on Linux) operation with `ioctl`. /// +/// On Linux, this uses `TCSETS2`. If that fails in a way that indicates that +/// the host doesn't support it, this falls back to the old `TCSETS`, and fails +/// with `io::Errno::RANGE` if the input or output speeds cannot be supported. +/// /// # References /// - [POSIX `tcsetattr`] /// - [Linux `ioctl_tty`] diff --git a/src/termios/types.rs b/src/termios/types.rs index 0772af297..5b998abce 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -1115,6 +1115,7 @@ impl core::ops::IndexMut for SpecialCodes { } /// Indices for use with [`Termios::special_codes`]. +#[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct SpecialCodeIndex(usize); #[rustfmt::skip] @@ -1183,6 +1184,43 @@ impl SpecialCodeIndex { pub const VEOL2: Self = Self(c::VEOL2 as usize); } +impl core::fmt::Debug for SpecialCodeIndex { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match *self { + Self::VINTR => write!(f, "VINTR"), + Self::VQUIT => write!(f, "VQUIT"), + Self::VERASE => write!(f, "VERASE"), + Self::VKILL => write!(f, "VKILL"), + Self::VEOF => write!(f, "VEOF"), + Self::VTIME => write!(f, "VTIME"), + Self::VMIN => write!(f, "VMIN"), + #[cfg(not(any( + bsd, + solarish, + target_os = "aix", + target_os = "haiku", + target_os = "hurd", + target_os = "nto", + )))] + Self::VSWTC => write!(f, "VSWTC"), + Self::VSTART => write!(f, "VSTART"), + Self::VSTOP => write!(f, "VSTOP"), + Self::VSUSP => write!(f, "VSUSP"), + Self::VEOL => write!(f, "VEOL"), + #[cfg(not(target_os = "haiku"))] + Self::VREPRINT => write!(f, "VREPRINT"), + #[cfg(not(any(target_os = "aix", target_os = "haiku")))] + Self::VDISCARD => write!(f, "VDISCARD"), + #[cfg(not(any(target_os = "aix", target_os = "haiku")))] + Self::VWERASE => write!(f, "VWERASE"), + #[cfg(not(target_os = "haiku"))] + Self::VLNEXT => write!(f, "VLNEXT"), + Self::VEOL2 => write!(f, "VEOL2"), + _ => write!(f, "unknown"), + } + } +} + /// `TCSA*` values for use with [`tcsetattr`]. /// /// [`tcsetattr`]: crate::termios::tcsetattr diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index 20fb19c8e..b50fa9ff8 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -81,6 +81,82 @@ fn test_termios_winsize() { 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"))] +#[test] +fn test_termios_modes() { + use rustix::pty::*; + use rustix::termios::*; + + let pty = match openpt(OpenptFlags::empty()) { + Ok(pty) => pty, + Err(rustix::io::Errno::NOSYS) => return, + Err(err) => panic!("{:?}", err), + }; + let mut tio = match tcgetattr(&pty) { + Ok(tio) => tio, + Err(rustix::io::Errno::NOSYS) => return, + #[cfg(apple)] + Err(rustix::io::Errno::NOTTY) => return, + Err(err) => panic!("{:?}", err), + }; + + assert!(!tio.local_modes.contains(LocalModes::TOSTOP)); + assert!(!tio.output_modes.contains(OutputModes::OFILL)); + assert!(!tio.input_modes.contains(InputModes::IGNBRK)); + assert!(!tio.control_modes.contains(ControlModes::CRTSCTS)); + + tio.local_modes.insert(LocalModes::TOSTOP); + tio.output_modes.insert(OutputModes::OFILL); + tio.input_modes.insert(InputModes::IGNBRK); + tio.control_modes.insert(ControlModes::CRTSCTS); + + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + + assert!(new_tio.local_modes.contains(LocalModes::TOSTOP)); + assert!(new_tio.output_modes.contains(OutputModes::OFILL)); + assert!(new_tio.input_modes.contains(InputModes::IGNBRK)); + assert!(new_tio.control_modes.contains(ControlModes::CRTSCTS)); +} + +// Disable on illumos where `tcgetattr` doesn't appear to support +// pseudoterminals. +#[cfg(not(target_os = "illumos"))] +#[test] +fn test_termios_special_codes() { + use rustix::pty::*; + use rustix::termios::*; + + let pty = match openpt(OpenptFlags::empty()) { + Ok(pty) => pty, + Err(rustix::io::Errno::NOSYS) => return, + Err(err) => panic!("{:?}", err), + }; + let mut tio = match tcgetattr(&pty) { + Ok(tio) => tio, + Err(rustix::io::Errno::NOSYS) => return, + #[cfg(apple)] + Err(rustix::io::Errno::NOTTY) => return, + Err(err) => panic!("{:?}", err), + }; + + assert_eq!(tio.special_codes[SpecialCodeIndex::VINTR], 3); + assert_eq!(tio.special_codes[SpecialCodeIndex::VEOL], 0); + + tio.special_codes[SpecialCodeIndex::VINTR] = 47; + tio.special_codes[SpecialCodeIndex::VEOL] = 99; + + tcsetattr(&pty, OptionalActions::Now, &tio).unwrap(); + + let new_tio = tcgetattr(&pty).unwrap(); + + assert_eq!(new_tio.special_codes[SpecialCodeIndex::VINTR], 47); + assert_eq!(new_tio.special_codes[SpecialCodeIndex::VEOL], 99); +} + // Disable on illumos where `tcgetattr` doesn't appear to support // pseudoterminals. #[cfg(not(target_os = "illumos"))] From b8a0b49d6c89a6a06bb30edae8eaa2eb58c22bc3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 17:22:53 -0700 Subject: [PATCH 05/16] Fix a portability issue in the new test. --- tests/termios/termios.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index b50fa9ff8..23067a506 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -103,12 +103,12 @@ fn test_termios_modes() { }; assert!(!tio.local_modes.contains(LocalModes::TOSTOP)); - assert!(!tio.output_modes.contains(OutputModes::OFILL)); + assert!(!tio.output_modes.contains(OutputModes::ONOCR)); assert!(!tio.input_modes.contains(InputModes::IGNBRK)); assert!(!tio.control_modes.contains(ControlModes::CRTSCTS)); tio.local_modes.insert(LocalModes::TOSTOP); - tio.output_modes.insert(OutputModes::OFILL); + tio.output_modes.insert(OutputModes::ONOCR); tio.input_modes.insert(InputModes::IGNBRK); tio.control_modes.insert(ControlModes::CRTSCTS); @@ -117,7 +117,7 @@ fn test_termios_modes() { let new_tio = tcgetattr(&pty).unwrap(); assert!(new_tio.local_modes.contains(LocalModes::TOSTOP)); - assert!(new_tio.output_modes.contains(OutputModes::OFILL)); + assert!(new_tio.output_modes.contains(OutputModes::ONOCR)); assert!(new_tio.input_modes.contains(InputModes::IGNBRK)); assert!(new_tio.control_modes.contains(ControlModes::CRTSCTS)); } From 90a5c6670311891503a74840238f13980bff5768 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 17:25:36 -0700 Subject: [PATCH 06/16] Fix compilation. --- src/backend/libc/termios/syscalls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index e24a5eec8..e435d0cdd 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -216,7 +216,7 @@ pub(crate) fn tcsetattr( // SAFETY: This invokes the `TCSETS2` ioctl. unsafe { - match ret(c::ioctl(borrowed_fd(fd), request, &termios2)) { + match ret(c::ioctl(borrowed_fd(fd), request as _, &termios2)) { Ok(()) => Ok(()), // Similar to `tcgetattr_fallback`, `NOTTY` or `ACCESS` might From fb4d3eef30c378346891cd5a64a3cff8702984fc Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 17:58:46 -0700 Subject: [PATCH 07/16] Fix compilation on solarish. --- src/termios/types.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/termios/types.rs b/src/termios/types.rs index 5b998abce..3b211b831 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -1191,9 +1191,19 @@ impl core::fmt::Debug for SpecialCodeIndex { Self::VQUIT => write!(f, "VQUIT"), Self::VERASE => write!(f, "VERASE"), Self::VKILL => write!(f, "VKILL"), + #[cfg(not(solarish))] Self::VEOF => write!(f, "VEOF"), Self::VTIME => write!(f, "VTIME"), + #[cfg(not(solarish))] Self::VMIN => write!(f, "VMIN"), + + // On Solarish platforms, `VMIN` and `VTIME` have the same value + // as `VEOF` and `VEOL`. + #[cfg(solarish)] + Self::VMIN => write!(f, "VMIN/VEOF"), + #[cfg(solarish)] + Self::VTIME => write!(f, "VTIME/VEOL"), + #[cfg(not(any( bsd, solarish, @@ -1206,6 +1216,7 @@ impl core::fmt::Debug for SpecialCodeIndex { Self::VSTART => write!(f, "VSTART"), Self::VSTOP => write!(f, "VSTOP"), Self::VSUSP => write!(f, "VSUSP"), + #[cfg(not(solarish))] Self::VEOL => write!(f, "VEOL"), #[cfg(not(target_os = "haiku"))] Self::VREPRINT => write!(f, "VREPRINT"), From 7c6d313c84024d3145093b1235b67854c1fee9f1 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 18:58:28 -0700 Subject: [PATCH 08/16] Fix compilation on solarish. --- src/termios/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/termios/types.rs b/src/termios/types.rs index 3b211b831..902002ea4 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -1193,6 +1193,7 @@ impl core::fmt::Debug for SpecialCodeIndex { Self::VKILL => write!(f, "VKILL"), #[cfg(not(solarish))] Self::VEOF => write!(f, "VEOF"), + #[cfg(not(solarish))] Self::VTIME => write!(f, "VTIME"), #[cfg(not(solarish))] Self::VMIN => write!(f, "VMIN"), From c4ef05fd6dd1c627530bf8306aa7f0628b8ec039 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 19:38:05 -0700 Subject: [PATCH 09/16] Fix compilation on Linux on SPARC. Also fix capitalization in various comments. --- src/backend/linux_raw/arch/mips.rs | 2 +- src/backend/linux_raw/arch/mips32r6.rs | 2 +- src/backend/linux_raw/arch/mips64.rs | 2 +- src/backend/linux_raw/arch/mips64r6.rs | 2 +- src/fs/ioctl.rs | 2 +- src/termios/types.rs | 36 +++++++++++++++++++------- tests/fs/ioctl.rs | 2 +- 7 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/backend/linux_raw/arch/mips.rs b/src/backend/linux_raw/arch/mips.rs index 37932e02b..3bd96fa00 100644 --- a/src/backend/linux_raw/arch/mips.rs +++ b/src/backend/linux_raw/arch/mips.rs @@ -3,7 +3,7 @@ //! On mipsel, Linux indicates success or failure using `$a3` rather //! than by returning a negative error code as most other architectures do. //! -//! Mips-family platforms have a special calling convention for `__NR_pipe`, +//! MIPS-family platforms have a special calling convention for `__NR_pipe`, //! however we use `__NR_pipe2` instead to avoid having to implement it. use crate::backend::reg::{ diff --git a/src/backend/linux_raw/arch/mips32r6.rs b/src/backend/linux_raw/arch/mips32r6.rs index c2d92447b..2b5b9076e 100644 --- a/src/backend/linux_raw/arch/mips32r6.rs +++ b/src/backend/linux_raw/arch/mips32r6.rs @@ -3,7 +3,7 @@ //! On mipsisa32r6el, Linux indicates success or failure using `$a3` rather //! than by returning a negative error code as most other architectures do. //! -//! Mips-family platforms have a special calling convention for `__NR_pipe`, +//! MIPS-family platforms have a special calling convention for `__NR_pipe`, //! however we use `__NR_pipe2` instead to avoid having to implement it. use crate::backend::reg::{ diff --git a/src/backend/linux_raw/arch/mips64.rs b/src/backend/linux_raw/arch/mips64.rs index 244daf3ef..5937344ab 100644 --- a/src/backend/linux_raw/arch/mips64.rs +++ b/src/backend/linux_raw/arch/mips64.rs @@ -3,7 +3,7 @@ //! On mips64el, Linux indicates success or failure using `$a3` (`$7`) rather //! than by returning a negative error code as most other architectures do. //! -//! Mips-family platforms have a special calling convention for `__NR_pipe`, +//! MIPS-family platforms have a special calling convention for `__NR_pipe`, //! however we use `__NR_pipe2` instead to avoid having to implement it. use crate::backend::reg::{ diff --git a/src/backend/linux_raw/arch/mips64r6.rs b/src/backend/linux_raw/arch/mips64r6.rs index 8c06d9ee9..d7284a239 100644 --- a/src/backend/linux_raw/arch/mips64r6.rs +++ b/src/backend/linux_raw/arch/mips64r6.rs @@ -4,7 +4,7 @@ //! rather than by returning a negative error code as most other architectures //! do. //! -//! Mips-family platforms have a special calling convention for `__NR_pipe`, +//! MIPS-family platforms have a special calling convention for `__NR_pipe`, //! however we use `__NR_pipe2` instead to avoid having to implement it. //! //! Note that MIPS R6 inline assembly currently doesn't differ from MIPS, diff --git a/src/fs/ioctl.rs b/src/fs/ioctl.rs index b44bd174e..718a2a404 100644 --- a/src/fs/ioctl.rs +++ b/src/fs/ioctl.rs @@ -44,7 +44,7 @@ pub fn ioctl_blkpbszget(fd: Fd) -> io::Result { /// `ioctl(fd, FICLONE, src_fd)`—Share data between open files. /// -/// This ioctl is not available on Sparc platforms. +/// This ioctl is not available on SPARC platforms. /// /// # References /// - [Linux] diff --git a/src/termios/types.rs b/src/termios/types.rs index 902002ea4..e92943778 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -1191,18 +1191,33 @@ impl core::fmt::Debug for SpecialCodeIndex { Self::VQUIT => write!(f, "VQUIT"), Self::VERASE => write!(f, "VERASE"), Self::VKILL => write!(f, "VKILL"), - #[cfg(not(solarish))] + #[cfg(not(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + )))] Self::VEOF => write!(f, "VEOF"), - #[cfg(not(solarish))] + #[cfg(not(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + )))] Self::VTIME => write!(f, "VTIME"), - #[cfg(not(solarish))] + #[cfg(not(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + )))] Self::VMIN => write!(f, "VMIN"), - // On Solarish platforms, `VMIN` and `VTIME` have the same value - // as `VEOF` and `VEOL`. - #[cfg(solarish)] + // On Solarish platforms, and Linux on SPARC, `VMIN` and `VTIME` + // have the same value as `VEOF` and `VEOL`. + #[cfg(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + ))] Self::VMIN => write!(f, "VMIN/VEOF"), - #[cfg(solarish)] + #[cfg(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + ))] Self::VTIME => write!(f, "VTIME/VEOL"), #[cfg(not(any( @@ -1217,7 +1232,10 @@ impl core::fmt::Debug for SpecialCodeIndex { Self::VSTART => write!(f, "VSTART"), Self::VSTOP => write!(f, "VSTOP"), Self::VSUSP => write!(f, "VSUSP"), - #[cfg(not(solarish))] + #[cfg(not(any( + solarish, + all(linux_kernel, any(target_arch = "sparc", target_arch = "sparc64")) + )))] Self::VEOL => write!(f, "VEOL"), #[cfg(not(target_os = "haiku"))] Self::VREPRINT => write!(f, "VREPRINT"), @@ -1344,7 +1362,7 @@ fn termios_layouts() { #[cfg(not(linux_raw))] { - // On Mips, Sparc, and Android, the libc lacks the ospeed and ispeed + // On MIPS, SPARC, and Android, the libc lacks the ospeed and ispeed // fields. #[cfg(all( not(all( diff --git a/tests/fs/ioctl.rs b/tests/fs/ioctl.rs index e95ea2750..d84d8eb0f 100644 --- a/tests/fs/ioctl.rs +++ b/tests/fs/ioctl.rs @@ -1,4 +1,4 @@ -// Sparc lacks `FICLONE`. +// SPARC lacks `FICLONE`. #[cfg(all(linux_kernel, not(any(target_arch = "sparc", target_arch = "sparc64"))))] #[test] fn test_ioctl_ficlone() { From 18f45a5e3353719e32db6cd998a80505e6677198 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 21:49:03 -0700 Subject: [PATCH 10/16] Fix test expectations on FreeBSD. --- tests/termios/termios.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/termios/termios.rs b/tests/termios/termios.rs index 23067a506..3492009c6 100644 --- a/tests/termios/termios.rs +++ b/tests/termios/termios.rs @@ -143,7 +143,13 @@ fn test_termios_special_codes() { Err(err) => panic!("{:?}", err), }; + // Check some initial values of `special_codes`. On Linux, `VINTR`'s code + // is set to ETX. On BSD's, it appears to be set to zero for + // pseudo-terminals. + #[cfg(linux_kernel)] assert_eq!(tio.special_codes[SpecialCodeIndex::VINTR], 3); + #[cfg(bsd)] + assert_eq!(tio.special_codes[SpecialCodeIndex::VINTR], 0); assert_eq!(tio.special_codes[SpecialCodeIndex::VEOL], 0); tio.special_codes[SpecialCodeIndex::VINTR] = 47; From 8decc616c5438c0ee94ff2da49af18d7eb439470 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 4 Sep 2024 21:50:52 -0700 Subject: [PATCH 11/16] Define more `SpecialCodeIndex` values for bsd and solarish. --- src/termios/types.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/termios/types.rs b/src/termios/types.rs index e92943778..7b97808a1 100644 --- a/src/termios/types.rs +++ b/src/termios/types.rs @@ -1182,6 +1182,22 @@ impl SpecialCodeIndex { /// `VEOL2` pub const VEOL2: Self = Self(c::VEOL2 as usize); + + /// `VSWTCH` + #[cfg(any(solarish, target_os = "haiku", target_os = "nto"))] + pub const VSWTCH: Self = Self(c::VSWTCH as usize); + + /// `VDSUSP` + #[cfg(any(bsd, solarish, target_os = "aix", target_os = "hurd", target_os = "nto"))] + pub const VDSUSP: Self = Self(c::VDSUSP as usize); + + /// `VSTATUS` + #[cfg(any(bsd, solarish, target_os = "hurd"))] + pub const VSTATUS: Self = Self(c::VSTATUS as usize); + + /// `VERASE2` + #[cfg(any(freebsdlike, solarish))] + pub const VERASE2: Self = Self(c::VERASE2 as usize); } impl core::fmt::Debug for SpecialCodeIndex { @@ -1246,6 +1262,21 @@ impl core::fmt::Debug for SpecialCodeIndex { #[cfg(not(target_os = "haiku"))] Self::VLNEXT => write!(f, "VLNEXT"), Self::VEOL2 => write!(f, "VEOL2"), + #[cfg(any(solarish, target_os = "haiku", target_os = "nto"))] + Self::VSWTCH => write!(f, "VSWTCH"), + #[cfg(any( + bsd, + solarish, + target_os = "aix", + target_os = "hurd", + target_os = "nto" + ))] + Self::VDSUSP => write!(f, "VDSUSP"), + #[cfg(any(bsd, solarish, target_os = "hurd"))] + Self::VSTATUS => write!(f, "VSTATUS"), + #[cfg(any(freebsdlike, solarish))] + Self::VERASE2 => write!(f, "VERASE2"), + _ => write!(f, "unknown"), } } From 3978919bf09c36b68c00d2f6b8dbbc6aef9943a3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 5 Sep 2024 04:33:12 -0700 Subject: [PATCH 12/16] Fix some copy+pastos in comments. --- src/backend/libc/termios/syscalls.rs | 10 +++++----- src/backend/linux_raw/termios/syscalls.rs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index e435d0cdd..44520f3d5 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -200,8 +200,8 @@ pub(crate) fn tcsetattr( .c_cc .copy_from_slice(&termios.special_codes.0[..nccs]); - // Translate from `optional_actions` into a `TCGETS2` ioctl request code. - // On MIPS, `optional_actions` has `TCGETS` added to it. + // Translate from `optional_actions` into a `TCSETS2` ioctl request code. + // On MIPS, `optional_actions` has `TCSETS` added to it. let request = c::TCSETS2 as c::c_ulong + if cfg!(any( target_arch = "mips", @@ -221,7 +221,7 @@ pub(crate) fn tcsetattr( // Similar to `tcgetattr_fallback`, `NOTTY` or `ACCESS` might // mean the OS doesn't support `TCSETS2`. Fall back to the old - // `TCGETS`. + // `TCSETS`. #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => { tcsetattr_fallback(fd, optional_actions, &termios2) @@ -262,8 +262,8 @@ pub(crate) fn tcsetattr_fallback( return Err(io::Errno::RANGE); } - // Translate from `optional_actions` into a `TCGETS` ioctl request - // code. On MIPS, `optional_actions` already has `TCGETS` added to + // Translate from `optional_actions` into a `TCSETS` ioctl request + // code. On MIPS, `optional_actions` already has `TCSETS` added to // it. let request = if cfg!(any( target_arch = "mips", diff --git a/src/backend/linux_raw/termios/syscalls.rs b/src/backend/linux_raw/termios/syscalls.rs index f575edbe5..fa6569963 100644 --- a/src/backend/linux_raw/termios/syscalls.rs +++ b/src/backend/linux_raw/termios/syscalls.rs @@ -120,8 +120,8 @@ pub(crate) fn tcsetattr( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { - // Translate from `optional_actions` into a `TCGETS2` ioctl request code. - // On MIPS, `optional_actions` has `TCGETS` added to it. + // Translate from `optional_actions` into a `TCSETS2` ioctl request code. + // On MIPS, `optional_actions` has `TCSETS` added to it. let request = c::TCSETS2 + if cfg!(any( target_arch = "mips", @@ -145,7 +145,7 @@ pub(crate) fn tcsetattr( Ok(()) => Ok(()), // Similar to `tcgetattr_fallback`, `NOTTY` or `ACCESS` might mean - // the OS doesn't support `TCSETS2`. Fall back to the old `TCGETS`. + // the OS doesn't support `TCSETS2`. Fall back to the old `TCSETS`. #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] Err(io::Errno::NOTTY) | Err(io::Errno::ACCESS) => { tcsetattr_fallback(fd, optional_actions, termios) From 9ff485c8c77eed720448bb14df8da76d0097ae0c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 5 Sep 2024 04:45:25 -0700 Subject: [PATCH 13/16] Comment formatting fixes. --- src/backend/libc/termios/syscalls.rs | 14 ++++++-------- src/backend/linux_raw/termios/syscalls.rs | 9 ++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index 44520f3d5..0316256a5 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -200,8 +200,8 @@ pub(crate) fn tcsetattr( .c_cc .copy_from_slice(&termios.special_codes.0[..nccs]); - // Translate from `optional_actions` into a `TCSETS2` ioctl request code. - // On MIPS, `optional_actions` has `TCSETS` added to it. + // Translate from `optional_actions` into a `TCSETS2` ioctl request + // code. On MIPS, `optional_actions` has `TCSETS` added to it. let request = c::TCSETS2 as c::c_ulong + if cfg!(any( target_arch = "mips", @@ -253,18 +253,16 @@ pub(crate) fn tcsetattr_fallback( optional_actions: OptionalActions, termios2: &c::termios2, ) -> io::Result<()> { - // `TCSETS` silently accepts a `BOTHER` in the `c_cflag` even though it - // doesn't read the `c_ispeed` or `c_ospeed` fields, so we detect this - // case and fail as needed. + // `TCSETS` silently accepts `BOTHER` in `c_cflag` even though it doesn't + // read `c_ispeed`/`c_ospeed`, so detect this case and fail if needed. let encoded_out = termios2.c_cflag & c::CBAUD; let encoded_in = (termios2.c_cflag & c::CIBAUD) >> c::IBSHIFT; if encoded_out == c::BOTHER || encoded_in == c::BOTHER { return Err(io::Errno::RANGE); } - // Translate from `optional_actions` into a `TCSETS` ioctl request - // code. On MIPS, `optional_actions` already has `TCSETS` added to - // it. + // Translate from `optional_actions` into a `TCSETS` ioctl request code. On + // MIPS, `optional_actions` already has `TCSETS` added to it. let request = if cfg!(any( target_arch = "mips", target_arch = "mips32r6", diff --git a/src/backend/linux_raw/termios/syscalls.rs b/src/backend/linux_raw/termios/syscalls.rs index fa6569963..cd0b5be70 100644 --- a/src/backend/linux_raw/termios/syscalls.rs +++ b/src/backend/linux_raw/termios/syscalls.rs @@ -164,9 +164,8 @@ fn tcsetattr_fallback( optional_actions: OptionalActions, termios: &Termios, ) -> io::Result<()> { - // `TCSETS` silently accepts a `BOTHER` in the `c_cflag` even though it - // doesn't read the `c_ispeed` or `c_ospeed` fields, so we detect this - // case and fail as needed. + // `TCSETS` silently accepts `BOTHER` in `c_cflag` even though it doesn't + // read `c_ispeed`/`c_ospeed`, so detect this case and fail if needed. let control_modes_bits = termios.control_modes.bits(); let encoded_out = control_modes_bits & c::CBAUD; let encoded_in = (control_modes_bits & c::CIBAUD) >> c::IBSHIFT; @@ -174,8 +173,8 @@ fn tcsetattr_fallback( return Err(io::Errno::RANGE); } - // Translate from `optional_actions` into a `TCSETS` ioctl request - // code. On MIPS, `optional_actions` already has `TCSETS` added to it. + // Translate from `optional_actions` into a `TCSETS` ioctl request code. On + // MIPS, `optional_actions` already has `TCSETS` added to it. let request = if cfg!(any( target_arch = "mips", target_arch = "mips32r6", From 05c2fd9b1b347a7c518a3e9c7b1cb28b59c5c222 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 5 Sep 2024 04:47:05 -0700 Subject: [PATCH 14/16] The fallback functions don't need to be `pub(crate)`. --- src/backend/libc/termios/syscalls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index 0316256a5..e036f8349 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -248,7 +248,7 @@ pub(crate) fn tcsetattr( not(any(target_arch = "powerpc", target_arch = "powerpc64")) ))] #[cold] -pub(crate) fn tcsetattr_fallback( +fn tcsetattr_fallback( fd: BorrowedFd<'_>, optional_actions: OptionalActions, termios2: &c::termios2, From f13a121d1ab6bcd61160d2c45944c9ed5d7228f4 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 5 Sep 2024 04:58:22 -0700 Subject: [PATCH 15/16] Simplify the code by using `Default::default`. --- src/backend/libc/termios/syscalls.rs | 7 +++---- src/utils.rs | 6 ------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index e036f8349..3d7dc854b 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -34,7 +34,6 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { #[cfg(linux_kernel)] { use crate::termios::{ControlModes, InputModes, LocalModes, OutputModes, SpecialCodes}; - use crate::utils::default_array; let mut termios2 = MaybeUninit::::uninit(); let ptr = termios2.as_mut_ptr(); @@ -67,7 +66,7 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { control_modes: ControlModes::from_bits_retain(termios2.c_cflag), local_modes: LocalModes::from_bits_retain(termios2.c_lflag), line_discipline: termios2.c_line, - special_codes: SpecialCodes(default_array()), + special_codes: SpecialCodes(Default::default()), input_speed: termios2.c_ispeed, output_speed: termios2.c_ospeed, }; @@ -174,7 +173,6 @@ pub(crate) fn tcsetattr( #[cfg(linux_kernel)] { use crate::termios::speed; - use crate::utils::default_array; let output_speed = termios.output_speed(); let input_speed = termios.input_speed(); @@ -184,7 +182,7 @@ pub(crate) fn tcsetattr( c_cflag: termios.control_modes.bits(), c_lflag: termios.local_modes.bits(), c_line: termios.line_discipline, - c_cc: default_array(), + c_cc: Default::default(), c_ispeed: input_speed, c_ospeed: output_speed, }; @@ -195,6 +193,7 @@ pub(crate) fn tcsetattr( termios2.c_cflag |= speed::encode(output_speed).unwrap_or(c::BOTHER); termios2.c_cflag &= !c::CIBAUD; termios2.c_cflag |= speed::encode(input_speed).unwrap_or(c::BOTHER) << c::IBSHIFT; + let nccs = termios2.c_cc.len(); termios2 .c_cc diff --git a/src/utils.rs b/src/utils.rs index d21ed88b6..77eb26c14 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -50,12 +50,6 @@ pub(crate) fn check_raw_pointer(value: *mut c_void) -> Option> { NonNull::new(value.cast()) } -/// Create an array value containing all default values, inferring the type. -#[inline] -pub(crate) fn default_array() -> [T; N] { - [T::default(); N] -} - /// Create a union value containing a default value in one of its arms. /// /// The field names a union field which must have the same size as the union From 0781762ca8aecdb2914fbcb31fb6cd16e56633eb Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 5 Sep 2024 04:58:47 -0700 Subject: [PATCH 16/16] Add more comments. --- src/backend/libc/termios/syscalls.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/backend/libc/termios/syscalls.rs b/src/backend/libc/termios/syscalls.rs index 3d7dc854b..bc26f68ce 100644 --- a/src/backend/libc/termios/syscalls.rs +++ b/src/backend/libc/termios/syscalls.rs @@ -71,7 +71,10 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result { output_speed: termios2.c_ospeed, }; - result.special_codes.0[..termios2.c_cc.len()].copy_from_slice(&termios2.c_cc); + // Copy in the control codes, since libc's `c_cc` array may have a + // different length from the ioctl's. + let nccs = termios2.c_cc.len(); + result.special_codes.0[..nccs].copy_from_slice(&termios2.c_cc); Ok(result) } @@ -194,6 +197,8 @@ pub(crate) fn tcsetattr( termios2.c_cflag &= !c::CIBAUD; termios2.c_cflag |= speed::encode(input_speed).unwrap_or(c::BOTHER) << c::IBSHIFT; + // Copy in the control codes, since libc's `c_cc` array may have a + // different length from the ioctl's. let nccs = termios2.c_cc.len(); termios2 .c_cc