Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement ttyname and ttyname_r #1259

Merged
merged 1 commit into from Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -37,6 +37,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
(#[1222](https://github.com/nix-rust/nix/pull/1222))
- `CpuSet` and `UnixCredentials` now implement `Default`.
(#[1244](https://github.com/nix-rust/nix/pull/1244))
- Added `unistd::ttyname`
(#[1259](https://github.com/nix-rust/nix/pull/1259))

### Changed
- Changed `fallocate` return type from `c_int` to `()` (#[1201](https://github.com/nix-rust/nix/pull/1201))
Expand Down
17 changes: 17 additions & 0 deletions src/unistd.rs
Expand Up @@ -2774,3 +2774,20 @@ impl Group {
})
}
}

/// Get the name of the terminal device that is open on file descriptor fd
/// (see [`ttyname(3)`](http://man7.org/linux/man-pages/man3/ttyname.3.html)).
pub fn ttyname(fd: RawFd) -> Result<PathBuf> {
const PATH_MAX: usize = libc::PATH_MAX as usize;
let mut buf = vec![0_u8; PATH_MAX];
let c_buf = buf.as_mut_ptr() as *mut libc::c_char;

let ret = unsafe { libc::ttyname_r(fd, c_buf, buf.len()) };
if ret != 0 {
return Err(Error::Sys(Errno::from_i32(ret)));
}

let nul = buf.iter().position(|c| *c == b'\0').unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually searching for NUL, why not just use CStr::from_ptr? In fact, you should technically be using OsString here instead of CString, because the string comes from the operating system and not from another programming language. So you should declare the buf as a Vec, and then use OsString::from_vec followed by to_string_lossy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think returning a PathBuf is actually better than String here, and using OsString for that makes sense. that does mean that i do have to still search for the nul myself though, since OsString won't do that (it'll just create a string with a bunch of trailing nuls). going through CStr (and then being forced to allocate a copy into a second Vec) doesn't seem worth it to avoid that?

buf.truncate(nul);
Ok(OsString::from_vec(buf).into())
}
47 changes: 47 additions & 0 deletions test/test_unistd.rs
Expand Up @@ -7,6 +7,8 @@ use nix::unistd::ForkResult::*;
use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, sigaction};
use nix::sys::wait::*;
use nix::sys::stat::{self, Mode, SFlag};
#[cfg(not(target_os = "redox"))]
use nix::pty::{posix_openpt, grantpt, unlockpt, ptsname};
use nix::errno::Errno;
#[cfg(not(target_os = "redox"))]
use nix::Error;
Expand All @@ -19,6 +21,8 @@ use std::fs::{self, File};
use std::io::Write;
use std::mem;
use std::os::unix::prelude::*;
#[cfg(not(target_os = "redox"))]
use std::path::Path;
use tempfile::{tempdir, tempfile};
use libc::{_exit, off_t};

Expand Down Expand Up @@ -964,3 +968,46 @@ fn test_setfsuid() {
// open the temporary file with the current thread filesystem UID
fs::File::open(temp_path_2).unwrap();
}

#[test]
#[cfg(not(target_os = "redox"))]
fn test_ttyname() {
let fd = posix_openpt(OFlag::O_RDWR).expect("posix_openpt failed");
assert!(fd.as_raw_fd() > 0);

// on linux, we can just call ttyname on the pty master directly, but
// apparently osx requires that ttyname is called on a slave pty (can't
// find this documented anywhere, but it seems to empirically be the case)
grantpt(&fd).expect("grantpt failed");
unlockpt(&fd).expect("unlockpt failed");
let sname = unsafe { ptsname(&fd) }.expect("ptsname failed");
let fds = open(
Path::new(&sname),
OFlag::O_RDWR,
stat::Mode::empty(),
).expect("open failed");
assert!(fds > 0);

let name = ttyname(fds).expect("ttyname failed");
assert!(name.starts_with("/dev"));
}

#[test]
#[cfg(not(target_os = "redox"))]
fn test_ttyname_not_pty() {
let fd = File::open("/dev/zero").unwrap();
assert!(fd.as_raw_fd() > 0);
assert_eq!(ttyname(fd.as_raw_fd()), Err(Error::Sys(Errno::ENOTTY)));
}

#[test]
#[cfg(all(not(target_os = "redox"), not(target_env = "musl")))]
fn test_ttyname_invalid_fd() {
assert_eq!(ttyname(-1), Err(Error::Sys(Errno::EBADF)));
}

#[test]
#[cfg(all(not(target_os = "redox"), target_env = "musl"))]
fn test_ttyname_invalid_fd() {
assert_eq!(ttyname(-1), Err(Error::Sys(Errno::ENOTTY)));
}