Skip to content

Commit

Permalink
fix remaining libc issues
Browse files Browse the repository at this point in the history
- use rustix version of sigwinch signal
- add a lifetime to FileDesc and replace FileDesc::Static to
  FileDesc::Borrowed. This made it necessary to either add a lifetime to
  the libc version of FileDesc or replace all the callers with multiple
  paths (libc, rustix). Changing FileDesc was more straightforward.
  There are no usages of FileDesc found in any repo on github, so this
  change should be reasonably safe.
  • Loading branch information
joshka committed May 17, 2024
1 parent 8da9a7c commit 62a0163
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/event/source/unix/mio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub(crate) struct UnixInternalEventSource {
events: Events,
parser: Parser,
tty_buffer: [u8; TTY_BUFFER_SIZE],
tty_fd: FileDesc,
tty_fd: FileDesc<'static>,
signals: Signals,
#[cfg(feature = "event-stream")]
waker: Waker,
Expand All @@ -37,7 +37,7 @@ impl UnixInternalEventSource {
UnixInternalEventSource::from_file_descriptor(tty_fd()?)
}

pub(crate) fn from_file_descriptor(input_fd: FileDesc) -> io::Result<Self> {
pub(crate) fn from_file_descriptor(input_fd: FileDesc<'static>) -> io::Result<Self> {
let poll = Poll::new()?;
let registry = poll.registry();

Expand Down
14 changes: 12 additions & 2 deletions src/event/source/unix/tty.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#[cfg(feature = "libc")]
use std::os::unix::prelude::AsRawFd;
use std::{collections::VecDeque, io, os::unix::net::UnixStream, time::Duration};

#[cfg(not(feature = "libc"))]
use rustix::fd::{AsFd, AsRawFd};

use signal_hook::low_level::pipe;

use crate::event::timeout::PollTimeout;
Expand Down Expand Up @@ -38,7 +42,7 @@ const TTY_BUFFER_SIZE: usize = 1_024;
pub(crate) struct UnixInternalEventSource {
parser: Parser,
tty_buffer: [u8; TTY_BUFFER_SIZE],
tty: FileDesc,
tty: FileDesc<'static>,
winch_signal_receiver: UnixStream,
#[cfg(feature = "event-stream")]
wake_pipe: WakePipe,
Expand All @@ -56,15 +60,18 @@ impl UnixInternalEventSource {
UnixInternalEventSource::from_file_descriptor(tty_fd()?)
}

pub(crate) fn from_file_descriptor(input_fd: FileDesc) -> io::Result<Self> {
pub(crate) fn from_file_descriptor(input_fd: FileDesc<'static>) -> io::Result<Self> {
Ok(UnixInternalEventSource {
parser: Parser::default(),
tty_buffer: [0u8; TTY_BUFFER_SIZE],
tty: input_fd,
winch_signal_receiver: {
let (receiver, sender) = nonblocking_unix_pair()?;
// Unregistering is unnecessary because EventSource is a singleton
#[cfg(feature = "libc")]
pipe::register(libc::SIGWINCH, sender)?;
#[cfg(not(feature = "libc"))]
pipe::register(rustix::process::Signal::Winch as i32, sender)?;
receiver
},
#[cfg(feature = "event-stream")]
Expand Down Expand Up @@ -157,7 +164,10 @@ impl EventSource for UnixInternalEventSource {
}
}
if fds[1].revents & POLLIN != 0 {
#[cfg(feature = "libc")]
let fd = FileDesc::new(self.winch_signal_receiver.as_raw_fd(), false);
#[cfg(not(feature = "libc"))]
let fd = FileDesc::Borrowed(self.winch_signal_receiver.as_fd());
// drain the pipe
while read_complete(&fd, &mut [0; 1024])? != 0 {}
// TODO Should we remove tput?
Expand Down
38 changes: 22 additions & 16 deletions src/terminal/sys/file_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustix::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd};
#[cfg(feature = "libc")]
use std::{
fs,
marker::PhantomData,
os::unix::{
io::{IntoRawFd, RawFd},
prelude::AsRawFd,
Expand All @@ -19,27 +20,32 @@ use std::{
/// mainly it closes the file descriptor once dropped.
#[derive(Debug)]
#[cfg(feature = "libc")]
pub struct FileDesc {
pub struct FileDesc<'a> {
fd: RawFd,
close_on_drop: bool,
phantom: PhantomData<&'a ()>,
}

#[cfg(not(feature = "libc"))]
pub enum FileDesc {
pub enum FileDesc<'a> {
Owned(OwnedFd),
Static(BorrowedFd<'static>),
Borrowed(BorrowedFd<'a>),
}

#[cfg(feature = "libc")]
impl FileDesc {
impl FileDesc<'_> {
/// Constructs a new `FileDesc` with the given `RawFd`.
///
/// # Arguments
///
/// * `fd` - raw file descriptor
/// * `close_on_drop` - specify if the raw file descriptor should be closed once the `FileDesc` is dropped
pub fn new(fd: RawFd, close_on_drop: bool) -> FileDesc {
FileDesc { fd, close_on_drop }
pub fn new(fd: RawFd, close_on_drop: bool) -> FileDesc<'static> {
FileDesc {
fd,
close_on_drop,
phantom: PhantomData,
}
}

pub fn read(&self, buffer: &mut [u8]) -> io::Result<usize> {
Expand All @@ -65,11 +71,11 @@ impl FileDesc {
}

#[cfg(not(feature = "libc"))]
impl FileDesc {
impl FileDesc<'_> {
pub fn read(&self, buffer: &mut [u8]) -> io::Result<usize> {

Check warning on line 75 in src/terminal/sys/file_descriptor.rs

View workflow job for this annotation

GitHub Actions / stable on macOS-latest

method `read` is never used

Check warning on line 75 in src/terminal/sys/file_descriptor.rs

View workflow job for this annotation

GitHub Actions / nightly on macOS-latest

method `read` is never used

Check warning on line 75 in src/terminal/sys/file_descriptor.rs

View workflow job for this annotation

GitHub Actions / nightly on ubuntu-latest

method `read` is never used

Check warning on line 75 in src/terminal/sys/file_descriptor.rs

View workflow job for this annotation

GitHub Actions / stable on ubuntu-latest

method `read` is never used
let fd = match self {
FileDesc::Owned(fd) => fd.as_fd(),
FileDesc::Static(fd) => fd.as_fd(),
FileDesc::Borrowed(fd) => fd.as_fd(),
};
let result = rustix::io::read(fd, buffer)?;
Ok(result)
Expand All @@ -78,13 +84,13 @@ impl FileDesc {
pub fn raw_fd(&self) -> RawFd {
match self {
FileDesc::Owned(fd) => fd.as_raw_fd(),
FileDesc::Static(fd) => fd.as_raw_fd(),
FileDesc::Borrowed(fd) => fd.as_raw_fd(),
}
}
}

#[cfg(feature = "libc")]
impl Drop for FileDesc {
impl Drop for FileDesc<'_> {
fn drop(&mut self) {
if self.close_on_drop {
// Note that errors are ignored when closing a file descriptor. The
Expand All @@ -97,25 +103,25 @@ impl Drop for FileDesc {
}
}

impl AsRawFd for FileDesc {
impl AsRawFd for FileDesc<'_> {
fn as_raw_fd(&self) -> RawFd {
self.raw_fd()
}
}

#[cfg(not(feature = "libc"))]
impl AsFd for FileDesc {
impl AsFd for FileDesc<'_> {
fn as_fd(&self) -> BorrowedFd<'_> {
match self {
FileDesc::Owned(fd) => fd.as_fd(),
FileDesc::Static(fd) => fd.as_fd(),
FileDesc::Borrowed(fd) => fd.as_fd(),
}
}
}

#[cfg(feature = "libc")]
/// Creates a file descriptor pointing to the standard input or `/dev/tty`.
pub fn tty_fd() -> io::Result<FileDesc> {
pub fn tty_fd() -> io::Result<FileDesc<'static>> {
let (fd, close_on_drop) = if unsafe { libc::isatty(libc::STDIN_FILENO) == 1 } {
(libc::STDIN_FILENO, false)
} else {
Expand All @@ -134,12 +140,12 @@ pub fn tty_fd() -> io::Result<FileDesc> {

#[cfg(not(feature = "libc"))]
/// Creates a file descriptor pointing to the standard input or `/dev/tty`.
pub fn tty_fd() -> io::Result<FileDesc> {
pub fn tty_fd() -> io::Result<FileDesc<'static>> {
use std::fs::File;

let stdin = rustix::stdio::stdin();
let fd = if rustix::termios::isatty(stdin) {
FileDesc::Static(stdin)
FileDesc::Borrowed(stdin)
} else {
let dev_tty = File::options().read(true).write(true).open("/dev/tty")?;
FileDesc::Owned(dev_tty.into())
Expand Down

0 comments on commit 62a0163

Please sign in to comment.