From fb827b5b4760350a1b98b4517b1d1d9f11dbbc0e Mon Sep 17 00:00:00 2001 From: Ron Williams Date: Sat, 26 Aug 2023 19:49:42 -0700 Subject: [PATCH] general clean-up, plus fix to ensure an event after read if data is still pending --- Cargo.toml | 1 + src/{master.rs => controlterm.rs} | 37 +++++++------ src/main.rs | 86 +++++++++++++++++-------------- src/pgrp.rs | 24 ++++----- src/pty.rs | 10 ++-- src/resource.rs | 12 ++--- src/scheme.rs | 34 ++++++------ src/{slave.rs => subterm.rs} | 37 +++++++------ src/termios.rs | 18 +++---- src/winsize.rs | 18 +++---- 10 files changed, 146 insertions(+), 131 deletions(-) rename src/{master.rs => controlterm.rs} (80%) rename src/{slave.rs => subterm.rs} (82%) diff --git a/Cargo.toml b/Cargo.toml index 8ea05037f8..1b99478e66 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [package] name = "ptyd" version = "0.1.0" +edition = "2021" [dependencies] redox-daemon = "0.1" diff --git a/src/master.rs b/src/controlterm.rs similarity index 80% rename from src/master.rs rename to src/controlterm.rs index 7ceee9addf..2ef8290525 100644 --- a/src/master.rs +++ b/src/controlterm.rs @@ -1,33 +1,33 @@ use std::cell::RefCell; use std::rc::{Rc, Weak}; -use syscall::error::{Error, Result, EINVAL, EAGAIN}; +use syscall::error::{Error, Result, EAGAIN, EINVAL}; use syscall::flag::{EventFlags, F_GETFL, F_SETFL, O_ACCMODE, O_NONBLOCK}; -use pty::Pty; -use resource::Resource; +use crate::pty::Pty; +use crate::resource::Resource; /// Read side of a pipe #[derive(Clone)] -pub struct PtyMaster { +pub struct PtyControlTerm { pty: Rc>, flags: usize, notified_read: bool, - notified_write: bool + notified_write: bool, } -impl PtyMaster { +impl PtyControlTerm { pub fn new(pty: Rc>, flags: usize) -> Self { - PtyMaster { + PtyControlTerm { pty: pty, flags: flags, notified_read: false, - notified_write: false + notified_write: false, } } } -impl Resource for PtyMaster { +impl Resource for PtyControlTerm { fn boxed_clone(&self) -> Box { Box::new(self.clone()) } @@ -40,11 +40,14 @@ impl Resource for PtyMaster { self.flags } - fn path(&self, buf: &mut [u8]) -> Result { + fn path(&mut self, buf: &mut [u8]) -> Result { self.pty.borrow_mut().path(buf) } - fn read(&self, buf: &mut [u8]) -> Result> { + fn read(&mut self, buf: &mut [u8]) -> Result> { + + self.notified_read = false; + let mut pty = self.pty.borrow_mut(); if let Some(packet) = pty.miso.pop_front() { @@ -71,7 +74,7 @@ impl Resource for PtyMaster { } } - fn write(&self, buf: &[u8]) -> Result> { + fn write(&mut self, buf: &[u8]) -> Result> { let mut pty = self.pty.borrow_mut(); if pty.mosi.len() >= 64 { @@ -83,7 +86,7 @@ impl Resource for PtyMaster { Ok(Some(buf.len())) } - fn sync(&self) -> Result { + fn sync(&mut self) -> Result { Ok(0) } @@ -91,10 +94,10 @@ impl Resource for PtyMaster { match cmd { F_GETFL => Ok(self.flags), F_SETFL => { - self.flags = (self.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + self.flags = (self.flags & O_ACCMODE) | (arg & !O_ACCMODE); Ok(0) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } } @@ -117,7 +120,7 @@ impl Resource for PtyMaster { self.notified_read = false; } - if ! self.notified_write { + if !self.notified_write { self.notified_write = true; events |= syscall::EVENT_WRITE; } diff --git a/src/main.rs b/src/main.rs index 59581c43de..70310c7daf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,27 +1,24 @@ -extern crate redox_termios; -extern crate syscall; - use std::fs::{File, OpenOptions}; use std::io::{self, Read, Write}; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::AsRawFd; use syscall::data::{Event, Packet, TimeSpec}; -use syscall::flag::{CloneFlags, EventFlags}; +use syscall::flag::EventFlags; use syscall::scheme::SchemeBlockMut; -mod master; +mod controlterm; mod pgrp; mod pty; mod resource; mod scheme; -mod slave; +mod subterm; mod termios; mod winsize; use scheme::PtyScheme; -fn main(){ +fn main() { redox_daemon::Daemon::new(move |daemon| { let mut event_file = OpenOptions::new() .read(true) @@ -49,47 +46,53 @@ fn main(){ daemon.ready().expect("pty: failed to notify parent"); - event_file.write(&Event { - id: socket.as_raw_fd() as usize, - flags: syscall::EVENT_READ, - data: 1, - }).expect("pty: failed to watch events on pty:"); + event_file + .write(&Event { + id: socket.as_raw_fd() as usize, + flags: syscall::EVENT_READ, + data: 1, + }) + .expect("pty: failed to watch events on pty:"); - event_file.write(&Event { - id: time_file.as_raw_fd() as usize, - flags: syscall::EVENT_READ, - data: 2, - }).expect("pty: failed to watch events on time:"); + event_file + .write(&Event { + id: time_file.as_raw_fd() as usize, + flags: syscall::EVENT_READ, + data: 2, + }) + .expect("pty: failed to watch events on time:"); //TODO: do not set timeout if not necessary - timeout(&mut time_file) - .expect("pty: failed to set timeout"); + timeout(&mut time_file).expect("pty: failed to set timeout"); let mut scheme = PtyScheme::new(); let mut todo = Vec::new(); let mut timeout_count = 0u64; loop { let mut event = Event::default(); - event_file.read(&mut event) + event_file + .read(&mut event) .expect("pty: failed to read event:"); match event.data { 1 => { let mut packet = Packet::default(); - socket.read(&mut packet).expect("pty: failed to read events from pty scheme"); - + socket + .read(&mut packet) + .expect("pty: failed to read events from pty scheme"); if let Some(a) = scheme.handle(&mut packet) { packet.a = a; - socket.write(&packet).expect("pty: failed to write responses to pty scheme"); + socket + .write(&packet) + .expect("pty: failed to write responses to pty scheme"); } else { todo.push(packet); } - }, + } 2 => { - timeout(&mut time_file) - .expect("pty: failed to set timeout"); + timeout(&mut time_file).expect("pty: failed to set timeout"); - timeout_count.wrapping_add(1); + timeout_count = timeout_count.wrapping_add(1); for (_id, handle) in scheme.handles.iter_mut() { handle.timeout(timeout_count); @@ -103,7 +106,9 @@ fn main(){ if let Some(a) = scheme.handle(&mut todo[i]) { let mut packet = todo.remove(i); packet.a = a; - socket.write(&packet).expect("pty: failed to write responses to pty scheme"); + socket + .write(&packet) + .expect("pty: failed to write responses to pty scheme"); } else { i += 1; } @@ -116,7 +121,8 @@ fn main(){ } } } - }).expect("pty: failed to daemonize"); + }) + .expect("pty: failed to daemonize"); } fn timeout(time_file: &mut File) -> io::Result<()> { @@ -133,14 +139,16 @@ fn timeout(time_file: &mut File) -> io::Result<()> { } fn post_fevent(socket: &mut File, id: usize, flags: EventFlags, count: usize) { - socket.write(&Packet { - id: 0, - pid: 0, - uid: 0, - gid: 0, - a: syscall::number::SYS_FEVENT, - b: id, - c: flags.bits(), - d: count - }).expect("pty: failed to write event"); + socket + .write(&Packet { + id: 0, + pid: 0, + uid: 0, + gid: 0, + a: syscall::number::SYS_FEVENT, + b: id, + c: flags.bits(), + d: count, + }) + .expect("pty: failed to write event"); } diff --git a/src/pgrp.rs b/src/pgrp.rs index 336f94c9da..a41f6c5272 100644 --- a/src/pgrp.rs +++ b/src/pgrp.rs @@ -1,12 +1,12 @@ -use std::{mem, slice}; use std::cell::RefCell; use std::rc::Weak; +use std::{mem, slice}; use syscall::error::{Error, Result, EBADF, EINVAL, EPIPE}; use syscall::flag::{EventFlags, F_GETFL, F_SETFL, O_ACCMODE}; -use pty::Pty; -use resource::Resource; +use crate::pty::Pty; +use crate::resource::Resource; /// Read side of a pipe #[derive(Clone)] @@ -37,7 +37,7 @@ impl Resource for PtyPgrp { self.flags } - fn path(&self, buf: &mut [u8]) -> Result { + fn path(&mut self, buf: &mut [u8]) -> Result { if let Some(pty_lock) = self.pty.upgrade() { pty_lock.borrow_mut().path(buf) } else { @@ -45,13 +45,13 @@ impl Resource for PtyPgrp { } } - fn read(&self, buf: &mut [u8]) -> Result> { + fn read(&mut self, buf: &mut [u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let pty = pty_lock.borrow(); let pgrp: &[u8] = unsafe { slice::from_raw_parts( &pty.pgrp as *const usize as *const u8, - mem::size_of::() + mem::size_of::(), ) }; @@ -66,13 +66,13 @@ impl Resource for PtyPgrp { } } - fn write(&self, buf: &[u8]) -> Result> { + fn write(&mut self, buf: &[u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); let pgrp: &mut [u8] = unsafe { slice::from_raw_parts_mut( &mut pty.pgrp as *mut usize as *mut u8, - mem::size_of::() + mem::size_of::(), ) }; @@ -87,7 +87,7 @@ impl Resource for PtyPgrp { } } - fn sync(&self) -> Result { + fn sync(&mut self) -> Result { Ok(0) } @@ -95,10 +95,10 @@ impl Resource for PtyPgrp { match cmd { F_GETFL => Ok(self.flags), F_SETFL => { - self.flags = (self.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + self.flags = (self.flags & O_ACCMODE) | (arg & !O_ACCMODE); Ok(0) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } } diff --git a/src/pty.rs b/src/pty.rs index c863c59bbc..b659cf2b88 100644 --- a/src/pty.rs +++ b/src/pty.rs @@ -51,9 +51,7 @@ impl Pty { let lfl = self.termios.c_lflag; let cc = self.termios.c_cc; - let is_cc = |b: u8, i: usize| -> bool { - b != 0 && b == cc[i] - }; + let is_cc = |b: u8, i: usize| -> bool { b != 0 && b == cc[i] }; let inlcr = ifl & INLCR == INLCR; let igncr = ifl & IGNCR == IGNCR; @@ -246,7 +244,7 @@ impl Pty { let vtime = cc[VTIME] as u64; // http://unixwiz.net/techtips/termios-vmin-vtime.html - if ! icanon { + if !icanon { if vtime == 0 { // No timeout specified if vmin == 0 { @@ -266,7 +264,7 @@ impl Pty { // Timeout specified using vtime if vmin == 0 { // Return when any data is available or the timer expires - if ! self.cooked.is_empty() { + if !self.cooked.is_empty() { self.mosi.push_back(self.cooked.clone()); self.cooked.clear(); } else { @@ -290,7 +288,7 @@ impl Pty { if self.cooked.len() >= vmin { self.mosi.push_back(self.cooked.clone()); self.cooked.clear(); - } else if ! self.cooked.is_empty() { + } else if !self.cooked.is_empty() { if let Some(timeout_character) = self.timeout_character { if self.timeout_count >= timeout_character.wrapping_add(vtime) { self.timeout_character = None; diff --git a/src/resource.rs b/src/resource.rs index 82c037251b..7ef300dbf8 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -4,21 +4,21 @@ use std::rc::Weak; use syscall::error::Result; use syscall::flag::EventFlags; -use pty::Pty; +use crate::pty::Pty; pub trait Resource { fn boxed_clone(&self) -> Box; fn pty(&self) -> Weak>; fn flags(&self) -> usize; - fn path(&self, buf: &mut [u8]) -> Result; - fn read(&self, buf: &mut [u8]) -> Result>; - fn write(&self, buf: &[u8]) -> Result>; - fn sync(&self) -> Result; + fn path(&mut self, buf: &mut [u8]) -> Result; + fn read(&mut self, buf: &mut [u8]) -> Result>; + fn write(&mut self, buf: &[u8]) -> Result>; + fn sync(&mut self) -> Result; fn fcntl(&mut self, cmd: usize, arg: usize) -> Result; fn fevent(&mut self) -> Result; fn events(&mut self) -> EventFlags; fn timeout(&self, _count: u64) { - // Handled only by PTY master + // Handled only by PTY control term } } diff --git a/src/scheme.rs b/src/scheme.rs index 2b936b4fb1..7bedd7882b 100644 --- a/src/scheme.rs +++ b/src/scheme.rs @@ -1,5 +1,5 @@ -use std::collections::BTreeMap; use std::cell::RefCell; +use std::collections::BTreeMap; use std::rc::Rc; use std::str; @@ -8,13 +8,13 @@ use syscall::error::{Error, Result, EBADF, EINVAL, ENOENT}; use syscall::flag::{EventFlags, MODE_CHR}; use syscall::scheme::SchemeBlockMut; -use master::PtyMaster; -use pgrp::PtyPgrp; -use pty::Pty; -use resource::Resource; -use slave::PtySlave; -use termios::PtyTermios; -use winsize::PtyWinsize; +use crate::controlterm::PtyControlTerm; +use crate::pgrp::PtyPgrp; +use crate::pty::Pty; +use crate::resource::Resource; +use crate::subterm::PtySubTerm; +use crate::termios::PtyTermios; +use crate::winsize::PtyWinsize; pub struct PtyScheme { next_id: usize, @@ -39,20 +39,22 @@ impl SchemeBlockMut for PtyScheme { self.next_id += 1; let pty = Rc::new(RefCell::new(Pty::new(id))); - self.handles.insert(id, Box::new(PtyMaster::new(pty, flags))); + self.handles + .insert(id, Box::new(PtyControlTerm::new(pty, flags))); Ok(Some(id)) } else { - let master_id = path.parse::().or(Err(Error::new(EINVAL)))?; + let control_term_id = path.parse::().or(Err(Error::new(EINVAL)))?; let pty = { - let handle = self.handles.get(&master_id).ok_or(Error::new(ENOENT))?; + let handle = self.handles.get(&control_term_id).ok_or(Error::new(ENOENT))?; handle.pty() }; let id = self.next_id; self.next_id += 1; - self.handles.insert(id, Box::new(PtySlave::new(pty, flags))); + self.handles + .insert(id, Box::new(PtySubTerm::new(pty, flags))); Ok(Some(id)) } @@ -83,12 +85,12 @@ impl SchemeBlockMut for PtyScheme { } fn read(&mut self, id: usize, buf: &mut [u8]) -> Result> { - let handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; + let handle = self.handles.get_mut(&id).ok_or(Error::new(EBADF))?; handle.read(buf) } fn write(&mut self, id: usize, buf: &[u8]) -> Result> { - let handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; + let handle = self.handles.get_mut(&id).ok_or(Error::new(EBADF))?; handle.write(buf) } @@ -103,7 +105,7 @@ impl SchemeBlockMut for PtyScheme { } fn fpath(&mut self, id: usize, buf: &mut [u8]) -> Result> { - let handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; + let handle = self.handles.get_mut(&id).ok_or(Error::new(EBADF))?; handle.path(buf).map(Some) } @@ -119,7 +121,7 @@ impl SchemeBlockMut for PtyScheme { } fn fsync(&mut self, id: usize) -> Result> { - let handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; + let handle = self.handles.get_mut(&id).ok_or(Error::new(EBADF))?; handle.sync().map(Some) } diff --git a/src/slave.rs b/src/subterm.rs similarity index 82% rename from src/slave.rs rename to src/subterm.rs index 3698e74462..d6202c8fb0 100644 --- a/src/slave.rs +++ b/src/subterm.rs @@ -1,33 +1,33 @@ use std::cell::RefCell; use std::rc::Weak; -use syscall::error::{Error, Result, EINVAL, EPIPE, EAGAIN}; +use syscall::error::{Error, Result, EAGAIN, EINVAL, EPIPE}; use syscall::flag::{EventFlags, F_GETFL, F_SETFL, O_ACCMODE, O_NONBLOCK}; -use pty::Pty; -use resource::Resource; +use crate::pty::Pty; +use crate::resource::Resource; /// Read side of a pipe #[derive(Clone)] -pub struct PtySlave { +pub struct PtySubTerm { pty: Weak>, flags: usize, notified_read: bool, - notified_write: bool + notified_write: bool, } -impl PtySlave { +impl PtySubTerm { pub fn new(pty: Weak>, flags: usize) -> Self { - PtySlave { + PtySubTerm { pty: pty, flags: flags, notified_read: false, - notified_write: false + notified_write: false, } } } -impl Resource for PtySlave { +impl Resource for PtySubTerm { fn boxed_clone(&self) -> Box { Box::new(self.clone()) } @@ -40,7 +40,7 @@ impl Resource for PtySlave { self.flags } - fn path(&self, buf: &mut [u8]) -> Result { + fn path(&mut self, buf: &mut [u8]) -> Result { if let Some(pty_lock) = self.pty.upgrade() { pty_lock.borrow_mut().path(buf) } else { @@ -48,7 +48,10 @@ impl Resource for PtySlave { } } - fn read(&self, buf: &mut [u8]) -> Result> { + fn read(&mut self, buf: &mut [u8]) -> Result> { + + self.notified_read = false; + if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); @@ -77,7 +80,7 @@ impl Resource for PtySlave { } } - fn write(&self, buf: &[u8]) -> Result> { + fn write(&mut self, buf: &[u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); @@ -93,7 +96,7 @@ impl Resource for PtySlave { } } - fn sync(&self) -> Result { + fn sync(&mut self) -> Result { if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); @@ -109,10 +112,10 @@ impl Resource for PtySlave { match cmd { F_GETFL => Ok(self.flags), F_SETFL => { - self.flags = (self.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + self.flags = (self.flags & O_ACCMODE) | (arg & !O_ACCMODE); Ok(0) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } } @@ -137,7 +140,7 @@ impl Resource for PtySlave { } } - if ! self.notified_write { + if !self.notified_write { self.notified_write = true; events |= syscall::EVENT_WRITE; } diff --git a/src/termios.rs b/src/termios.rs index 0672e9b007..e71337f65d 100644 --- a/src/termios.rs +++ b/src/termios.rs @@ -5,8 +5,8 @@ use std::rc::Weak; use syscall::error::{Error, Result, EBADF, EINVAL, EPIPE}; use syscall::flag::{EventFlags, F_GETFL, F_SETFL, O_ACCMODE}; -use pty::Pty; -use resource::Resource; +use crate::pty::Pty; +use crate::resource::Resource; /// Read side of a pipe #[derive(Clone)] @@ -37,7 +37,7 @@ impl Resource for PtyTermios { self.flags } - fn path(&self, buf: &mut [u8]) -> Result { + fn path(&mut self, buf: &mut [u8]) -> Result { if let Some(pty_lock) = self.pty.upgrade() { pty_lock.borrow_mut().path(buf) } else { @@ -45,7 +45,7 @@ impl Resource for PtyTermios { } } - fn read(&self, buf: &mut [u8]) -> Result> { + fn read(&mut self, buf: &mut [u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let pty = pty_lock.borrow(); let termios: &[u8] = pty.termios.deref(); @@ -61,7 +61,7 @@ impl Resource for PtyTermios { } } - fn write(&self, buf: &[u8]) -> Result> { + fn write(&mut self, buf: &[u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); let termios: &mut [u8] = pty.termios.deref_mut(); @@ -77,7 +77,7 @@ impl Resource for PtyTermios { } } - fn sync(&self) -> Result { + fn sync(&mut self) -> Result { Ok(0) } @@ -85,10 +85,10 @@ impl Resource for PtyTermios { match cmd { F_GETFL => Ok(self.flags), F_SETFL => { - self.flags = (self.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + self.flags = (self.flags & O_ACCMODE) | (arg & !O_ACCMODE); Ok(0) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } } diff --git a/src/winsize.rs b/src/winsize.rs index 9c255d8eff..33942ebb82 100644 --- a/src/winsize.rs +++ b/src/winsize.rs @@ -5,8 +5,8 @@ use std::rc::Weak; use syscall::error::{Error, Result, EBADF, EINVAL, EPIPE}; use syscall::flag::{EventFlags, F_GETFL, F_SETFL, O_ACCMODE}; -use pty::Pty; -use resource::Resource; +use crate::pty::Pty; +use crate::resource::Resource; /// Read side of a pipe #[derive(Clone)] @@ -37,7 +37,7 @@ impl Resource for PtyWinsize { self.flags } - fn path(&self, buf: &mut [u8]) -> Result { + fn path(&mut self, buf: &mut [u8]) -> Result { if let Some(pty_lock) = self.pty.upgrade() { pty_lock.borrow_mut().path(buf) } else { @@ -45,7 +45,7 @@ impl Resource for PtyWinsize { } } - fn read(&self, buf: &mut [u8]) -> Result> { + fn read(&mut self, buf: &mut [u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let pty = pty_lock.borrow(); let winsize: &[u8] = pty.winsize.deref(); @@ -61,7 +61,7 @@ impl Resource for PtyWinsize { } } - fn write(&self, buf: &[u8]) -> Result> { + fn write(&mut self, buf: &[u8]) -> Result> { if let Some(pty_lock) = self.pty.upgrade() { let mut pty = pty_lock.borrow_mut(); let winsize: &mut [u8] = pty.winsize.deref_mut(); @@ -77,7 +77,7 @@ impl Resource for PtyWinsize { } } - fn sync(&self) -> Result { + fn sync(&mut self) -> Result { Ok(0) } @@ -85,10 +85,10 @@ impl Resource for PtyWinsize { match cmd { F_GETFL => Ok(self.flags), F_SETFL => { - self.flags = (self.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + self.flags = (self.flags & O_ACCMODE) | (arg & !O_ACCMODE); Ok(0) - }, - _ => Err(Error::new(EINVAL)) + } + _ => Err(Error::new(EINVAL)), } }