Skip to content

Make nix::sys::termios::Termios implement Sync and Copy. #1324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
123 changes: 57 additions & 66 deletions src/sys/termios.rs
Original file line number Diff line number Diff line change
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,20 @@ 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`, their types, and even their interpretations
// vary from one platform to another. We want to expose a portable subset as public fields of
// `Termios`, using stable types, and allow access to the rest via accessors, if at all.
//
// So we carry around this private `libc::termios`, initialize our public fields from it
// whenever it's changed (see `set_libc_termios`), and propagate our public fields' values back
// into it whenever we need a real `libc::termios` to pass to `tcsetattr`, `cfmakeraw`, or the
// like (see `get_libc_termios`).
//
// Any fields unknown to us get reasonable values from the initial `tcgetattr` used to create
// this `Termios`, and are thus passed through to subsequent `tcsetattr` calls unchanged.
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 @@ -183,43 +193,24 @@ pub struct Termios {
}

impl Termios {
/// Exposes an immutable reference to the underlying `libc::termios` data structure.
/// Return a copy of the internal `libc::termios` structure.
///
/// 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;
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();
/// Set the internal `libc::termios` structure to `termios`,
/// and update the public fields.
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 +222,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 +234,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 +872,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 +881,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 +900,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 +912,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 +926,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 +935,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 +954,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 +966,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 +981,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 +994,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 +1025,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