From 52c5058dbf72a10a395bb0c8744f71bade75202c Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 00:38:50 -0700 Subject: [PATCH 1/9] Fix soundness issues and resource leaks - The close methods were incorrect, replaced with impl Drop - Dropping Connection would invalidate Blocking, so tied the lifetimes together to prevent further soundness issues (see required changes for doctests). --- src/telemetry.rs | 103 +++++++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 48 deletions(-) diff --git a/src/telemetry.rs b/src/telemetry.rs index ad97fb8..6de94ee 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -8,6 +8,7 @@ use std::error::Error; use std::ffi::CStr; use std::fmt::{self, Display}; use std::io::Result as IOResult; +use std::marker::PhantomData; use std::mem::transmute; use std::os::raw::{c_char, c_void}; use std::os::windows::raw::HANDLE; @@ -16,9 +17,9 @@ use std::time::Duration; use winapi::shared::minwindef::LPVOID; use winapi::um::errhandlingapi::GetLastError; use winapi::um::handleapi::CloseHandle; -use winapi::um::memoryapi::{MapViewOfFile, OpenFileMappingW, FILE_MAP_READ}; -use winapi::um::minwinbase::LPSECURITY_ATTRIBUTES; -use winapi::um::synchapi::{CreateEventW, ResetEvent, WaitForSingleObject}; +use winapi::um::memoryapi::{MapViewOfFile, OpenFileMappingW, UnmapViewOfFile, FILE_MAP_READ}; +use winapi::um::synchapi::{OpenEventW, ResetEvent, WaitForSingleObject}; +use winapi::um::winnt::SYNCHRONIZE; /// System path where the shared memory map is located. pub const TELEMETRY_PATH: &str = r"Local\IRSDKMemMapFileName"; @@ -55,10 +56,11 @@ pub struct Header { /// /// Calling `sample()` on a Blocking interface will block until a new telemetry sample is made available. /// -pub struct Blocking { +pub struct Blocking<'conn> { origin: *const c_void, header: Header, event_handle: HANDLE, + phantom: PhantomData<&'conn Connection>, } #[derive(Copy, Clone, Debug)] @@ -120,7 +122,8 @@ pub struct Sample { /// # fn main() -> Result<(), Box> { /// # use iracing::telemetry::{Connection, Value}; /// # use std::time::Duration; -/// # let sampler = Connection::new()?.blocking()?; +/// # let conn = Connection::new()?; +/// # let sampler = conn.blocking()?; /// # let sample = sampler.sample(Duration::from_millis(50))?; /// use std::convert::TryInto; /// @@ -135,7 +138,8 @@ pub struct Sample { /// # fn main() -> Result<(), Box> { /// # use iracing::telemetry::{Connection, Value}; /// # use std::time::Duration; -/// # let sampler = Connection::new()?.blocking()?; +/// # let conn = Connection::new()?; +/// # let sampler = conn.blocking()?; /// # let sample = sampler.sample(Duration::from_millis(50))?; /// match sample.get("some_key") { /// Err(err) => println!("Didn't find that value: {}", err), @@ -515,14 +519,12 @@ impl Display for TelemetryError { impl Error for TelemetryError {} -impl Blocking { +impl<'conn> Blocking<'conn> { pub fn new(location: *const c_void, head: Header) -> std::io::Result { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); - let sc: LPSECURITY_ATTRIBUTES = unsafe { std::mem::zeroed() }; - - let handle: HANDLE = unsafe { CreateEventW(sc, 0, 0, event_name.as_ptr()) }; + let handle: HANDLE = unsafe { OpenEventW(SYNCHRONIZE, 0, event_name.as_ptr()) }; if handle.is_null() { let errno: i32 = unsafe { GetLastError() as i32 }; @@ -534,37 +536,10 @@ impl Blocking { origin: location, header: head, event_handle: handle, + phantom: PhantomData, }) } - pub fn close(&self) -> std::io::Result<()> { - if self.event_handle.is_null() { - return Ok(()); - } - - let succ = unsafe { CloseHandle(self.event_handle) }; - - if succ == 0 { - let err: i32 = unsafe { GetLastError() as i32 }; - - return Err(std::io::Error::from_raw_os_error(err)); - } - - if self.origin.is_null() { - return Ok(()); - } - - let succ = unsafe { CloseHandle(self.origin as HANDLE) }; - - if succ == 0 { - let err: i32 = unsafe { GetLastError() as i32 }; - - Err(std::io::Error::from_raw_os_error(err)) - } else { - Ok(()) - } - } - /// /// Sample Telemetry Data /// @@ -578,7 +553,8 @@ impl Blocking { /// use iracing::telemetry::Connection; /// use std::time::Duration; /// - /// let sampler = Connection::new()?.blocking()?; + /// let conn = Connection::new()?; + /// let sampler = conn.blocking()?; /// let sample = sampler.sample(Duration::from_millis(50))?; /// # Ok(()) /// # } @@ -609,6 +585,20 @@ impl Blocking { } } +impl<'conn> Drop for Blocking<'conn> { + fn drop(&mut self) { + let succ = unsafe { CloseHandle(self.event_handle) }; + + if succ == 0 { + let errno: i32 = unsafe { GetLastError() as i32 }; + panic!( + "Unable to close handle: {}", + std::io::Error::from_raw_os_error(errno) + ); + } + } +} + /// /// iRacing live telemetry and session data connection. /// @@ -624,6 +614,7 @@ impl Blocking { /// let _ = Connection::new().expect("Unable to find telemetry data"); /// ``` pub struct Connection { + mapping: HANDLE, location: *mut c_void, } @@ -661,7 +652,10 @@ impl Connection { return Err(std::io::Error::from_raw_os_error(errno)); } - Ok(Connection { location: view }) + Ok(Connection { + mapping, + location: view, + }) } /// @@ -738,24 +732,37 @@ impl Connection { /// use iracing::telemetry::Connection; /// use std::time::Duration; /// - /// let sampler = Connection::new()?.blocking()?; + /// let conn = Connection::new()?; + /// let sampler = conn.blocking()?; /// let sample = sampler.sample(Duration::from_millis(50))?; /// # Ok(()) /// # } /// ``` - pub fn blocking(&self) -> IOResult { + pub fn blocking(&self) -> IOResult> { Blocking::new(self.location, unsafe { Self::read_header(self.location) }) } +} + +impl Drop for Connection { + fn drop(&mut self) { + let succ = unsafe { UnmapViewOfFile(self.location) }; + if succ == 0 { + let errno: i32 = unsafe { GetLastError() as i32 }; - pub fn close(&self) -> IOResult<()> { - let succ = unsafe { CloseHandle(self.location) }; + panic!( + "Unable to unmap file: {}", + std::io::Error::from_raw_os_error(errno) + ); + } - if succ != 0 { - Ok(()) - } else { + let succ = unsafe { CloseHandle(self.mapping) }; + if succ == 0 { let errno: i32 = unsafe { GetLastError() as i32 }; - Err(std::io::Error::from_raw_os_error(errno)) + panic!( + "Unable to close handle: {}", + std::io::Error::from_raw_os_error(errno) + ); } } } From 608a1a0bc3f3c18ec6fcc281b6c9c2ad83fcf95f Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 00:53:23 -0700 Subject: [PATCH 2/9] Reduce unsafe keyword uses - This extends the surface area of unsafe slightly. --- src/telemetry.rs | 146 ++++++++++++++++++++++------------------------- 1 file changed, 68 insertions(+), 78 deletions(-) diff --git a/src/telemetry.rs b/src/telemetry.rs index 6de94ee..37bdee1 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -524,13 +524,16 @@ impl<'conn> Blocking<'conn> { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); - let handle: HANDLE = unsafe { OpenEventW(SYNCHRONIZE, 0, event_name.as_ptr()) }; + let handle = unsafe { + let handle: HANDLE = OpenEventW(SYNCHRONIZE, 0, event_name.as_ptr()); - if handle.is_null() { - let errno: i32 = unsafe { GetLastError() as i32 }; + if handle.is_null() { + let errno = GetLastError() as i32; + return Err(std::io::Error::from_raw_os_error(errno)); + } - return Err(std::io::Error::from_raw_os_error(errno)); - } + handle + }; Ok(Blocking { origin: location, @@ -565,36 +568,39 @@ impl<'conn> Blocking<'conn> { Err(e) => return Err(Box::new(e)), }; - let signal = unsafe { WaitForSingleObject(self.event_handle, wait_time) }; - - match signal { - 0x80 => Err(Box::new(TelemetryError::ABANDONED)), // Abandoned - 0x102 => Err(Box::new(TelemetryError::TIMEOUT(wait_time as usize))), // Timeout - 0xFFFFFFFF => { - // Error - let errno = unsafe { GetLastError() as i32 }; - Err(Box::new(std::io::Error::from_raw_os_error(errno))) - } - 0x00 => { - // OK - unsafe { ResetEvent(self.event_handle) }; - self.header.telemetry(self.origin) + unsafe { + let signal = WaitForSingleObject(self.event_handle, wait_time); + + match signal { + 0x80 => Err(Box::new(TelemetryError::ABANDONED)), // Abandoned + 0x102 => Err(Box::new(TelemetryError::TIMEOUT(wait_time as usize))), // Timeout + 0xFFFFFFFF => { + // Error + let errno = GetLastError() as i32; + Err(Box::new(std::io::Error::from_raw_os_error(errno))) + } + 0x00 => { + // OK + ResetEvent(self.event_handle); + self.header.telemetry(self.origin) + } + _ => Err(Box::new(TelemetryError::UNKNOWN(signal as u32))), } - _ => Err(Box::new(TelemetryError::UNKNOWN(signal as u32))), } } } impl<'conn> Drop for Blocking<'conn> { fn drop(&mut self) { - let succ = unsafe { CloseHandle(self.event_handle) }; - - if succ == 0 { - let errno: i32 = unsafe { GetLastError() as i32 }; - panic!( - "Unable to close handle: {}", - std::io::Error::from_raw_os_error(errno) - ); + unsafe { + let succ = CloseHandle(self.event_handle); + if succ == 0 { + let errno = GetLastError() as i32; + panic!( + "Unable to close handle: {}", + std::io::Error::from_raw_os_error(errno) + ); + } } } } @@ -623,39 +629,21 @@ impl Connection { let mut path: Vec = TELEMETRY_PATH.encode_utf16().collect(); path.push(0); - let mapping: HANDLE; - let errno: i32; - unsafe { - mapping = OpenFileMappingW(FILE_MAP_READ, 0, path.as_ptr()); - }; - - if mapping.is_null() { - unsafe { - errno = GetLastError() as i32; + let mapping: HANDLE = OpenFileMappingW(FILE_MAP_READ, 0, path.as_ptr()); + if mapping.is_null() { + let errno = GetLastError() as i32; + return Err(std::io::Error::from_raw_os_error(errno)); } - return Err(std::io::Error::from_raw_os_error(errno)); - } - - let view: LPVOID; - - unsafe { - view = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, 0); - } - - if view.is_null() { - unsafe { - errno = GetLastError() as i32; + let location: LPVOID = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, 0); + if location.is_null() { + let errno = GetLastError() as i32; + return Err(std::io::Error::from_raw_os_error(errno)); } - return Err(std::io::Error::from_raw_os_error(errno)); + Ok(Connection { mapping, location }) } - - Ok(Connection { - mapping, - location: view, - }) } /// @@ -663,9 +651,11 @@ impl Connection { /// /// Reads the data header from the shared memory map and returns a copy of the header /// which can be used safely elsewhere. - unsafe fn read_header(from: *const c_void) -> Header { - let raw_header: *const Header = transmute(from); - *raw_header + fn read_header(&self) -> Header { + unsafe { + let raw_header: *const Header = transmute(self.location); + *raw_header + } } /// @@ -685,7 +675,7 @@ impl Connection { /// }; /// ``` pub fn session_info(&mut self) -> Result> { - let header = unsafe { Self::read_header(self.location) }; + let header = self.read_header(); let start = (self.location as usize + header.session_info_offset as usize) as *const u8; let size = header.session_info_length as usize; @@ -715,7 +705,7 @@ impl Connection { /// # } /// ``` pub fn telemetry(&self) -> Result> { - let header = unsafe { Self::read_header(self.location) }; + let header = self.read_header(); header.telemetry(self.location as *const std::ffi::c_void) } @@ -739,30 +729,30 @@ impl Connection { /// # } /// ``` pub fn blocking(&self) -> IOResult> { - Blocking::new(self.location, unsafe { Self::read_header(self.location) }) + Blocking::new(self.location, self.read_header()) } } impl Drop for Connection { fn drop(&mut self) { - let succ = unsafe { UnmapViewOfFile(self.location) }; - if succ == 0 { - let errno: i32 = unsafe { GetLastError() as i32 }; - - panic!( - "Unable to unmap file: {}", - std::io::Error::from_raw_os_error(errno) - ); - } - - let succ = unsafe { CloseHandle(self.mapping) }; - if succ == 0 { - let errno: i32 = unsafe { GetLastError() as i32 }; + unsafe { + let succ = UnmapViewOfFile(self.location); + if succ == 0 { + let errno = GetLastError() as i32; + panic!( + "Unable to unmap file: {}", + std::io::Error::from_raw_os_error(errno) + ); + } - panic!( - "Unable to close handle: {}", - std::io::Error::from_raw_os_error(errno) - ); + let succ = CloseHandle(self.mapping); + if succ == 0 { + let errno = GetLastError() as i32; + panic!( + "Unable to close handle: {}", + std::io::Error::from_raw_os_error(errno) + ); + } } } } From 243feb358adc483a28607f13ef23a4e651c8311b Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 01:06:53 -0700 Subject: [PATCH 3/9] These cannot safely be exposed publicly because they accept raw pointers --- src/telemetry.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/telemetry.rs b/src/telemetry.rs index 37bdee1..e8f6916 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -348,7 +348,7 @@ impl Header { content } - pub fn telemetry(&self, from_loc: *const c_void) -> Result> { + fn telemetry(&self, from_loc: *const c_void) -> Result> { let (tick, vbh) = self.latest_buffer(); let value_buffer = self.var_buffer(vbh, from_loc); let value_header = self.get_var_header(from_loc); @@ -520,7 +520,7 @@ impl Display for TelemetryError { impl Error for TelemetryError {} impl<'conn> Blocking<'conn> { - pub fn new(location: *const c_void, head: Header) -> std::io::Result { + fn new(location: *const c_void, head: Header) -> std::io::Result { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); From d56217979ca353041c8fec62c6ad75887ee1a0b9 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 01:11:13 -0700 Subject: [PATCH 4/9] This does not need mutable access --- examples/get_telemetry.rs | 3 +-- examples/view_session.rs | 2 +- src/telemetry.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/examples/get_telemetry.rs b/examples/get_telemetry.rs index 07cc96c..7ac4c8c 100644 --- a/examples/get_telemetry.rs +++ b/examples/get_telemetry.rs @@ -4,8 +4,7 @@ use std::convert::TryInto; use std::time::Duration; pub fn main() { - let mut conn = Connection::new().expect("Unable to open telemetry"); - + let conn = Connection::new().expect("Unable to open telemetry"); let session = conn.session_info().expect("Invalid Session"); let shift_up_rpm = session.drivers.shift_light_shift_rpm; diff --git a/examples/view_session.rs b/examples/view_session.rs index 6879660..e052049 100644 --- a/examples/view_session.rs +++ b/examples/view_session.rs @@ -1,7 +1,7 @@ use iracing::telemetry::Connection; pub fn main() { - let mut conn = Connection::new().expect("Unable to open telemetry"); + let conn = Connection::new().expect("Unable to open telemetry"); let session = conn.session_info().expect("Invalid session data"); print!("{:#?}", session); diff --git a/src/telemetry.rs b/src/telemetry.rs index e8f6916..12cda84 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -674,7 +674,7 @@ impl Connection { /// Err(e) => println!("Invalid Session") /// }; /// ``` - pub fn session_info(&mut self) -> Result> { + pub fn session_info(&self) -> Result> { let header = self.read_header(); let start = (self.location as usize + header.session_info_offset as usize) as *const u8; From f91de4319c5e963e485c14e91cd858bb996d9143 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 11:39:28 -0700 Subject: [PATCH 5/9] Transmute is unnecessary --- src/telemetry.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/telemetry.rs b/src/telemetry.rs index 12cda84..93c7856 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -9,7 +9,6 @@ use std::ffi::CStr; use std::fmt::{self, Display}; use std::io::Result as IOResult; use std::marker::PhantomData; -use std::mem::transmute; use std::os::raw::{c_char, c_void}; use std::os::windows::raw::HANDLE; use std::slice::from_raw_parts; @@ -652,10 +651,8 @@ impl Connection { /// Reads the data header from the shared memory map and returns a copy of the header /// which can be used safely elsewhere. fn read_header(&self) -> Header { - unsafe { - let raw_header: *const Header = transmute(self.location); - *raw_header - } + let raw_header = self.location as *const Header; + unsafe { *raw_header } } /// From 9964e7eea53fb320f76e31de7c3c15e227854232 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Mon, 26 Jul 2021 13:53:52 -0700 Subject: [PATCH 6/9] Fix a resource leak while handling errors --- src/telemetry.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/telemetry.rs b/src/telemetry.rs index 93c7856..e41ec22 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -638,6 +638,7 @@ impl Connection { let location: LPVOID = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, 0); if location.is_null() { let errno = GetLastError() as i32; + CloseHandle(mapping); // Ignore return value; we are already handling another error. return Err(std::io::Error::from_raw_os_error(errno)); } @@ -736,6 +737,7 @@ impl Drop for Connection { let succ = UnmapViewOfFile(self.location); if succ == 0 { let errno = GetLastError() as i32; + CloseHandle(self.mapping); // Ignore return value; we are already handling another error. panic!( "Unable to unmap file: {}", std::io::Error::from_raw_os_error(errno) From c5150ae03f1246f0cb0c9dbd7d04651189710cd6 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Mon, 26 Jul 2021 18:13:28 -0700 Subject: [PATCH 7/9] Lazily read `Header` - Fixes #3 --- src/telemetry.rs | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/telemetry.rs b/src/telemetry.rs index e41ec22..404e6d4 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -57,7 +57,6 @@ pub struct Header { /// pub struct Blocking<'conn> { origin: *const c_void, - header: Header, event_handle: HANDLE, phantom: PhantomData<&'conn Connection>, } @@ -318,10 +317,15 @@ impl fmt::Debug for ValueHeader { impl Header { fn latest_buffer(&self) -> (i32, ValueBuffer) { - let mut latest_tick: i32 = 0; let mut buffer = self.buffers[0]; - - for b in self.buffers.iter() { + let mut latest_tick = buffer.ticks; + + for b in self + .buffers + .iter() + .skip(1) + .take((self.n_buffers - 1) as usize) + { if b.ticks > latest_tick { buffer = *b; latest_tick = b.ticks; @@ -519,7 +523,7 @@ impl Display for TelemetryError { impl Error for TelemetryError {} impl<'conn> Blocking<'conn> { - fn new(location: *const c_void, head: Header) -> std::io::Result { + fn new(location: *const c_void) -> std::io::Result { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); @@ -536,12 +540,22 @@ impl<'conn> Blocking<'conn> { Ok(Blocking { origin: location, - header: head, event_handle: handle, phantom: PhantomData, }) } + /// Get the data header + /// + /// Reads the data header from the shared memory map and returns a copy of the header + /// which can be used safely elsewhere. + fn read_header(&self) -> Header { + let raw_header = self.origin as *const Header; + // SAFETY: This read CANNOT be done safely, since it is an unsynchronized clone of a struct + // larger than AtomicUsize. + unsafe { *raw_header } + } + /// /// Sample Telemetry Data /// @@ -581,7 +595,7 @@ impl<'conn> Blocking<'conn> { 0x00 => { // OK ResetEvent(self.event_handle); - self.header.telemetry(self.origin) + self.read_header().telemetry(self.origin) } _ => Err(Box::new(TelemetryError::UNKNOWN(signal as u32))), } @@ -653,6 +667,8 @@ impl Connection { /// which can be used safely elsewhere. fn read_header(&self) -> Header { let raw_header = self.location as *const Header; + // SAFETY: This read CANNOT be done safely, since it is an unsynchronized clone of a struct + // larger than AtomicUsize. unsafe { *raw_header } } @@ -727,7 +743,7 @@ impl Connection { /// # } /// ``` pub fn blocking(&self) -> IOResult> { - Blocking::new(self.location, self.read_header()) + Blocking::new(self.location) } } From d755c459af555e6326a9a0589fd41c4b6ce78ee4 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sun, 25 Jul 2021 02:03:54 -0700 Subject: [PATCH 8/9] Fix error handling - Closes #4 --- Cargo.toml | 4 +- examples/dump_sample.rs | 2 +- src/telemetry.rs | 124 +++++++++++++++++++++------------------- 3 files changed, 70 insertions(+), 60 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fe9d525..d68829a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,9 +14,11 @@ telemetry = ["winapi"] bitflags = "1.2" chrono = "0.4" encoding_rs = "0.8" +libc = { version = "0.2", optional = true } serde = {version = "1.0", features = ["derive"] } serde_yaml = "0.8" -winapi = {version = "0.3.9", features = ["std","memoryapi","winnt","errhandlingapi","synchapi","handleapi"], optional = true } +thiserror = "1.0" +winapi = {version = "0.3", features = ["std","memoryapi","winnt","errhandlingapi","synchapi","handleapi"], optional = true } [package.metadata.docs.rs] default-target = "x86_64-pc-windows-msvc" diff --git a/examples/dump_sample.rs b/examples/dump_sample.rs index 0c55384..5ac30ee 100644 --- a/examples/dump_sample.rs +++ b/examples/dump_sample.rs @@ -3,7 +3,7 @@ use serde_yaml::to_string; pub fn main() { let conn = Connection::new().expect("Unable to open telemetry"); - let telem = conn.telemetry().expect("Telem"); + let telem = conn.telemetry(); print!("{}", to_string(&telem.all()).unwrap()); } diff --git a/src/telemetry.rs b/src/telemetry.rs index 404e6d4..7094ed1 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -2,17 +2,18 @@ use crate::session::*; use encoding_rs::mem::decode_latin1; use serde::{Deserialize, Serialize}; use serde_yaml::from_str as yaml_from; +use std::borrow::Cow; use std::convert::TryInto; use std::default::Default; -use std::error::Error; use std::ffi::CStr; -use std::fmt::{self, Display}; +use std::fmt; use std::io::Result as IOResult; use std::marker::PhantomData; use std::os::raw::{c_char, c_void}; use std::os::windows::raw::HANDLE; use std::slice::from_raw_parts; use std::time::Duration; +use thiserror::Error; use winapi::shared::minwindef::LPVOID; use winapi::um::errhandlingapi::GetLastError; use winapi::um::handleapi::CloseHandle; @@ -196,47 +197,47 @@ impl Value { } impl TryInto for Value { - type Error = &'static str; + type Error = ValueError; fn try_into(self) -> Result { match self { Self::INT(n) => Ok(n), - _ => Err("Value is not a signed 4-byte integer"), + _ => Err(ValueError::TryInto("Value is not a signed 4-byte integer")), } } } impl TryInto for Value { - type Error = &'static str; + type Error = ValueError; fn try_into(self) -> Result { match self { Self::INT(n) => Ok(n as u32), Self::BITS(n) => Ok(n), - _ => Err("Value is not a 4-byte integer"), + _ => Err(ValueError::TryInto("Value is not a 4-byte integer")), } } } impl TryInto for Value { - type Error = &'static str; + type Error = ValueError; fn try_into(self) -> Result { match self { Self::FLOAT(n) => Ok(n), - _ => Err("Value is not a float"), + _ => Err(ValueError::TryInto("Value is not a float")), } } } impl TryInto for Value { - type Error = &'static str; + type Error = ValueError; fn try_into(self) -> Result { match self { Self::DOUBLE(n) => Ok(n), Self::FLOAT(f) => Ok(f as f64), - _ => Err("Value is not a float or double"), + _ => Err(ValueError::TryInto("Value is not a float or double")), } } } @@ -303,7 +304,7 @@ impl Default for ValueHeader { } impl fmt::Debug for ValueHeader { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, "ValueHeader(name=\"{}\", type={}, count={}, offset={})", @@ -351,16 +352,12 @@ impl Header { content } - fn telemetry(&self, from_loc: *const c_void) -> Result> { + fn telemetry(&self, from_loc: *const c_void) -> Sample { let (tick, vbh) = self.latest_buffer(); let value_buffer = self.var_buffer(vbh, from_loc); let value_header = self.get_var_header(from_loc); - Ok(Sample::new( - tick, - value_header.to_vec(), - value_buffer.to_vec(), - )) + Sample::new(tick, value_header.to_vec(), value_buffer.to_vec()) } } @@ -425,9 +422,9 @@ impl Sample { /// /// `name` Name of the telemetry variable to get /// - see the iRacing Telemtry documentation for a complete list of possible values - pub fn get(&self, name: &'static str) -> Result { + pub fn get(&self, name: &'static str) -> Result { match self.header_for(name) { - None => Err(format!("No value '{}' found", name)), + None => Err(SampleError::NoValue(format!("No value '{}' found", name))), Some(vh) => Ok(self.value(&vh)), } } @@ -499,31 +496,51 @@ impl Sample { } } -/// -/// Telemetry Error -/// /// An error which occurs when telemetry samples cannot be read from the memory buffer. -#[derive(Debug)] +#[derive(Debug, Error)] pub enum TelemetryError { - ABANDONED, - TIMEOUT(usize), - UNKNOWN(u32), + #[error("Invalid duration: {0}")] + InvalidDuration(#[from] std::num::TryFromIntError), + + #[error("Windows Mutex was abandoned.")] + Abandoned, + + #[error("Timeout elapsed while waiting for a signal. Duration={0}")] + Timeout(usize), + + #[error("Unknown error occurred: {0}")] + Unknown(u32), + + #[error("Wait failed: {0}")] + WaitFailed(std::io::Error), } -impl Display for TelemetryError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::ABANDONED => write!(f, "Abandoned"), - Self::TIMEOUT(ms) => write!(f, "Timeout after {}ms", ms), - Self::UNKNOWN(v) => write!(f, "Unknown error code = {:x?}", v), - } - } +/// An error which occurs while retrieving session info. +#[derive(Debug, Error)] +pub enum SessionError { + #[error("Text decode error: {0}")] + Decode(Cow<'static, str>), + + #[error("YAML parse error: {0}")] + ParseYAML(#[from] serde_yaml::Error), } -impl Error for TelemetryError {} +/// An error which occurs when value conversions fail. +#[derive(Debug, Error)] +pub enum ValueError { + #[error("{0}")] + TryInto(&'static str), +} + +/// An error which may occur when reading values from a sample. +#[derive(Debug, Error)] +pub enum SampleError { + #[error("{0}")] + NoValue(String), +} impl<'conn> Blocking<'conn> { - fn new(location: *const c_void) -> std::io::Result { + fn new(location: *const c_void) -> IOResult { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); @@ -575,29 +592,26 @@ impl<'conn> Blocking<'conn> { /// # Ok(()) /// # } /// ``` - pub fn sample(&self, timeout: Duration) -> Result> { - let wait_time: u32 = match timeout.as_millis().try_into() { - Ok(v) => v, - Err(e) => return Err(Box::new(e)), - }; + pub fn sample(&self, timeout: Duration) -> Result { + let wait_time: u32 = timeout.as_millis().try_into()?; unsafe { let signal = WaitForSingleObject(self.event_handle, wait_time); match signal { - 0x80 => Err(Box::new(TelemetryError::ABANDONED)), // Abandoned - 0x102 => Err(Box::new(TelemetryError::TIMEOUT(wait_time as usize))), // Timeout + 0x80 => Err(TelemetryError::Abandoned), + 0x102 => Err(TelemetryError::Timeout(wait_time as usize)), 0xFFFFFFFF => { // Error - let errno = GetLastError() as i32; - Err(Box::new(std::io::Error::from_raw_os_error(errno))) + let errno = std::io::Error::from_raw_os_error(GetLastError() as i32); + Err(TelemetryError::WaitFailed(errno)) } 0x00 => { // OK ResetEvent(self.event_handle); - self.read_header().telemetry(self.origin) + Ok(self.read_header().telemetry(self.origin)) } - _ => Err(Box::new(TelemetryError::UNKNOWN(signal as u32))), + _ => Err(TelemetryError::Unknown(signal as u32)), } } } @@ -688,7 +702,7 @@ impl Connection { /// Err(e) => println!("Invalid Session") /// }; /// ``` - pub fn session_info(&self) -> Result> { + pub fn session_info(&self) -> Result { let header = self.read_header(); let start = (self.location as usize + header.session_info_offset as usize) as *const u8; @@ -711,14 +725,14 @@ impl Connection { /// # Examples /// /// ``` - /// # fn main() -> Result<(), Box> { + /// # fn main() -> Result<(), std::io::Error> { /// use iracing::telemetry::Connection; /// - /// let sample = Connection::new()?.telemetry()?; + /// let sample = Connection::new()?.telemetry(); /// # Ok(()) /// # } /// ``` - pub fn telemetry(&self) -> Result> { + pub fn telemetry(&self) -> Sample { let header = self.read_header(); header.telemetry(self.location as *const std::ffi::c_void) } @@ -788,12 +802,6 @@ mod tests { fn test_latest_telemetry() { let session_tick: u32 = Connection::new() .expect("Unable to open telemetry") - .telemetry() - .expect("Couldn't get latest telem") - .get("SessionTick") - .unwrap() - .try_into() - .unwrap(); - assert!(session_tick > 0); + .telemetry(); } } From f1b2ba514cbf2685adea011e0bd3c671bf6b8440 Mon Sep 17 00:00:00 2001 From: Jason Dilworth Date: Fri, 21 Feb 2025 14:43:25 +0000 Subject: [PATCH 9/9] Linting fixes --- src/replay.rs | 3 +-- src/telemetry.rs | 13 ++++++------- src/track_surface.rs | 10 +++++----- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/replay.rs b/src/replay.rs index 586b462..e69df53 100644 --- a/src/replay.rs +++ b/src/replay.rs @@ -3,7 +3,6 @@ use std::io; use std::io::Read; use std::io::Result as IOResult; use std::io::{Error as IOError, ErrorKind}; -use std::u32; /// Magic number found at the start of replay files pub const FILE_MAGIC: &[u8] = b"YLPR"; @@ -170,7 +169,7 @@ fn read_str(mut reader: R, length: usize) -> IOResult { .position(|&b| b == 0) .expect("Given string does not terminate within given length"); - Ok(String::from_utf8((&raw_string_bytes[..nul]).to_vec()).unwrap()) + Ok(String::from_utf8((raw_string_bytes[..nul]).to_vec()).unwrap()) } impl Replay { diff --git a/src/telemetry.rs b/src/telemetry.rs index 7094ed1..83bd7d6 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -438,8 +438,7 @@ impl Sample { let raw_val = &self.buffer[vs..ve]; - let v: Value; - v = match vt { + let v: Value = match vt { Value::INT(_) => { if vc == 1 { Value::INT(i32::from_le_bytes(raw_val.try_into().unwrap())) @@ -475,7 +474,7 @@ impl Sample { } Value::DOUBLE(_) => Value::DOUBLE(f64::from_le_bytes(raw_val.try_into().unwrap())), Value::BITS(_) => Value::BITS(u32::from_le_bytes(raw_val.try_into().unwrap())), - Value::CHAR(_) => Value::CHAR(raw_val[0] as u8), + Value::CHAR(_) => Value::CHAR(raw_val[0]), Value::BOOL(_) => { if vc == 1 { Value::BOOL(raw_val[0] > 0) @@ -539,8 +538,8 @@ pub enum SampleError { NoValue(String), } -impl<'conn> Blocking<'conn> { - fn new(location: *const c_void) -> IOResult { +impl Blocking<'_> { + fn new(location: *const c_void) -> std::io::Result { let mut event_name: Vec = DATA_EVENT_NAME.encode_utf16().collect(); event_name.push(0); @@ -617,7 +616,7 @@ impl<'conn> Blocking<'conn> { } } -impl<'conn> Drop for Blocking<'conn> { +impl Drop for Blocking<'_> { fn drop(&mut self) { unsafe { let succ = CloseHandle(self.event_handle); @@ -800,7 +799,7 @@ mod tests { #[test] fn test_latest_telemetry() { - let session_tick: u32 = Connection::new() + let session_tick: Sample = Connection::new() .expect("Unable to open telemetry") .telemetry(); } diff --git a/src/track_surface.rs b/src/track_surface.rs index d86ddd9..e3bc163 100644 --- a/src/track_surface.rs +++ b/src/track_surface.rs @@ -25,15 +25,15 @@ impl From for TrackSurface { match idx { -1 => TrackSurface::NotInWorld, 0 => TrackSurface::Undefined, - 1 | 2 | 3 | 4 => TrackSurface::Asphalt(ix), + 1..=4 => TrackSurface::Asphalt(ix), 6 | 7 => TrackSurface::Concrete(ix - 4), 8 | 9 => TrackSurface::RacingDirt(ix - 7), 10 | 11 => TrackSurface::Paint(ix - 9), - 12 | 13 | 14 | 15 => TrackSurface::Rumble(ix - 11), - 16 | 17 | 18 | 19 => TrackSurface::Grass(ix - 15), - 20 | 21 | 22 | 23 => TrackSurface::Dirt(ix - 19), + 12..=15 => TrackSurface::Rumble(ix - 11), + 16..=19 => TrackSurface::Grass(ix - 15), + 20..=23 => TrackSurface::Dirt(ix - 19), 24 => TrackSurface::Sand, - 25 | 26 | 27 | 28 => TrackSurface::Gravel(ix - 24), + 25..=28 => TrackSurface::Gravel(ix - 24), 29 => TrackSurface::Grasscrete, 30 => TrackSurface::Astroturf, _ => TrackSurface::Unknown(ix),