Skip to content

Commit

Permalink
Limit internal termios API to pub(crate)
Browse files Browse the repository at this point in the history
  • Loading branch information
Susurrus committed Jun 5, 2020
1 parent a777389 commit 28b4ef4
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 57 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).

- Removed `sys::socket::addr::from_libc_sockaddr` from the public API.
(#[1215](https://github.com/nix-rust/nix/pull/1215))
- Removed `sys::termios::{get_libc_termios, get_libc_termios_mut, update_wrapper`
from the public API. These were previously hidden in the docs but still usable
by downstream.
(#[1235](https://github.com/nix-rust/nix/pull/1235))

- Nix no longer implements `NixPath` for `Option<P> where P: NixPath`. Most
Nix functions that accept `NixPath` arguments can't do anything useful with
Expand Down
67 changes: 17 additions & 50 deletions src/sys/termios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//! ```
//! # use self::nix::sys::termios::SpecialCharacterIndices::VEOF;
//! # use self::nix::sys::termios::{_POSIX_VDISABLE, Termios};
//! # let mut termios = unsafe { Termios::default_uninit() };
//! # let mut termios: Termios = unsafe { std::mem::zeroed() };
//! termios.control_chars[VEOF as usize] = _POSIX_VDISABLE;
//! ```
//!
Expand All @@ -38,7 +38,7 @@
//!
//! ```
//! # use self::nix::sys::termios::{ControlFlags, Termios};
//! # let mut termios = unsafe { Termios::default_uninit() };
//! # let mut termios: Termios = unsafe { std::mem::zeroed() };
//! termios.control_flags & ControlFlags::CSIZE == ControlFlags::CS5;
//! termios.control_flags |= ControlFlags::CS5;
//! ```
Expand All @@ -61,10 +61,9 @@
//! platforms:
//!
//! ```rust
//! # #[macro_use] extern crate nix;
//! # use nix::sys::termios::{BaudRate, cfsetispeed, cfsetospeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! cfsetispeed(&mut t, BaudRate::B9600);
//! cfsetospeed(&mut t, BaudRate::B9600);
//! cfsetspeed(&mut t, BaudRate::B9600);
Expand All @@ -76,7 +75,7 @@
//! ```rust
//! # use nix::sys::termios::{BaudRate, cfgetispeed, cfgetospeed, cfsetispeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! # cfsetspeed(&mut t, BaudRate::B9600);
//! let speed = cfgetispeed(&t);
//! assert_eq!(speed, cfgetospeed(&t));
Expand All @@ -94,7 +93,7 @@
doc = " ```rust")]
//! # use nix::sys::termios::{BaudRate, cfgetispeed, cfgetospeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! # cfsetspeed(&mut t, BaudRate::B9600);
//! assert_eq!(cfgetispeed(&t), BaudRate::B9600);
//! assert_eq!(cfgetospeed(&t), BaudRate::B9600);
Expand All @@ -111,7 +110,7 @@
doc = " ```rust,ignore")]
//! # use nix::sys::termios::{BaudRate, cfgetispeed, cfgetospeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! # cfsetspeed(&mut t, 9600u32);
//! assert_eq!(cfgetispeed(&t), 9600u32);
//! assert_eq!(cfgetospeed(&t), 9600u32);
Expand All @@ -128,7 +127,7 @@
doc = " ```rust,ignore")]
//! # use nix::sys::termios::{BaudRate, cfgetispeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! # cfsetspeed(&mut t, 9600u32);
//! assert_eq!(cfgetispeed(&t), BaudRate::B9600.into());
//! assert_eq!(u32::from(BaudRate::B9600), 9600u32);
Expand All @@ -146,7 +145,7 @@
doc = " ```rust,ignore")]
//! # use nix::sys::termios::{cfsetispeed, cfsetospeed, cfsetspeed, Termios};
//! # fn main() {
//! # let mut t = unsafe { Termios::default_uninit() };
//! # let mut t: Termios = unsafe { std::mem::zeroed() };
//! cfsetispeed(&mut t, 9600u32);
//! cfsetospeed(&mut t, 9600u32);
//! cfsetspeed(&mut t, 9600u32);
Expand Down Expand Up @@ -186,22 +185,9 @@ pub struct Termios {
impl Termios {
/// Exposes an immutable reference to the underlying `libc::termios` data structure.
///
/// This can be used for interfacing with other FFI functions like:
///
/// ```rust
/// # fn main() {
/// # use nix::sys::termios::Termios;
/// # let mut termios = unsafe { Termios::default_uninit() };
/// let inner_termios = termios.get_libc_termios();
/// unsafe { libc::cfgetispeed(&*inner_termios) };
/// # }
/// ```
///
/// There is no public API exposed for functions that modify the underlying `libc::termios`
/// data because it requires additional work to maintain type safety.
// FIXME: Switch this over to use pub(crate)
#[doc(hidden)]
pub fn get_libc_termios(&self) -> Ref<libc::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();
Expand All @@ -215,12 +201,11 @@ impl Termios {

/// 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. Therefore we disable docs to
/// effectively limit its use to nix internals. In this case it should also be paired with a
/// call to `update_wrapper()` so that the wrapper-type and internal representation stay
/// consistent.
unsafe fn get_libc_termios_mut(&mut self) -> *mut libc::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();
Expand All @@ -232,26 +217,8 @@ impl Termios {
self.inner.as_ptr()
}

/// Allows for easily creating new `Termios` structs that will be overwritten with real data.
///
/// This should only be used when the inner libc::termios struct will be overwritten before it's
/// read.
// FIXME: Switch this over to use pub(crate)
#[doc(hidden)]
pub unsafe fn default_uninit() -> Self {
Termios {
inner: RefCell::new(mem::zeroed()),
input_flags: InputFlags::empty(),
output_flags: OutputFlags::empty(),
control_flags: ControlFlags::empty(),
local_flags: LocalFlags::empty(),
control_chars: [0 as libc::cc_t; NCCS],
}
}

/// Updates the wrapper values from the internal `libc::termios` data structure.
#[doc(hidden)]
pub fn update_wrapper(&mut self) {
pub(crate) fn update_wrapper(&mut self) {
let termios = *self.inner.borrow_mut();
self.input_flags = InputFlags::from_bits_truncate(termios.c_iflag);
self.output_flags = OutputFlags::from_bits_truncate(termios.c_oflag);
Expand Down
8 changes: 1 addition & 7 deletions test/sys/test_termios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tempfile::tempfile;
use nix::{Error, fcntl};
use nix::errno::Errno;
use nix::pty::openpty;
use nix::sys::termios::{self, LocalFlags, OutputFlags, Termios, tcgetattr};
use nix::sys::termios::{self, LocalFlags, OutputFlags, tcgetattr};
use nix::unistd::{read, write, close};

/// Helper function analogous to `std::io::Write::write_all`, but for `RawFd`s
Expand Down Expand Up @@ -128,9 +128,3 @@ fn test_local_flags() {
close(pty.slave).unwrap();
assert_eq!(read, Error::Sys(Errno::EAGAIN));
}

#[test]
fn test_cfmakeraw() {
let mut termios = unsafe { Termios::default_uninit() };
termios::cfmakeraw(&mut termios);
}

0 comments on commit 28b4ef4

Please sign in to comment.