Merge pull request #1235 from nix-rust/termios_constructor

Limit internal termios API to pub(crate)

Author: Susurrus

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<P> where P: NixPath`.  Most
   Nix functions that accept `NixPath` arguments can't do anything useful with

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<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();
@@ -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);

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);
-}