diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5ebf1261..e37e1c7497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). - `MapFlags`, `MmapAdvise`, and `MsFlags` expose some more variants and only officially-supported variants are provided for each target. ([#731](https://github.com/nix-rust/nix/pull/731)) +- Marked `pty::ptsname` function to be as `unsafe` + ([#744](https://github.com/nix-rust/nix/pull/744)) # Fixed - Fix compilation and tests for OpenBSD targets diff --git a/src/pty.rs b/src/pty.rs index 26c8b500cd..1f192425bb 100644 --- a/src/pty.rs +++ b/src/pty.rs @@ -100,7 +100,7 @@ pub fn grantpt(fd: &PtyMaster) -> Result<()> { /// unlockpt(&master_fd)?; /// /// // Get the name of the slave -/// let slave_name = ptsname(&master_fd)?; +/// let slave_name = unsafe { ptsname(&master_fd) }?; /// /// // Try to open the slave /// # #[allow(unused_variables)] @@ -125,20 +125,24 @@ pub fn posix_openpt(flags: fcntl::OFlag) -> Result { /// [ptsname(3)](http://man7.org/linux/man-pages/man3/ptsname.3.html)) /// /// `ptsname()` returns the name of the slave pseudoterminal device corresponding to the master -/// referred to by `fd`. Note that this function is *not* threadsafe. For that see `ptsname_r()`. +/// referred to by `fd`. /// /// This value is useful for opening the slave pty once the master has already been opened with /// `posix_openpt()`. +/// +/// On some platforms, `ptsname()` mutates global variables and is *not* threadsafe. +/// Mutating global variables is always considered `unsafe` by Rust and this +/// function is marked as `unsafe` to reflect that. +/// +/// For a threadsafe and non-`unsafe` alternative on Linux, see `ptsname_r()`. #[inline] -pub fn ptsname(fd: &PtyMaster) -> Result { - let name_ptr = unsafe { libc::ptsname(fd.as_raw_fd()) }; +pub unsafe fn ptsname(fd: &PtyMaster) -> Result { + let name_ptr = libc::ptsname(fd.as_raw_fd()); if name_ptr.is_null() { return Err(Error::last().into()); } - let name = unsafe { - CStr::from_ptr(name_ptr) - }; + let name = CStr::from_ptr(name_ptr); Ok(name.to_string_lossy().into_owned()) } @@ -187,7 +191,7 @@ pub fn unlockpt(fd: &PtyMaster) -> Result<()> { /// Create a new pseudoterminal, returning the slave and master file descriptors /// in `OpenptyResult` -/// (see [openpty](http://man7.org/linux/man-pages/man3/openpty.3.html)). +/// (see [openpty](http://man7.org/linux/man-pages/man3/openpty.3.html)). /// /// If `winsize` is not `None`, the window size of the slave will be set to /// the values in `winsize`. If `termios` is not `None`, the pseudoterminal's diff --git a/test/test_pty.rs b/test/test_pty.rs index 75ef4923af..89706f0cfb 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -35,7 +35,7 @@ fn test_ptsname_equivalence() { assert!(master_fd.as_raw_fd() > 0); // Get the name of the slave - let slave_name = ptsname(&master_fd).unwrap(); + let slave_name = unsafe { ptsname(&master_fd) }.unwrap() ; let slave_name_r = ptsname_r(&master_fd).unwrap(); assert_eq!(slave_name, slave_name_r); } @@ -53,8 +53,8 @@ fn test_ptsname_copy() { assert!(master_fd.as_raw_fd() > 0); // Get the name of the slave - let slave_name1 = ptsname(&master_fd).unwrap(); - let slave_name2 = ptsname(&master_fd).unwrap(); + let slave_name1 = unsafe { ptsname(&master_fd) }.unwrap(); + let slave_name2 = unsafe { ptsname(&master_fd) }.unwrap(); assert!(slave_name1 == slave_name2); // Also make sure that the string was actually copied and they point to different parts of // memory. @@ -92,8 +92,8 @@ fn test_ptsname_unique() { assert!(master2_fd.as_raw_fd() > 0); // Get the name of the slave - let slave_name1 = ptsname(&master1_fd).unwrap(); - let slave_name2 = ptsname(&master2_fd).unwrap(); + let slave_name1 = unsafe { ptsname(&master1_fd) }.unwrap(); + let slave_name2 = unsafe { ptsname(&master2_fd) }.unwrap(); assert!(slave_name1 != slave_name2); } @@ -116,7 +116,7 @@ fn test_open_ptty_pair() { unlockpt(&master_fd).expect("unlockpt failed"); // Get the name of the slave - let slave_name = ptsname(&master_fd).expect("ptsname failed"); + let slave_name = unsafe { ptsname(&master_fd) }.expect("ptsname failed"); // Open the slave device let slave_fd = open(Path::new(&slave_name), O_RDWR, stat::Mode::empty()).unwrap();