From 55fa21d45c5b7178c206f8eadfa56f8bc4428d53 Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Sat, 7 Nov 2020 23:25:30 +1100 Subject: [PATCH] Broke out errors module and handle error codes as rust enum --- src/errors.rs | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 184 +++++++++++++---------------------------- 2 files changed, 277 insertions(+), 127 deletions(-) create mode 100644 src/errors.rs diff --git a/src/errors.rs b/src/errors.rs new file mode 100644 index 0000000..efa09ac --- /dev/null +++ b/src/errors.rs @@ -0,0 +1,220 @@ +use crate::c_abi; +use crate::FileMode; +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, PartialEq)] +pub enum ErrorTask { + OpenFile(PathBuf, FileMode), + ReadNumAtoms, + Read, + Write, + Flush, + ToCString(Option), +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Error { + code: Option, + task: ErrorTask, +} + +impl Error { + pub fn task(&self) -> &ErrorTask { + &self.task + } + + pub fn code(&self) -> &Option { + &self.code + } + + pub fn is_eof(&self) -> bool { + self.code.as_ref().map_or(false, ErrorCode::is_eof) + } + + pub fn from_convert() -> Self { + Self { + code: None, + task: ErrorTask::ToCString(None), + } + } + + pub fn from_open(path: impl AsRef, mode: FileMode) -> Self { + Self { + code: None, + task: ErrorTask::OpenFile(path.as_ref().into(), mode), + } + } + + pub fn from_read_num_atoms(code: impl Into) -> Self { + Self { + code: Some(code.into()), + task: ErrorTask::ReadNumAtoms, + } + } + + pub fn from_read(code: impl Into) -> Self { + Self { + code: Some(code.into()), + task: ErrorTask::Read, + } + } + + pub fn from_write(code: impl Into) -> Self { + Self { + code: Some(code.into()), + task: ErrorTask::Write, + } + } + + pub fn from_flush(code: impl Into) -> Self { + Self { + code: Some(code.into()), + task: ErrorTask::Flush, + } + } +} + +impl From for Error { + fn from(err: std::ffi::NulError) -> Self { + Self { + code: None, + task: ErrorTask::ToCString(Some(err)), + } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use ErrorTask::*; + match &self.task { + OpenFile(path, mode) => write!( + f, + "Failed to open file at {path:?} with mode {mode:?}", + path = path, + mode = mode + ), + ReadNumAtoms => write!( + f, + "Failed to read atom number from trajectory: C API returned error code {:?}", + self.code + ), + Read => write!( + f, + "Failed to read trajectory: C API returned error code {:?}", + self.code + ), + Write => write!( + f, + "Failed to write trajectory: C API returned error code {:?}", + self.code + ), + Flush => write!( + f, + "Failed to flush trajectory: C API returned error code {:?}", + self.code + ), + ToCString(_) => write!( + f, + "Path cannot be converted to a C string because it has a null byte" + ), + } + } +} + +#[derive(Debug, Clone, PartialEq)] +pub enum ErrorCode { + ExdrOk, + ExdrHeader, + ExdrString, + ExdrDouble, + ExdrInt, + ExdrFloat, + ExdrUint, + Exdr3dx, + ExdrClose, + ExdrMagic, + ExdrNoMem, + ExdrEndOfFile, + ExdrFileNotFound, + ExdrNr, + UnmatchedCode(c_abi::xdrfile::BindgenTy1), +} + +impl ErrorCode { + pub fn is_eof(&self) -> bool { + match self { + Self::ExdrEndOfFile => true, + _ => false, + } + } +} + +impl From for ErrorCode { + fn from(code: c_abi::xdrfile::BindgenTy1) -> Self { + match code { + c_abi::xdrfile::exdrOK => Self::ExdrOk, + c_abi::xdrfile::exdrHEADER => Self::ExdrHeader, + c_abi::xdrfile::exdrSTRING => Self::ExdrString, + c_abi::xdrfile::exdrDOUBLE => Self::ExdrDouble, + c_abi::xdrfile::exdrINT => Self::ExdrInt, + c_abi::xdrfile::exdrFLOAT => Self::ExdrFloat, + c_abi::xdrfile::exdrUINT => Self::ExdrUint, + c_abi::xdrfile::exdr3DX => Self::Exdr3dx, + c_abi::xdrfile::exdrCLOSE => Self::ExdrClose, + c_abi::xdrfile::exdrMAGIC => Self::ExdrMagic, + c_abi::xdrfile::exdrNOMEM => Self::ExdrNoMem, + c_abi::xdrfile::exdrENDOFFILE => Self::ExdrEndOfFile, + c_abi::xdrfile::exdrFILENOTFOUND => Self::ExdrFileNotFound, + c_abi::xdrfile::exdrNR => Self::ExdrNr, + code => Self::UnmatchedCode(code), + } + } +} + +impl std::error::Error for Error {} + +pub type Result = std::result::Result; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_is_eof() { + let error = Error { + code: Some(c_abi::xdrfile::exdrENDOFFILE.into()), + task: ErrorTask::Read, + }; + assert!(error.is_eof()); + + let error = Error { + code: Some(ErrorCode::ExdrEndOfFile), + task: ErrorTask::Read, + }; + assert!(error.is_eof()); + + let error = Error { + code: Some((c_abi::xdrfile::exdrENDOFFILE + 1).into()), + task: ErrorTask::Read, + }; + assert!(!error.is_eof()); + + let error = Error { + code: Some(0.into()), + task: ErrorTask::Write, + }; + assert!(!error.is_eof()); + + let error = Error { + code: Some(255.into()), + task: ErrorTask::Flush, + }; + assert!(!error.is_eof()); + + let error = Error { + code: None, + task: ErrorTask::OpenFile(PathBuf::from("not/a/file"), FileMode::Read), + }; + assert!(!error.is_eof()); + } +} diff --git a/src/lib.rs b/src/lib.rs index 86ad06d..a8300d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,8 +62,10 @@ extern crate assert_approx_eq; extern crate lazy_init; pub mod c_abi; +mod errors; mod frame; mod iterator; +pub use errors::*; pub use frame::Frame; pub use iterator::*; @@ -78,75 +80,6 @@ use std::cell::Cell; use std::ffi::CString; use std::path::{Path, PathBuf}; -#[derive(Debug, Clone, PartialEq)] -pub enum Error { - CouldNotOpenFile(std::path::PathBuf, FileMode), - CouldNotReadAtomNumber(u32), - CouldNotRead(u32), - CouldNotWrite(u32), - CouldNotFlush(u32), - PathInvalidCstring, -} - -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use Error::*; - match self { - CouldNotOpenFile(path, mode) => write!( - f, - "Failed to open file at {path:?} with mode {mode:?}", - path = path, - mode = mode - ), - CouldNotReadAtomNumber(code) => write!( - f, - "Failed to read atom number from trajectory: C API returned error code {}", - code - ), - CouldNotRead(code) => write!( - f, - "Failed to read trajectory: C API returned error code {}", - code - ), - CouldNotWrite(code) => write!( - f, - "Failed to write trajectory: C API returned error code {}", - code - ), - CouldNotFlush(code) => write!( - f, - "Failed to flush trajectory: C API returned error code {}", - code - ), - PathInvalidCstring => write!( - f, - "Path cannot be converted to a C string because it has a 0 byte" - ), - } - } -} - -impl Error { - pub fn is_eof(&self) -> bool { - use Error::*; - match self { - &CouldNotReadAtomNumber(code) - | &CouldNotRead(code) - | &CouldNotWrite(code) - | &CouldNotFlush(code) - if code == xdrfile::exdrENDOFFILE => - { - true - } - _ => false, - } - } -} - -impl std::error::Error for Error {} - -pub type Result = std::result::Result; - #[derive(Debug, Clone, PartialEq)] pub enum FileMode { Write, @@ -168,8 +101,8 @@ impl FileMode { } fn path_to_cstring(path: impl AsRef) -> Result { - let s = path.as_ref().to_str().ok_or(Error::PathInvalidCstring)?; - CString::new(s).map_err(|_| Error::PathInvalidCstring) + let s = path.as_ref().to_str().ok_or_else(Error::from_convert)?; + Ok(CString::new(s)?) } /// A safe wrapper around the c implementation of an XDRFile @@ -202,7 +135,7 @@ impl XDRFile { }) } else { // Something went wrong. But the C api does not tell us what - Err(Error::CouldNotOpenFile(path.into(), filemode)) + Err(Error::from_open(path, filemode)) } } } @@ -282,9 +215,9 @@ impl Trajectory for XTCTrajectory { &mut self.precision.get(), ) as u32; frame.step = step as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotRead(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_read(code)), } } } @@ -300,9 +233,9 @@ impl Trajectory for XTCTrajectory { frame.coords[..].as_ptr() as *mut [f32; 3], 1000.0, ) as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotWrite(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_write(code)), } } } @@ -310,9 +243,9 @@ impl Trajectory for XTCTrajectory { fn flush(&mut self) -> Result<()> { unsafe { let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotFlush(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_flush(code)), } } } @@ -328,9 +261,10 @@ impl Trajectory for XTCTrajectory { xdrfile_xtc::read_xtc_natoms(path_p, &mut num_atoms as *const i32) as u32; // Reconstitute the CString so it is deallocated correctly let _ = CString::from_raw(path_p); - match code { - xdrfile::exdrOK => Ok(num_atoms as u32), - _ => Err(Error::CouldNotReadAtomNumber(code)), + + match code.into() { + ErrorCode::ExdrOk => Ok(num_atoms as u32), + _ => Err(Error::from_read_num_atoms(code)), } } }) @@ -390,9 +324,9 @@ impl Trajectory for TRRTrajectory { std::ptr::null_mut(), ) as u32; frame.step = step as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotRead(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_read(code)), } } } @@ -410,9 +344,9 @@ impl Trajectory for TRRTrajectory { std::ptr::null_mut(), std::ptr::null_mut(), ) as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotWrite(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_write(code)), } } } @@ -420,9 +354,9 @@ impl Trajectory for TRRTrajectory { fn flush(&mut self) -> Result<()> { unsafe { let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32; - match code { - xdrfile::exdrOK => Ok(()), - _ => Err(Error::CouldNotFlush(code)), + match code.into() { + ErrorCode::ExdrOk => Ok(()), + _ => Err(Error::from_flush(code)), } } } @@ -438,9 +372,10 @@ impl Trajectory for TRRTrajectory { xdrfile_trr::read_trr_natoms(path_p, &mut num_atoms as *const i32) as u32; // Reconstitute the CString so it is deallocated correctly let _ = CString::from_raw(path_p); - match code { - xdrfile::exdrOK => Ok(num_atoms as u32), - _ => Err(Error::CouldNotReadAtomNumber(code)), + + match code.into() { + ErrorCode::ExdrOk => Ok(num_atoms as u32), + _ => Err(Error::from_read_num_atoms(code)), } } }) @@ -536,9 +471,21 @@ mod tests { #[test] fn test_path_to_cstring() -> Result<(), Box> { - let result_invalid = path_to_cstring("invalid/\0path"); + let result_invalid = path_to_cstring(PathBuf::from("invalid/\0path")); - assert_eq!(result_invalid, Err(Error::PathInvalidCstring)); + assert_eq!( + result_invalid, + CString::new("invalid/\0path").map_err(Error::from) + ); + assert!(!result_invalid.is_ok()); + if let Err(e) = result_invalid { + match e.task() { + ErrorTask::ToCString(_) => (), + _ => panic!("path_to_cstring's errortask should be ErrorTask::ToCString(_)"), + } + } else { + panic!("path_to_cstring on a NULL-containing string should return an error"); + } let result_valid = path_to_cstring("valid/path"); @@ -549,12 +496,13 @@ mod tests { #[test] fn test_err_could_not_open() { let file_name = "non-existent.xtc"; - let expexted_path = Path::new(&file_name); + + let path = Path::new(&file_name); if let Err(e) = XDRFile::open(file_name, FileMode::Read) { - match e { - Error::CouldNotOpenFile(err_path, err_mode) => { - assert_eq!(expexted_path, err_path); - assert!(FileMode::Read == err_mode) + match e.task() { + ErrorTask::OpenFile(err_path, err_mode) => { + assert_eq!(path, err_path); + assert_eq!(FileMode::Read, *err_mode) } _ => panic!("Wrong Error type"), } @@ -566,9 +514,9 @@ mod tests { let file_name = "README.md"; // not a trajectory let mut trr = TRRTrajectory::open_read(file_name)?; if let Err(e) = trr.get_num_atoms() { - match e { - Error::CouldNotReadAtomNumber(code) => { - assert_eq!(xdrfile::exdrMAGIC, code); + match e.task() { + ErrorTask::ReadNumAtoms => { + assert_eq!(Some(ErrorCode::ExdrMagic), *e.code()); } _ => panic!("Wrong Error type"), } @@ -582,9 +530,9 @@ mod tests { let mut frame = Frame::with_capacity(1); let mut trr = TRRTrajectory::open_read(file_name)?; if let Err(e) = trr.read(&mut frame) { - match e { - Error::CouldNotRead(code) => { - assert_eq!(xdrfile::exdrMAGIC, code); + match e.task() { + ErrorTask::Read => { + assert_eq!(Some(ErrorCode::ExdrMagic), *e.code()); } _ => panic!("Wrong Error type"), } @@ -592,24 +540,6 @@ mod tests { Ok(()) } - #[test] - fn test_err_eof() { - let error = Error::CouldNotRead(xdrfile::exdrENDOFFILE); - assert!(error.is_eof()); - - let error = Error::CouldNotRead(xdrfile::exdrENDOFFILE + 1); - assert!(!error.is_eof()); - - let error = Error::CouldNotWrite(0); - assert!(!error.is_eof()); - - let error = Error::CouldNotFlush(255); - assert!(!error.is_eof()); - - let error = Error::CouldNotOpenFile(PathBuf::from("not/a/file"), FileMode::Read); - assert!(!error.is_eof()); - } - #[test] fn test_err_file_eof() -> Result<(), Box> { let tempfile = NamedTempFile::new()?;