diff --git a/src/errors.rs b/src/errors.rs index 9e619d2..831e4b6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -14,9 +14,7 @@ pub enum Error { /// C API failed to open a file (No return code provided) CouldNotOpen { path: PathBuf, mode: FileMode }, /// A path could not be converted to &OsStr - InvalidOsStr, - /// A path could not be converted to &CStr because it had a null byte - NullInStr(std::ffi::NulError), + InvalidOsStr(Option), /// Checking the number of atoms failed while reading a frame CouldNotCheckNAtoms(Box), /// Error for an out-of-range numeric conversion @@ -61,7 +59,13 @@ impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { use Error::*; match &self { - NullInStr(err) => Some(err), + InvalidOsStr(err) => { + if let Some(err) = err { + Some(err) + } else { + None + } + }, CouldNotCheckNAtoms(err) => Some(err.as_ref()), _ => None, } @@ -75,12 +79,6 @@ impl From<(ErrorCode, ErrorTask)> for Error { } } -impl From for Error { - fn from(err: std::ffi::NulError) -> Self { - Self::NullInStr(err) - } -} - impl From<(&Path, FileMode)> for Error { fn from(value: (&Path, FileMode)) -> Self { let (path, mode) = value; @@ -119,9 +117,8 @@ impl std::fmt::Display for Error { CouldNotOpen { path, mode } => { write!(f, "Could not open file at {:?} in mode {:?}", path, mode) } - InvalidOsStr => write!(f, "Paths must be valid unicode on this platform"), - NullInStr(_err) => write!(f, "Paths cannot include null bytes"), - CouldNotCheckNAtoms(_err) => { + InvalidOsStr(_) => write!(f, "Cannot convert path to CString."), + CouldNotCheckNAtoms(_) => { write!(f, "Failed to read number of atoms in trajectory file") } OutOfRange { @@ -249,8 +246,6 @@ pub type Result = std::result::Result; #[cfg(test)] mod tests { use super::*; - use std::ffi::CString; - use std::ffi::NulError; #[test] fn test_is_eof() { @@ -294,12 +289,6 @@ mod tests { #[test] fn test_from_correct_type() { - let nul_err: NulError = CString::new(b"foo\0".to_vec()).unwrap_err(); - let nul_err2: NulError = CString::new(b"foo\0".to_vec()).unwrap_err(); - let err = Error::from(nul_err); - let expected = Error::NullInStr(nul_err2); - assert_eq!(expected, err); - let code = 3.into(); let task = ErrorTask::Read; let expected = Error::CApiError { code, task }; diff --git a/src/lib.rs b/src/lib.rs index 7eb953b..ab1b25a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,8 +105,11 @@ impl FileMode { } fn path_to_cstring(path: impl AsRef) -> Result { - let s = path.as_ref().to_str().ok_or(Error::InvalidOsStr)?; - Ok(CString::new(s)?) + if let Some(s) = path.as_ref().to_str() { + CString::new(s).map_err(|e| Error::InvalidOsStr(Some(e))) + } else { + Err(Error::InvalidOsStr(None)) + } } fn to(value: I, task: ErrorTask, name: &'static str) -> Result @@ -621,21 +624,28 @@ mod tests { #[test] fn test_path_to_cstring() -> Result<(), Box> { - let result_invalid = path_to_cstring(PathBuf::from("invalid/\0path")); - - if let Err(err) = result_invalid { - match err { - Error::NullInStr(_) => (), - Error::InvalidOsStr => (), - _ => panic!("Improper error type for path_to_cstring"), + // A valid string should convert to CString successfully + let valid_result = path_to_cstring(PathBuf::from("test")); + match valid_result { + Ok(s) => { + assert_eq!(s, CString::new("test")?); } - } else { - panic!("path_to_cstring should return Err if there are null bytes"); + Err(_) => panic!("Valid Path failed to convert to CString.") } - let result_valid = path_to_cstring("valid/path"); - assert_eq!(result_valid, Ok(CString::new("valid/path")?)); - + // \0 in path should result in an InvalidOsStr(Some(NulError)) + let result = path_to_cstring(PathBuf::from("invalid/\0path")); + match result { + Ok(_) => panic!("Cstring conversion did not fail"), + Err(e) => { + match e { + Error::InvalidOsStr(opt) => { + assert!(opt.is_some()) + } + _ => panic!("Wrong error type. (This should never happend).") + } + } + } Ok(()) }