From 56334c3ceaf4d007d39b0b9686b3c4be253ea183 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 9 Sep 2024 08:43:01 -0700 Subject: [PATCH 1/3] Update the API for I/O safety Add new `terminal_size_of` fuctions which use the `AsFd` and `AsHandle` traits to accept file descriptors (on Unix) and handles (on Windows). Deprecate the existing `terminal_size_using_*` functions which take raw file descriptors or handles without being `unsafe`. And update the `get_size` example to use the new `terminal_size_of` functions. The example can now be much simpler because the main API is now the same between Windows and Unix in common cases. --- examples/get_size.rs | 51 ++++++++------------------------------------ src/lib.rs | 6 ++++-- src/unix.rs | 35 +++++++++++++++++++----------- src/windows.rs | 19 ++++++++++++++--- 4 files changed, 51 insertions(+), 60 deletions(-) diff --git a/examples/get_size.rs b/examples/get_size.rs index 9039b42..31f3ab2 100644 --- a/examples/get_size.rs +++ b/examples/get_size.rs @@ -1,52 +1,19 @@ -#[cfg(windows)] -fn run() { - use std::os::windows::io::RawHandle; - use windows_sys::Win32::System::Console::{ - GetStdHandle, STD_ERROR_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, - }; - - let stdout = unsafe { GetStdHandle(STD_OUTPUT_HANDLE) } as RawHandle; - println!( - "Size from terminal_size_using_handle(stdout): {:?}", - terminal_size::terminal_size_using_handle(stdout) - ); - - let stderr = unsafe { GetStdHandle(STD_ERROR_HANDLE) } as RawHandle; - println!( - "Size from terminal_size_using_handle(stderr): {:?}", - terminal_size::terminal_size_using_handle(stderr) - ); - - let stdin = unsafe { GetStdHandle(STD_INPUT_HANDLE) } as RawHandle; +fn main() { println!( - "Size from terminal_size_using_handle(stdin): {:?}", - terminal_size::terminal_size_using_handle(stdin) + "Size from terminal_size(): {:?}", + terminal_size::terminal_size() ); -} - -#[cfg(not(windows))] -fn run() { - use std::os::unix::io::AsRawFd; println!( - "Size from terminal_size_using_fd(stdout): {:?}", - terminal_size::terminal_size_using_fd(std::io::stdout().as_raw_fd()) + "Size from terminal_size_of(stdout): {:?}", + terminal_size::terminal_size_of(std::io::stdout()) ); println!( - "Size from terminal_size_using_fd(stderr): {:?}", - terminal_size::terminal_size_using_fd(std::io::stderr().as_raw_fd()) + "Size from terminal_size_of(stderr): {:?}", + terminal_size::terminal_size_of(std::io::stderr()) ); println!( - "Size from terminal_size_using_fd(stdin): {:?}", - terminal_size::terminal_size_using_fd(std::io::stdin().as_raw_fd()) - ); -} - -fn main() { - println!( - "Size from terminal_size(): {:?}", - terminal_size::terminal_size() + "Size from terminal_size_of(stdin): {:?}", + terminal_size::terminal_size_of(std::io::stdin()) ); - - run(); } diff --git a/src/lib.rs b/src/lib.rs index 6f8990c..2c4a628 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,12 +26,14 @@ pub struct Height(pub u16); #[cfg(unix)] mod unix; #[cfg(unix)] -pub use crate::unix::{terminal_size, terminal_size_using_fd}; +#[allow(deprecated)] +pub use crate::unix::{terminal_size, terminal_size_of, terminal_size_using_fd}; #[cfg(windows)] mod windows; #[cfg(windows)] -pub use crate::windows::{terminal_size, terminal_size_using_handle}; +#[allow(deprecated)] +pub use crate::windows::{terminal_size, terminal_size_of, terminal_size_using_handle}; #[cfg(not(any(unix, windows)))] pub fn terminal_size() -> Option<(Width, Height)> { diff --git a/src/unix.rs b/src/unix.rs index 5fc7256..095c3ed 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -1,6 +1,5 @@ use super::{Height, Width}; -use rustix::fd::{BorrowedFd, AsRawFd}; -use std::os::unix::io::RawFd; +use std::os::unix::io::{AsFd, BorrowedFd, RawFd}; /// Returns the size of the terminal. /// @@ -8,11 +7,11 @@ use std::os::unix::io::RawFd; /// The size of the first stream that is a TTY will be returned. If nothing /// is a TTY, then `None` is returned. pub fn terminal_size() -> Option<(Width, Height)> { - if let Some(size) = terminal_size_using_fd(std::io::stdout().as_raw_fd()) { + if let Some(size) = terminal_size_of(std::io::stdout()) { Some(size) - } else if let Some(size) = terminal_size_using_fd(std::io::stderr().as_raw_fd()) { + } else if let Some(size) = terminal_size_of(std::io::stderr()) { Some(size) - } else if let Some(size) = terminal_size_using_fd(std::io::stdin().as_raw_fd()) { + } else if let Some(size) = terminal_size_of(std::io::stdin()) { Some(size) } else { None @@ -22,19 +21,14 @@ pub fn terminal_size() -> Option<(Width, Height)> { /// Returns the size of the terminal using the given file descriptor, if available. /// /// If the given file descriptor is not a tty, returns `None` -pub fn terminal_size_using_fd(fd: RawFd) -> Option<(Width, Height)> { +pub fn terminal_size_of(fd: Fd) -> Option<(Width, Height)> { use rustix::termios::{isatty, tcgetwinsize}; - // TODO: Once I/O safety is stabilized, the enlosing function here should - // be unsafe due to taking a `RawFd`. We should then move the main - // logic here into a new function which takes a `BorrowedFd` and is safe. - let fd = unsafe { BorrowedFd::borrow_raw(fd) }; - - if !isatty(fd) { + if !isatty(&fd) { return None; } - let winsize = tcgetwinsize(fd).ok()?; + let winsize = tcgetwinsize(&fd).ok()?; let rows = winsize.ws_row; let cols = winsize.ws_col; @@ -46,6 +40,21 @@ pub fn terminal_size_using_fd(fd: RawFd) -> Option<(Width, Height)> { } } +/// Returns the size of the terminal using the given raw file descriptor, if available. +/// +/// The given file descriptor must be an open file descriptor. +/// +/// If the given file descriptor is not a tty, returns `None` +#[deprecated(note = "Use `terminal_size_of` instead. + Use `BorrowedFd::borrow_raw` to convert a raw fd into a `BorrowedFd` if needed.")] +pub fn terminal_size_using_fd(fd: RawFd) -> Option<(Width, Height)> { + // SAFETY: Under I/O safety, this function should be `unsafe`, but we can't + // remove it without breaking compatibility, so we instead deprecate it. + // This unsafe block has the same precondition that the function implicitly + // does: `fd` must be an open handle. + unsafe { terminal_size_of(BorrowedFd::borrow_raw(fd)) } +} + #[test] /// Compare with the output of `stty size` fn compare_with_stty() { diff --git a/src/windows.rs b/src/windows.rs index ff98b86..b9f4036 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -1,5 +1,5 @@ use super::{Height, Width}; -use std::os::windows::io::RawHandle; +use std::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, RawHandle}; /// Returns the size of the terminal. /// @@ -34,14 +34,14 @@ pub fn terminal_size() -> Option<(Width, Height)> { /// Returns the size of the terminal using the given handle, if available. /// /// If the given handle is not a tty, returns `None` -pub fn terminal_size_using_handle(handle: RawHandle) -> Option<(Width, Height)> { +pub fn terminal_size_of(handle: Handle) -> Option<(Width, Height)> { use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; use windows_sys::Win32::System::Console::{ GetConsoleScreenBufferInfo, CONSOLE_SCREEN_BUFFER_INFO, COORD, SMALL_RECT, }; // convert between windows_sys::Win32::Foundation::HANDLE and std::os::windows::raw::HANDLE - let hand = handle as windows_sys::Win32::Foundation::HANDLE; + let hand = handle.as_handle().as_raw_handle() as windows_sys::Win32::Foundation::HANDLE; if hand == INVALID_HANDLE_VALUE { return None; @@ -68,3 +68,16 @@ pub fn terminal_size_using_handle(handle: RawHandle) -> Option<(Width, Height)> let h: Height = Height((csbi.srWindow.Bottom - csbi.srWindow.Top + 1) as u16); Some((w, h)) } + +/// Returns the size of the terminal using the given handle, if available. +/// +/// The given handle must be an open handle. +/// +/// If the given handle is not a tty, returns `None` +pub fn terminal_size_using_handle(handle: RawHandle) -> Option<(Width, Height)> { + // SAFETY: Under I/O safety, this function should be `unsafe`, but we can't + // remove it without breaking compatibility, so we instead deprecate it. + // This unsafe block has the same precondition that the function implicitly + // does: `handle` must be a valid open file descriptor. + unsafe { terminal_size_of(BorrowedHandle::borrow_raw(handle)) } +} From ea92388054dd64c9ad452bea2e1e667ef8462b99 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 16 Sep 2024 08:35:07 -0700 Subject: [PATCH 2/3] Mark `terminal_size_using_fd` as unsafe. --- src/unix.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/unix.rs b/src/unix.rs index 095c3ed..a07b5e8 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -45,14 +45,14 @@ pub fn terminal_size_of(fd: Fd) -> Option<(Width, Height)> { /// The given file descriptor must be an open file descriptor. /// /// If the given file descriptor is not a tty, returns `None` +/// +/// # Safety +/// +/// `fd` must be a valid open file descriptor. #[deprecated(note = "Use `terminal_size_of` instead. Use `BorrowedFd::borrow_raw` to convert a raw fd into a `BorrowedFd` if needed.")] -pub fn terminal_size_using_fd(fd: RawFd) -> Option<(Width, Height)> { - // SAFETY: Under I/O safety, this function should be `unsafe`, but we can't - // remove it without breaking compatibility, so we instead deprecate it. - // This unsafe block has the same precondition that the function implicitly - // does: `fd` must be an open handle. - unsafe { terminal_size_of(BorrowedFd::borrow_raw(fd)) } +pub unsafe fn terminal_size_using_fd(fd: RawFd) -> Option<(Width, Height)> { + terminal_size_of(BorrowedFd::borrow_raw(fd)) } #[test] From a29b90458018676b30e1bba02aed186bd0b3a907 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 17 Sep 2024 16:46:22 -0700 Subject: [PATCH 3/3] Mark `terminal_size_using_handle` as unsafe too. --- src/windows.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index b9f4036..5c09663 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -14,17 +14,17 @@ pub fn terminal_size() -> Option<(Width, Height)> { GetStdHandle, STD_ERROR_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, }; - if let Some(size) = - terminal_size_using_handle(unsafe { GetStdHandle(STD_OUTPUT_HANDLE) as RawHandle }) - { + if let Some(size) = terminal_size_of(unsafe { + BorrowedHandle::borrow_raw(GetStdHandle(STD_OUTPUT_HANDLE) as RawHandle) + }) { Some(size) - } else if let Some(size) = - terminal_size_using_handle(unsafe { GetStdHandle(STD_ERROR_HANDLE) as RawHandle }) - { + } else if let Some(size) = terminal_size_of(unsafe { + BorrowedHandle::borrow_raw(GetStdHandle(STD_ERROR_HANDLE) as RawHandle) + }) { Some(size) - } else if let Some(size) = - terminal_size_using_handle(unsafe { GetStdHandle(STD_INPUT_HANDLE) as RawHandle }) - { + } else if let Some(size) = terminal_size_of(unsafe { + BorrowedHandle::borrow_raw(GetStdHandle(STD_INPUT_HANDLE) as RawHandle) + }) { Some(size) } else { None @@ -74,10 +74,12 @@ pub fn terminal_size_of(handle: Handle) -> Option<(Width, Heig /// The given handle must be an open handle. /// /// If the given handle is not a tty, returns `None` -pub fn terminal_size_using_handle(handle: RawHandle) -> Option<(Width, Height)> { - // SAFETY: Under I/O safety, this function should be `unsafe`, but we can't - // remove it without breaking compatibility, so we instead deprecate it. - // This unsafe block has the same precondition that the function implicitly - // does: `handle` must be a valid open file descriptor. - unsafe { terminal_size_of(BorrowedHandle::borrow_raw(handle)) } +/// +/// # Safety +/// +/// `handle` must be a valid open file handle. +#[deprecated(note = "Use `terminal_size_of` instead. + Use `BorrowedHandle::borrow_raw` to convert a raw handle into a `BorrowedHandle` if needed.")] +pub unsafe fn terminal_size_using_handle(handle: RawHandle) -> Option<(Width, Height)> { + terminal_size_of(BorrowedHandle::borrow_raw(handle)) }