Skip to content

Commit

Permalink
Make nix::sys::termios::Termios implement Sync and Copy.
Browse files Browse the repository at this point in the history 
It's kind of tough that `Termios` doesn't implement `Sync`, because then you
can't use them with the `signal_hook` crate's `register` function, or any other
signal-handling wrapper that tries to be safe.

This approach probably entails a bit more copying bits around, but I'd argue the
convenience of `Sync` and `Copy` implementations is worth it.
  • Loading branch information
@jimblandy
jimblandy committed Oct 21, 2020
1 parent 5d2a8c2 commit 8b097a5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 67 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased] - ReleaseDate
### Added
- Added `mremap` (#[1306](https://github.com/nix-rust/nix/pull/1306))
- `nix::sys::termios::Termios` now implements `Sync` and `Copy`.
- There are now `From` implementations for converting both ways between
`libc::termios` and `nix::sys::termios::Termios`.
### Changed
### Fixed
### Removed
Expand Down
6 changes: 3 additions & 3 deletions src/pty.rs
View file
Expand Up @@ -243,7 +243,7 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
&*inner_termios as *const libc::termios as *mut _,
&inner_termios as *const libc::termios as *mut _,
winsize as *const Winsize as *mut _,
)
}
Expand All @@ -266,7 +266,7 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
master.as_mut_ptr(),
slave.as_mut_ptr(),
ptr::null_mut(),
&*inner_termios as *const libc::termios as *mut _,
&inner_termios as *const libc::termios as *mut _,
ptr::null_mut(),
)
}
Expand Down Expand Up @@ -313,7 +313,7 @@ pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
let term = match termios.into() {
Some(termios) => {
let inner_termios = termios.get_libc_termios();
&*inner_termios as *const libc::termios as *mut _
&inner_termios as *const libc::termios as *mut _
},
None => ptr::null_mut(),
};
Expand Down
112 changes: 48 additions & 64 deletions src/sys/termios.rs
View file
Expand Up @@ -155,7 +155,6 @@ use cfg_if::cfg_if;
use crate::{Error, Result};
use crate::errno::Errno;
use libc::{self, c_int, tcflag_t};
use std::cell::{Ref, RefCell};
use std::convert::{From, TryFrom};
use std::mem;
use std::os::unix::io::RawFd;
Expand All @@ -167,9 +166,14 @@ use crate::unistd::Pid;
/// This is a wrapper around the `libc::termios` struct that provides a safe interface for the
/// standard fields. The only safe way to obtain an instance of this struct is to extract it from
/// an open port using `tcgetattr()`.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct Termios {
inner: RefCell<libc::termios>,
// The actual fields present in `libc::termios` vary from one platform to
// another. By only constructing `Termios` values from actual tcgetattr
// calls, we ensure that unknown fields have reasonable values. We just need
// to remember to update `inner` before it's read, and update the public
// fields after `inner` has been modified.
inner: libc::termios,
/// Input mode flags (see `termios.c_iflag` documentation)
pub input_flags: InputFlags,
/// Output mode flags (see `termios.c_oflag` documentation)
Expand All @@ -187,39 +191,19 @@ impl Termios {
///
/// This is not part of `nix`'s public API because it requires additional work to maintain type
/// safety.
pub(crate) fn get_libc_termios(&self) -> Ref<libc::termios> {
{
let mut termios = self.inner.borrow_mut();
termios.c_iflag = self.input_flags.bits();
termios.c_oflag = self.output_flags.bits();
termios.c_cflag = self.control_flags.bits();
termios.c_lflag = self.local_flags.bits();
termios.c_cc = self.control_chars;
}
self.inner.borrow()
}

/// Exposes the inner `libc::termios` datastore within `Termios`.
///
/// This is unsafe because if this is used to modify the inner `libc::termios` struct, it will
/// not automatically update the safe wrapper type around it. In this case it should also be
/// paired with a call to `update_wrapper()` so that the wrapper-type and internal
/// representation stay consistent.
pub(crate) unsafe fn get_libc_termios_mut(&mut self) -> *mut libc::termios {
{
let mut termios = self.inner.borrow_mut();
termios.c_iflag = self.input_flags.bits();
termios.c_oflag = self.output_flags.bits();
termios.c_cflag = self.control_flags.bits();
termios.c_lflag = self.local_flags.bits();
termios.c_cc = self.control_chars;
}
self.inner.as_ptr()
pub(crate) fn get_libc_termios(&self) -> libc::termios {
let mut termios = self.inner.clone();
termios.c_iflag = self.input_flags.bits();
termios.c_oflag = self.output_flags.bits();
termios.c_cflag = self.control_flags.bits();
termios.c_lflag = self.local_flags.bits();
termios.c_cc = self.control_chars;
termios
}

/// Updates the wrapper values from the internal `libc::termios` data structure.
pub(crate) fn update_wrapper(&mut self) {
let termios = *self.inner.borrow_mut();
pub(crate) fn set_libc_termios(&mut self, termios: &libc::termios) {
self.inner = *termios;
self.input_flags = InputFlags::from_bits_truncate(termios.c_iflag);
self.output_flags = OutputFlags::from_bits_truncate(termios.c_oflag);
self.control_flags = ControlFlags::from_bits_truncate(termios.c_cflag);
Expand All @@ -231,7 +215,7 @@ impl Termios {
impl From<libc::termios> for Termios {
fn from(termios: libc::termios) -> Self {
Termios {
inner: RefCell::new(termios),
inner: termios,
input_flags: InputFlags::from_bits_truncate(termios.c_iflag),
output_flags: OutputFlags::from_bits_truncate(termios.c_oflag),
control_flags: ControlFlags::from_bits_truncate(termios.c_cflag),
Expand All @@ -243,7 +227,7 @@ impl From<libc::termios> for Termios {

impl From<Termios> for libc::termios {
fn from(termios: Termios) -> Self {
termios.inner.into_inner()
termios.inner
}
}

Expand Down Expand Up @@ -881,7 +865,7 @@ cfg_if!{
/// `cfgetispeed()` extracts the input baud rate from the given `Termios` structure.
pub fn cfgetispeed(termios: &Termios) -> u32 {
let inner_termios = termios.get_libc_termios();
unsafe { libc::cfgetispeed(&*inner_termios) as u32 }
unsafe { libc::cfgetispeed(&inner_termios) as u32 }
}

/// Get output baud rate (see
Expand All @@ -890,17 +874,17 @@ cfg_if!{
/// `cfgetospeed()` extracts the output baud rate from the given `Termios` structure.
pub fn cfgetospeed(termios: &Termios) -> u32 {
let inner_termios = termios.get_libc_termios();
unsafe { libc::cfgetospeed(&*inner_termios) as u32 }
unsafe { libc::cfgetospeed(&inner_termios) as u32 }
}

/// Set input baud rate (see
/// [cfsetispeed(3p)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/cfsetispeed.html)).
///
/// `cfsetispeed()` sets the intput baud rate in the given `Termios` structure.
pub fn cfsetispeed<T: Into<u32>>(termios: &mut Termios, baud: T) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetispeed(inner_termios, baud.into() as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetispeed(&mut inner_termios as *mut _, baud.into() as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}

Expand All @@ -909,9 +893,9 @@ cfg_if!{
///
/// `cfsetospeed()` sets the output baud rate in the given termios structure.
pub fn cfsetospeed<T: Into<u32>>(termios: &mut Termios, baud: T) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetospeed(inner_termios, baud.into() as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetospeed(&mut inner_termios as *mut _, baud.into() as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}

Expand All @@ -921,9 +905,9 @@ cfg_if!{
/// `cfsetspeed()` sets the input and output baud rate in the given termios structure. Note that
/// this is part of the 4.4BSD standard and not part of POSIX.
pub fn cfsetspeed<T: Into<u32>>(termios: &mut Termios, baud: T) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetspeed(inner_termios, baud.into() as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetspeed(&mut inner_termios as *mut _, baud.into() as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}
} else {
Expand All @@ -935,7 +919,7 @@ cfg_if!{
/// `cfgetispeed()` extracts the input baud rate from the given `Termios` structure.
pub fn cfgetispeed(termios: &Termios) -> BaudRate {
let inner_termios = termios.get_libc_termios();
unsafe { libc::cfgetispeed(&*inner_termios) }.try_into().unwrap()
unsafe { libc::cfgetispeed(&inner_termios) }.try_into().unwrap()
}

/// Get output baud rate (see
Expand All @@ -944,17 +928,17 @@ cfg_if!{
/// `cfgetospeed()` extracts the output baud rate from the given `Termios` structure.
pub fn cfgetospeed(termios: &Termios) -> BaudRate {
let inner_termios = termios.get_libc_termios();
unsafe { libc::cfgetospeed(&*inner_termios) }.try_into().unwrap()
unsafe { libc::cfgetospeed(&inner_termios) }.try_into().unwrap()
}

/// Set input baud rate (see
/// [cfsetispeed(3p)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/cfsetispeed.html)).
///
/// `cfsetispeed()` sets the intput baud rate in the given `Termios` structure.
pub fn cfsetispeed(termios: &mut Termios, baud: BaudRate) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetispeed(inner_termios, baud as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetispeed(&mut inner_termios as *mut _, baud as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}

Expand All @@ -963,9 +947,9 @@ cfg_if!{
///
/// `cfsetospeed()` sets the output baud rate in the given `Termios` structure.
pub fn cfsetospeed(termios: &mut Termios, baud: BaudRate) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetospeed(inner_termios, baud as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetospeed(&mut inner_termios as *mut _, baud as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}

Expand All @@ -975,9 +959,9 @@ cfg_if!{
/// `cfsetspeed()` sets the input and output baud rate in the given `Termios` structure. Note that
/// this is part of the 4.4BSD standard and not part of POSIX.
pub fn cfsetspeed(termios: &mut Termios, baud: BaudRate) -> Result<()> {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let res = unsafe { libc::cfsetspeed(inner_termios, baud as libc::speed_t) };
termios.update_wrapper();
let mut inner_termios = termios.get_libc_termios();
let res = unsafe { libc::cfsetspeed(&mut inner_termios as *mut _, baud as libc::speed_t) };
termios.set_libc_termios(&inner_termios);
Errno::result(res).map(drop)
}
}
Expand All @@ -990,11 +974,11 @@ cfg_if!{
/// character, echoing is disabled, and all special input and output processing is disabled. Note
/// that this is a non-standard function, but is available on Linux and BSDs.
pub fn cfmakeraw(termios: &mut Termios) {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let mut inner_termios = termios.get_libc_termios();
unsafe {
libc::cfmakeraw(inner_termios);
libc::cfmakeraw(&mut inner_termios as *mut _);
}
termios.update_wrapper();
termios.set_libc_termios(&inner_termios);
}

/// Configures the port to "sane" mode (like the configuration of a newly created terminal) (see
Expand All @@ -1003,11 +987,11 @@ pub fn cfmakeraw(termios: &mut Termios) {
/// Note that this is a non-standard function, available on FreeBSD.
#[cfg(target_os = "freebsd")]
pub fn cfmakesane(termios: &mut Termios) {
let inner_termios = unsafe { termios.get_libc_termios_mut() };
let mut inner_termios = termios.get_libc_termios();
unsafe {
libc::cfmakesane(inner_termios);
libc::cfmakesane(&mut inner_termios as *mut _);
}
termios.update_wrapper();
termios.set_libc_termios(&inner_termios);
}

/// Return the configuration of a port
Expand All @@ -1034,7 +1018,7 @@ pub fn tcgetattr(fd: RawFd) -> Result<Termios> {
/// *any* of the parameters were successfully set, not only if all were set successfully.
pub fn tcsetattr(fd: RawFd, actions: SetArg, termios: &Termios) -> Result<()> {
let inner_termios = termios.get_libc_termios();
Errno::result(unsafe { libc::tcsetattr(fd, actions as c_int, &*inner_termios) }).map(drop)
Errno::result(unsafe { libc::tcsetattr(fd, actions as c_int, &inner_termios) }).map(drop)
}

/// Block until all output data is written (see
Expand Down

0 comments on commit 8b097a5

Please sign in to comment.