diff options
author | Nelson Chen <crazysim@gmail.com> | 2017-08-24 00:47:20 -0700 |
---|---|---|
committer | Nelson Chen <crazysim@gmail.com> | 2017-08-24 10:40:07 -0700 |
commit | 505bed350d37811c1ed52302f4b820711a57041f (patch) | |
tree | cfe0af2bc6cbbd18242870f32f558dd9db95a6e2 | |
parent | b76cbeea39a1e11872e9316711ac093dc99ad91d (diff) | |
download | nix-505bed350d37811c1ed52302f4b820711a57041f.zip |
Mark and document pty::ptsname() as unsafe
`ptsname()` mutates global variables and mutating global variables is
always considered `unsafe` by Rust.
Reference:
https://github.com/nix-rust/nix/pull/742#issuecomment-324385919
-rw-r--r-- | CHANGELOG.md | 2 | ||||
-rw-r--r-- | src/pty.rs | 22 | ||||
-rw-r--r-- | test/test_pty.rs | 12 |
3 files changed, 22 insertions, 14 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5ebf12..148f0c6d 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 as `unsafe` + ([#744](https://github.com/nix-rust/nix/pull/744)) # Fixed - Fix compilation and tests for OpenBSD targets @@ -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 diff --git a/test/test_pty.rs b/test/test_pty.rs index 75ef4923..89706f0c 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(); |