From 28b4ef48cf8d87d99ff9bb0a3e0834543761d99c Mon Sep 17 00:00:00 2001 From: Bryant Mairs Date: Sun, 10 May 2020 21:09:19 -0700 Subject: [PATCH] Limit internal termios API to pub(crate) --- CHANGELOG.md | 4 +++ src/sys/termios.rs | 67 ++++++++++------------------------------ test/sys/test_termios.rs | 8 +---- 3 files changed, 22 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e2b5ba1a..4df5615bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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

where P: NixPath`. Most Nix functions that accept `NixPath` arguments can't do anything useful with diff --git a/src/sys/termios.rs b/src/sys/termios.rs index 5af5a1588f..c30de80d4b 100644 --- a/src/sys/termios.rs +++ b/src/sys/termios.rs @@ -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; //! ``` //! @@ -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; //! ``` @@ -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); @@ -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)); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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 { + /// 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 { { let mut termios = self.inner.borrow_mut(); termios.c_iflag = self.input_flags.bits(); @@ -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(); @@ -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); diff --git a/test/sys/test_termios.rs b/test/sys/test_termios.rs index 40b47af4e1..4fa6df9b29 100644 --- a/test/sys/test_termios.rs +++ b/test/sys/test_termios.rs @@ -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 @@ -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); -}