From 72df0c06bf70b28378739d8097d3b2ad9b3a553d Mon Sep 17 00:00:00 2001 From: sterlingjensen <5555776+sterlingjensen@users.noreply.github.com> Date: Thu, 5 Dec 2019 11:14:47 -0600 Subject: [PATCH] Remove unnecessary lifetimes from winpty --- alacritty_terminal/src/tty/windows/conpty.rs | 10 +- alacritty_terminal/src/tty/windows/mod.rs | 20 ++-- alacritty_terminal/src/tty/windows/winpty.rs | 22 ++-- winpty/src/windows.rs | 116 +++++++------------ 4 files changed, 64 insertions(+), 104 deletions(-) diff --git a/alacritty_terminal/src/tty/windows/conpty.rs b/alacritty_terminal/src/tty/windows/conpty.rs index f34ad92..31f4c25 100644 --- a/alacritty_terminal/src/tty/windows/conpty.rs +++ b/alacritty_terminal/src/tty/windows/conpty.rs @@ -98,11 +98,7 @@ impl Drop for Conpty { unsafe impl Send for Conpty {} unsafe impl Sync for Conpty {} -pub fn new<'a, C>( - config: &Config, - size: &SizeInfo, - _window_id: Option, -) -> Option> { +pub fn new(config: &Config, size: &SizeInfo, _window_id: Option) -> Option { if !config.enable_experimental_conpty_backend { return None; } @@ -132,7 +128,7 @@ pub fn new<'a, C>( ) }; - assert!(result == S_OK); + assert_eq!(result, S_OK); let mut success; @@ -266,7 +262,7 @@ impl OnResize for ConptyHandle { fn on_resize(&mut self, sizeinfo: &SizeInfo) { if let Some(coord) = coord_from_sizeinfo(sizeinfo) { let result = unsafe { (self.api.ResizePseudoConsole)(self.handle, coord) }; - assert!(result == S_OK); + assert_eq!(result, S_OK); } } } diff --git a/alacritty_terminal/src/tty/windows/mod.rs b/alacritty_terminal/src/tty/windows/mod.rs index 436b5ed..c9c0be7 100644 --- a/alacritty_terminal/src/tty/windows/mod.rs +++ b/alacritty_terminal/src/tty/windows/mod.rs @@ -39,13 +39,13 @@ pub fn is_conpty() -> bool { } #[derive(Clone)] -pub enum PtyHandle<'a> { - Winpty(winpty::WinptyHandle<'a>), +pub enum PtyHandle { + Winpty(winpty::WinptyHandle), Conpty(conpty::ConptyHandle), } -pub struct Pty<'a> { - handle: PtyHandle<'a>, +pub struct Pty { + handle: PtyHandle, // TODO: It's on the roadmap for the Conpty API to support Overlapped I/O. // See https://github.com/Microsoft/console/issues/262 // When support for that lands then it should be possible to use @@ -58,13 +58,13 @@ pub struct Pty<'a> { child_watcher: ChildExitWatcher, } -impl<'a> Pty<'a> { - pub fn resize_handle(&self) -> impl OnResize + 'a { +impl Pty { + pub fn resize_handle(&self) -> impl OnResize { self.handle.clone() } } -pub fn new<'a, C>(config: &Config, size: &SizeInfo, window_id: Option) -> Pty<'a> { +pub fn new(config: &Config, size: &SizeInfo, window_id: Option) -> Pty { if let Some(pty) = conpty::new(config, size, window_id) { info!("Using Conpty agent"); IS_CONPTY.store(true, Ordering::Relaxed); @@ -187,7 +187,7 @@ impl Write for EventedWritablePipe { } } -impl<'a> OnResize for PtyHandle<'a> { +impl OnResize for PtyHandle { fn on_resize(&mut self, sizeinfo: &SizeInfo) { match self { PtyHandle::Winpty(w) => w.resize(sizeinfo), @@ -199,7 +199,7 @@ impl<'a> OnResize for PtyHandle<'a> { } } -impl<'a> EventedReadWrite for Pty<'a> { +impl EventedReadWrite for Pty { type Reader = EventedReadablePipe; type Writer = EventedWritablePipe; @@ -293,7 +293,7 @@ impl<'a> EventedReadWrite for Pty<'a> { } } -impl<'a> EventedPty for Pty<'a> { +impl EventedPty for Pty { fn child_event_token(&self) -> mio::Token { self.child_event_token } diff --git a/alacritty_terminal/src/tty/windows/winpty.rs b/alacritty_terminal/src/tty/windows/winpty.rs index 258b7b2..032c339 100644 --- a/alacritty_terminal/src/tty/windows/winpty.rs +++ b/alacritty_terminal/src/tty/windows/winpty.rs @@ -34,24 +34,24 @@ use crate::tty::windows::Pty; // We store a raw pointer because we need mutable access to call // on_resize from a separate thread. Winpty internally uses a mutex // so this is safe, despite outwards appearance. -pub struct Agent<'a> { - winpty: *mut Winpty<'a>, +pub struct Agent { + winpty: *mut Winpty, } /// Handle can be cloned freely and moved between threads. -pub type WinptyHandle<'a> = Arc>; +pub type WinptyHandle = Arc; // Because Winpty has a mutex, we can do this. -unsafe impl<'a> Send for Agent<'a> {} -unsafe impl<'a> Sync for Agent<'a> {} +unsafe impl Send for Agent {} +unsafe impl Sync for Agent {} -impl<'a> Agent<'a> { - pub fn new(winpty: Winpty<'a>) -> Self { +impl Agent { + pub fn new(winpty: Winpty) -> Self { Self { winpty: Box::into_raw(Box::new(winpty)) } } /// Get immutable access to Winpty. - pub fn winpty(&self) -> &Winpty<'a> { + pub fn winpty(&self) -> &Winpty { unsafe { &*self.winpty } } @@ -63,7 +63,7 @@ impl<'a> Agent<'a> { } } -impl<'a> Drop for Agent<'a> { +impl Drop for Agent { fn drop(&mut self) { unsafe { Box::from_raw(self.winpty); @@ -75,7 +75,7 @@ impl<'a> Drop for Agent<'a> { /// This is a placeholder value until we see how often long responses happen const AGENT_TIMEOUT: u32 = 10000; -pub fn new<'a, C>(config: &Config, size: &SizeInfo, _window_id: Option) -> Pty<'a> { +pub fn new(config: &Config, size: &SizeInfo, _window_id: Option) -> Pty { // Create config let mut wconfig = WinptyConfig::new(ConfigFlags::empty()).unwrap(); @@ -150,7 +150,7 @@ pub fn new<'a, C>(config: &Config, size: &SizeInfo, _window_id: Option } } -impl<'a> OnResize for Winpty<'a> { +impl OnResize for Winpty { fn on_resize(&mut self, sizeinfo: &SizeInfo) { let (cols, lines) = (sizeinfo.cols().0, sizeinfo.lines().0); if cols > 0 && cols <= u16::MAX as usize && lines > 0 && lines <= u16::MAX as usize { diff --git a/winpty/src/windows.rs b/winpty/src/windows.rs index ada8bb0..27780d4 100644 --- a/winpty/src/windows.rs +++ b/winpty/src/windows.rs @@ -20,17 +20,20 @@ pub enum ErrorCodes { AgentTimeout, AgentCreationFailed, } + pub enum MouseMode { None, Auto, Force, } + bitflags!( pub struct SpawnFlags: u64 { const AUTO_SHUTDOWN = 0x1; const EXIT_AFTER_SHUTDOWN = 0x2; } ); + bitflags!( pub struct ConfigFlags: u64 { const CONERR = 0x1; @@ -40,42 +43,32 @@ bitflags!( ); #[derive(Debug)] -pub struct Err<'a> { - ptr: &'a mut winpty_error_t, +pub struct Err { code: u32, message: String, } -// Check to see whether winpty gave us an error -fn check_err<'a>(e: *mut winpty_error_t) -> Option> { - let err = unsafe { +// Check to see whether winpty gave us an error, and perform the necessary memory freeing +fn check_err(e: *mut winpty_error_t) -> Option { + unsafe { + let code = winpty_error_code(e); let raw = winpty_error_msg(e); - Err { - ptr: &mut *e, - code: winpty_error_code(e), - message: String::from_utf16_lossy(std::slice::from_raw_parts(raw, wcslen(raw))), + let message = String::from_utf16_lossy(std::slice::from_raw_parts(raw, wcslen(raw))); + winpty_error_free(e); + match code { + 0 => None, + _ => Some(Err { code, message }), } - }; - if err.code == 0 { - None - } else { - Some(err) } } -impl<'a> Drop for Err<'a> { - fn drop(&mut self) { - unsafe { - winpty_error_free(self.ptr); - } - } -} -impl<'a> Display for Err<'a> { +impl Display for Err { fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { write!(f, "Code: {}, Message: {}", self.code, self.message) } } -impl<'a> Error for Err<'a> { + +impl Error for Err { fn description(&self) -> &str { &self.message } @@ -83,18 +76,13 @@ impl<'a> Error for Err<'a> { #[derive(Debug)] /// Winpty agent config -pub struct Config<'a>(&'a mut winpty_config_t); +pub struct Config(*mut winpty_config_t); -impl<'a, 'b> Config<'a> { - pub fn new(flags: ConfigFlags) -> Result> { +impl Config { + pub fn new(flags: ConfigFlags) -> Result { let mut err = null_mut() as *mut winpty_error_t; let config = unsafe { winpty_config_new(flags.bits(), &mut err) }; - - if let Some(err) = check_err(err) { - Result::Err(err) - } else { - unsafe { Ok(Config(&mut *config)) } - } + check_err(err).map_or(Ok(Self(config)), Result::Err) } /// Set the initial size of the console window @@ -127,7 +115,7 @@ impl<'a, 'b> Config<'a> { } } -impl<'a> Drop for Config<'a> { +impl Drop for Config { fn drop(&mut self) { unsafe { winpty_config_free(self.0); @@ -137,23 +125,16 @@ impl<'a> Drop for Config<'a> { #[derive(Debug)] /// A struct representing the winpty agent process -pub struct Winpty<'a>(&'a mut winpty_t); +pub struct Winpty(*mut winpty_t); -impl<'a, 'b> Winpty<'a> { +impl Winpty { /// Starts the agent. This process will connect to the agent /// over a control pipe, and the agent will open data pipes /// (e.g. CONIN and CONOUT). - pub fn open(cfg: &Config) -> Result> { + pub fn open(cfg: &Config) -> Result { let mut err = null_mut() as *mut winpty_error_t; - unsafe { - let winpty = winpty_open(cfg.0, &mut err); - let err = check_err(err); - if let Some(err) = err { - Result::Err(err) - } else { - Ok(Winpty(&mut *winpty)) - } - } + let winpty = unsafe { winpty_open(cfg.0, &mut err) }; + check_err(err).map_or(Ok(Self(winpty)), Result::Err) } /// Returns the handle to the winpty agent process @@ -200,11 +181,7 @@ impl<'a, 'b> Winpty<'a> { winpty_set_size(self.0, i32::from(cols), i32::from(rows), &mut err); } - if let Some(err) = check_err(err) { - Result::Err(err) - } else { - Ok(()) - } + check_err(err).map_or(Ok(()), Result::Err) } /// Get the list of processes running in the winpty agent. Returns <= count processes @@ -227,11 +204,7 @@ impl<'a, 'b> Winpty<'a> { process_list.set_len(len); } - if let Some(err) = check_err(err) { - Result::Err(err) - } else { - Ok(process_list) - } + check_err(err).map_or(Ok(process_list), Result::Err) } /// Spawns the new process. @@ -259,19 +232,15 @@ impl<'a, 'b> Winpty<'a> { } } - if let Some(err) = check_err(err) { - Result::Err(err) - } else { - Ok(()) - } + check_err(err).map_or(Ok(()), Result::Err) } } // winpty_t is thread-safe -unsafe impl<'a> Sync for Winpty<'a> {} -unsafe impl<'a> Send for Winpty<'a> {} +unsafe impl Sync for Winpty {} +unsafe impl Send for Winpty {} -impl<'a> Drop for Winpty<'a> { +impl Drop for Winpty { fn drop(&mut self) { unsafe { winpty_free(self.0); @@ -281,9 +250,9 @@ impl<'a> Drop for Winpty<'a> { #[derive(Debug)] /// Information about a process for winpty to spawn -pub struct SpawnConfig<'a>(&'a mut winpty_spawn_config_t); +pub struct SpawnConfig(*mut winpty_spawn_config_t); -impl<'a, 'b> SpawnConfig<'a> { +impl SpawnConfig { /// Creates a new spawnconfig pub fn new( spawnflags: SpawnFlags, @@ -291,7 +260,7 @@ impl<'a, 'b> SpawnConfig<'a> { cmdline: Option<&str>, cwd: Option<&str>, end: Option<&str>, - ) -> Result> { + ) -> Result { let mut err = null_mut() as *mut winpty_error_t; let (appname, cmdline, cwd, end) = ( appname.map_or(null(), |s| WideCString::from_str(s).unwrap().into_raw()), @@ -320,14 +289,11 @@ impl<'a, 'b> SpawnConfig<'a> { } } - if let Some(err) = check_err(err) { - Result::Err(err) - } else { - unsafe { Ok(SpawnConfig(&mut *spawn_config)) } - } + check_err(err).map_or(Ok(Self(spawn_config)), Result::Err) } } -impl<'a> Drop for SpawnConfig<'a> { + +impl Drop for SpawnConfig { fn drop(&mut self) { unsafe { winpty_spawn_config_free(self.0); @@ -338,10 +304,8 @@ impl<'a> Drop for SpawnConfig<'a> { #[cfg(test)] mod tests { use named_pipe::PipeClient; - use winapi; - - use self::winapi::um::processthreadsapi::OpenProcess; - use self::winapi::um::winnt::READ_CONTROL; + use winapi::um::processthreadsapi::OpenProcess; + use winapi::um::winnt::READ_CONTROL; use crate::{Config, ConfigFlags, SpawnConfig, SpawnFlags, Winpty};