Merge #744

744: Mark and document pty::ptsname() as unsafe r=asomers a=nelsonjchen

On some platforms, `ptsname()` mutates global variables and mutating
global variables is always considered `unsafe` by Rust.

Reference:

#742 (comment)

Author: bors[bot]

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 as `unsafe`
+  ([#744](https://github.com/nix-rust/nix/pull/744))
 
 # Fixed
 - Fix compilation and tests for OpenBSD targets

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,26 @@ pub fn posix_openpt(flags: fcntl::OFlag) -> Result<PtyMaster> {
 /// [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()`.
+///
+/// # Safety
+///
+/// `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<String> {
-    let name_ptr = unsafe { libc::ptsname(fd.as_raw_fd()) };
+pub unsafe fn ptsname(fd: &PtyMaster) -> Result<String> {
+    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 +193,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

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