Merge pull request #7 from Yoshanuikabundi/no-fallible-casts

No fallible casts

src/c_abi/mod.rs

@@ -1,3 +1,4 @@
+//! # Low level bindings to the c library from GROMACS
 #![allow(non_upper_case_globals, non_camel_case_types)]
 
 pub mod xdr_seek;

src/c_abi/xdrfile.rs

@@ -6,21 +6,20 @@ pub struct XDRFILE {
     _unused: [u8; 0],
 }
 
-pub type BindgenTy1 = u32;
-pub const exdrOK: BindgenTy1 = 0;
-pub const exdrHEADER: BindgenTy1 = 1;
-pub const exdrSTRING: BindgenTy1 = 2;
-pub const exdrDOUBLE: BindgenTy1 = 3;
-pub const exdrINT: BindgenTy1 = 4;
-pub const exdrFLOAT: BindgenTy1 = 5;
-pub const exdrUINT: BindgenTy1 = 6;
-pub const exdr3DX: BindgenTy1 = 7;
-pub const exdrCLOSE: BindgenTy1 = 8;
-pub const exdrMAGIC: BindgenTy1 = 9;
-pub const exdrNOMEM: BindgenTy1 = 10;
-pub const exdrENDOFFILE: BindgenTy1 = 11;
-pub const exdrFILENOTFOUND: BindgenTy1 = 12;
-pub const exdrNR: BindgenTy1 = 13;
+pub const exdrOK: i32 = 0;
+pub const exdrHEADER: i32 = 1;
+pub const exdrSTRING: i32 = 2;
+pub const exdrDOUBLE: i32 = 3;
+pub const exdrINT: i32 = 4;
+pub const exdrFLOAT: i32 = 5;
+pub const exdrUINT: i32 = 6;
+pub const exdr3DX: i32 = 7;
+pub const exdrCLOSE: i32 = 8;
+pub const exdrMAGIC: i32 = 9;
+pub const exdrNOMEM: i32 = 10;
+pub const exdrENDOFFILE: i32 = 11;
+pub const exdrFILENOTFOUND: i32 = 12;
+pub const exdrNR: i32 = 13;
 
 extern "C" {
     pub static mut exdr_message: [*mut ::std::os::raw::c_char; 13usize];

src/c_abi/xdrfile_trr.rs

@@ -65,7 +65,7 @@ mod tests {
 
         unsafe {
             let code = read_trr_nframes(path.as_ptr(), &mut nframes);
-            assert!(code as u32 == exdrOK);
+            assert!(code == exdrOK);
         }
         assert!(nframes == 38, "{:?}", nframes);
         Ok(())
@@ -106,7 +106,7 @@ mod tests {
                 v.as_ptr() as *mut Rvec,
                 f.as_ptr() as *mut Rvec,
             );
-            assert!(write_code as u32 == exdrOK);
+            assert!(write_code == exdrOK);
             xdrfile_close(xdr);
         }
 
@@ -134,7 +134,7 @@ mod tests {
                 v2.as_ptr() as *mut Rvec,
                 f2.as_ptr() as *mut Rvec,
             );
-            assert!(read_code as u32 == exdrOK);
+            assert!(read_code == exdrOK);
             xdrfile_close(xdr);
         }
 

src/c_abi/xdrfile_xtc.rs

@@ -29,8 +29,8 @@ extern "C" {
         natoms: ::std::os::raw::c_int,
         step: ::std::os::raw::c_int,
         time: ::std::os::raw::c_float,
-        box_vec: *mut Matrix,
-        x: *mut Rvec,
+        box_vec: *const Matrix,
+        x: *const Rvec,
         prec: ::std::os::raw::c_float,
     ) -> ::std::os::raw::c_int;
 }
@@ -61,7 +61,7 @@ mod tests {
 
         unsafe {
             let code = read_xtc_nframes(path.as_ptr(), &mut nframes);
-            assert!(code as u32 == exdrOK);
+            assert!(code == exdrOK);
         }
         assert!(nframes == 38, "{:?}", nframes);
         Ok(())
@@ -96,7 +96,7 @@ mod tests {
                 x.as_ptr() as *mut Rvec,
                 1000.0,
             );
-            assert!(write_code as u32 == exdrOK);
+            assert!(write_code == exdrOK);
             xdrfile_close(xdr);
         }
 
@@ -119,7 +119,7 @@ mod tests {
                 x2.as_ptr() as *mut Rvec,
                 &mut prec,
             );
-            assert!(read_code as u32 == exdrOK);
+            assert!(read_code == exdrOK);
             xdrfile_close(xdr);
         }
 

src/errors.rs

@@ -8,25 +8,22 @@ use std::path::{Path, PathBuf};
 #[derive(Debug, Clone, PartialEq)]
 pub enum Error {
     /// An error code from the C API
-    CApiError {
-        code: ErrorCode,
-        task: ErrorTask,
-    },
+    CApiError { code: ErrorCode, task: ErrorTask },
     /// Passed in a frame of the wrong size
-    WrongSizeFrame {
-        expected: usize,
-        found: usize,
-    },
+    WrongSizeFrame { expected: usize, found: usize },
     /// C API failed to open a file (No return code provided)
-    CouldNotOpen {
-        path: PathBuf,
-        mode: FileMode,
-    },
+    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<std::ffi::NulError>),
+    /// Checking the number of atoms failed while reading a frame
     CouldNotCheckNAtoms(Box<Error>),
+    /// Error for an out-of-range numeric conversion
+    OutOfRange {
+        name: &'static str,
+        task: ErrorTask,
+        value: String,
+        target: &'static str,
+    },
 }
 
 impl Error {
@@ -60,10 +57,15 @@ impl Error {
 
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use Error::*;
         match &self {
-            NullInStr(err) => Some(err),
-            CouldNotCheckNAtoms(err) => Some(err.as_ref()),
+            Error::InvalidOsStr(err) => {
+                if let Some(err) = err {
+                    Some(err)
+                } else {
+                    None
+                }
+            },
+            Error::CouldNotCheckNAtoms(err) => Some(err.as_ref()),
             _ => None,
         }
     }
@@ -76,12 +78,6 @@ impl From<(ErrorCode, ErrorTask)> for Error {
     }
 }
 
-impl From<std::ffi::NulError> 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;
@@ -104,28 +100,38 @@ impl From<(&Frame, usize)> for Error {
 
 impl std::fmt::Display for Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use Error::*;
-        match &self {
-            CApiError { code, task } => write!(
+        match self {
+            Error::CApiError { code, task } => write!(
                 f,
                 "Error while {task}: C API returned error code {code}",
                 task = task,
                 code = code
             ),
-            WrongSizeFrame { expected, found } => write!(
+            Error::WrongSizeFrame { expected, found } => write!(
                 f,
                 "Expected frame of size {:?}, found {:?}",
                 expected, found
             ),
-            CouldNotOpen { path, mode } => {
+            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) => write!(
+            Error::InvalidOsStr(_) => write!(f, "Cannot convert path to CString."),
+            Error::CouldNotCheckNAtoms(_) => {
+                write!(f, "Failed to read number of atoms in trajectory file")
+            }
+            Error::OutOfRange {
+                name,
+                task,
+                value,
+                target,
+            } => write!(
                 f,
-                "Failed to read number of atoms in trajectory file"
-            )
+                "Illegal {name} while {task}: Failed to cast {value} to {target}",
+                name = name,
+                task = task,
+                value = value,
+                target = target
+            ),
         }
     }
 }
@@ -147,13 +153,12 @@ pub enum ErrorTask {
 
 impl std::fmt::Display for ErrorTask {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use ErrorTask::*;
         match &self {
-            ReadNumAtoms => write!(f, "reading atom number from trajectory"),
-            Read => write!(f, "reading trajectory"),
-            Write => write!(f, "writing trajectory"),
-            Flush => write!(f, "flushing trajectory"),
-            Seek => write!(f, "seeking in trajectory"),
+            ErrorTask::ReadNumAtoms => write!(f, "reading atom number from trajectory"),
+            ErrorTask::Read => write!(f, "reading trajectory"),
+            ErrorTask::Write => write!(f, "writing trajectory"),
+            ErrorTask::Flush => write!(f, "flushing trajectory"),
+            ErrorTask::Seek => write!(f, "seeking in trajectory"),
         }
     }
 }
@@ -190,7 +195,7 @@ pub enum ErrorCode {
     /// Failed to seek within file
     ExdrNr,
     /// Something unexpected happened
-    UnmatchedCode(c_abi::xdrfile::BindgenTy1),
+    UnmatchedCode(i32),
 }
 
 impl ErrorCode {
@@ -200,8 +205,8 @@ impl ErrorCode {
     }
 }
 
-impl From<c_abi::xdrfile::BindgenTy1> for ErrorCode {
-    fn from(code: c_abi::xdrfile::BindgenTy1) -> Self {
+impl From<i32> for ErrorCode {
+    fn from(code: i32) -> Self {
         match code {
             c_abi::xdrfile::exdrOK => Self::ExdrOk,
             c_abi::xdrfile::exdrHEADER => Self::ExdrHeader,
@@ -238,37 +243,34 @@ pub type Result<T, E = Error> = std::result::Result<T, E>;
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::ffi::CString;
-    use std::ffi::NulError;
 
     #[test]
     fn test_is_eof() {
-        use Error::CApiError;
-        let error = CApiError {
+        let error = Error::CApiError {
             code: c_abi::xdrfile::exdrENDOFFILE.into(),
             task: ErrorTask::Read,
         };
         assert!(error.is_eof());
 
-        let error = CApiError {
+        let error = Error::CApiError {
             code: ErrorCode::ExdrEndOfFile,
             task: ErrorTask::Read,
         };
         assert!(error.is_eof());
 
-        let error = CApiError {
+        let error = Error::CApiError {
             code: (c_abi::xdrfile::exdrENDOFFILE + 1).into(),
             task: ErrorTask::Read,
         };
         assert!(!error.is_eof());
 
-        let error = CApiError {
+        let error = Error::CApiError {
             code: 0.into(),
             task: ErrorTask::Read,
         };
         assert!(!error.is_eof());
 
-        let error = CApiError {
+        let error = Error::CApiError {
             code: 255.into(),
             task: ErrorTask::Read,
         };
@@ -283,12 +285,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 };

src/frame.rs

@@ -28,9 +28,7 @@ impl Default for Frame {
 impl Frame {
     /// Creates an empty frame with a capacity of 0
     pub fn new() -> Frame {
-        Frame {
-            ..Default::default()
-        }
+        Default::default()
     }
 
     /// Creates a frame with the given capacity
@@ -55,6 +53,11 @@ impl Frame {
 
     /// Length of the frame (number of atoms)
     pub fn len(self: &Frame) -> usize {
+        self.num_atoms()
+    }
+
+    /// The number of atoms in the frame
+    pub fn num_atoms(self: &Frame) -> usize {
         self.coords.len()
     }
 

src/iterator.rs

@@ -32,11 +32,10 @@ impl IntoIterator for TRRTrajectory {
     }
 }
 
-/*
-Iterator for trajectories. This iterator yields a Result<Frame, Error>
-for each frame in the trajectory file and stops with yielding None once the
-trajectory is finished. Also yields None after the first occurence of an error
-*/
+/// Iterator for trajectories.
+/// This iterator yields a Result<Frame, Error> for each frame in the
+/// trajectory file and stops with yielding None once the trajectory is
+/// EOF. Also yields None after the first occurrence of an error
 pub struct TrajectoryIterator<T> {
     trajectory: T,
     item: Rc<Frame>,

src/lib.rs

@@ -77,12 +77,14 @@ use c_abi::xdrfile_xtc;
 
 use lazy_init::Lazy;
 use std::cell::Cell;
+use std::convert::{TryFrom, TryInto};
 use std::ffi::CString;
 use std::io;
-use std::path::{Path, PathBuf};
-use std::convert::TryInto;
 use std::io::SeekFrom;
+use std::os::raw::{c_float, c_int};
+use std::path::{Path, PathBuf};
 
+/// File Mode for accessing trajectories.
 #[derive(Debug, Clone, PartialEq)]
 pub enum FileMode {
     Write,
@@ -104,16 +106,37 @@ impl FileMode {
 }
 
 fn path_to_cstring(path: impl AsRef<Path>) -> Result<CString> {
-    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<I, O>(value: I, task: ErrorTask, name: &'static str) -> Result<O>
+where
+    I: TryInto<O> + std::fmt::Display + Copy,
+{
+    value.try_into().map_err(|_| Error::OutOfRange {
+        name,
+        value: format!("{}", &value),
+        target: std::any::type_name::<O>(),
+        task,
+    })
+}
+
+macro_rules! to {
+    ($value:expr, $task:expr) => {
+        to($value, $task, stringify!($value))
+    };
 }
 
 /// Convert an error code from a C call to an Error
 ///
 /// `code` should be an integer return code returned from the C API.
-/// If `code` indicates the function returned successfully, Nothing is returned;
+/// If `code` indicates the function returned successfully, None is returned;
 /// otherwise, the code is converted into the appropriate `Error`.
-pub fn check_code(code: impl Into<ErrorCode>, task: ErrorTask) -> Option<Error> {
+fn check_code(code: impl Into<ErrorCode>, task: ErrorTask) -> Option<Error> {
     let code: ErrorCode = code.into();
     if let ErrorCode::ExdrOk = code {
         None
@@ -170,12 +193,15 @@ impl XDRFile {
 impl io::Seek for XDRFile {
     fn seek(&mut self, pos: io::SeekFrom) -> io::Result<u64> {
         let (whence, pos) = match pos {
-            SeekFrom::Start(u) => (0, u as i64),
+            SeekFrom::Start(u) => (
+                0,
+                i64::try_from(u).expect("Seek position did not fit in i64"),
+            ),
             SeekFrom::Current(i) => (1, i),
             SeekFrom::End(i) => (2, i),
         };
         unsafe {
-            let code = xdr_seek::xdr_seek(self.xdrfile, pos, whence) as u32;
+            let code = xdr_seek::xdr_seek(self.xdrfile, pos, whence);
             match check_code(code, ErrorTask::Seek) {
                 None => Ok(self.tell()),
                 Some(err) => Err(io::Error::new(io::ErrorKind::Other, err)),
@@ -208,10 +234,10 @@ pub trait Trajectory {
     fn get_num_atoms(&mut self) -> Result<usize>;
 }
 
-/// Read/Write XTC Trajectories
+/// Handle to Read/Write XTC Trajectories
 pub struct XTCTrajectory {
     handle: XDRFile,
-    precision: Cell<f32>, // internal mutability required for read method
+    precision: Cell<c_float>, // internal mutability required for read method
     num_atoms: Lazy<Result<usize>>,
 }
 
@@ -243,34 +269,30 @@ impl XTCTrajectory {
 
 impl Trajectory for XTCTrajectory {
     fn read(&mut self, frame: &mut Frame) -> Result<()> {
-        let mut step: i32 = 0;
+        let mut step: c_int = 0;
 
         let num_atoms = self
             .get_num_atoms()
             .map_err(|e| Error::CouldNotCheckNAtoms(Box::new(e)))?;
         if num_atoms != frame.coords.len() {
             Err((&*frame, num_atoms))?;
-        }
+        };
 
         unsafe {
-            // C lib requires an i32 to be passed, but step is exposed it as u32
-            // (A step cannot be negative, can it?). So we need to create a step
-            // variable to pass to read_xtc and cast it afterwards to u32
             let code = xdrfile_xtc::read_xtc(
                 self.handle.xdrfile,
-                num_atoms as i32,
+                to!(num_atoms, ErrorTask::Read)?,
                 &mut step,
                 &mut frame.time,
                 &mut frame.box_vector,
                 frame.coords.as_mut_ptr(),
                 &mut self.precision.get(),
-            ) as u32;
-            frame.step = step as usize;
+            );
             if let Some(err) = check_code(code, ErrorTask::Read) {
-                Err(err)
-            } else {
-                Ok(())
+                return Err(err);
             }
+            frame.step = to!(step, ErrorTask::Read)?;
+            Ok(())
         }
     }
 
@@ -278,13 +300,13 @@ impl Trajectory for XTCTrajectory {
         unsafe {
             let code = xdrfile_xtc::write_xtc(
                 self.handle.xdrfile,
-                frame.len() as i32,
-                frame.step as i32,
+                to!(frame.num_atoms(), ErrorTask::Write)?,
+                to!(frame.step, ErrorTask::Write)?,
                 frame.time,
-                frame.box_vector.as_ptr() as *mut [[f32; 3]; 3],
-                frame.coords[..].as_ptr() as *mut [f32; 3],
+                &frame.box_vector,
+                frame.coords.as_ptr(),
                 1000.0,
-            ) as u32;
+            );
             if let Some(err) = check_code(code, ErrorTask::Write) {
                 Err(err)
             } else {
@@ -295,8 +317,8 @@ impl Trajectory for XTCTrajectory {
 
     fn flush(&mut self) -> Result<()> {
         unsafe {
-            let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32;
-            if let Some(err) = check_code(code, ErrorTask::Read) {
+            let code = xdr_seek::xdr_flush(self.handle.xdrfile);
+            if let Some(err) = check_code(code, ErrorTask::Flush) {
                 Err(err)
             } else {
                 Ok(())
@@ -307,20 +329,19 @@ impl Trajectory for XTCTrajectory {
     fn get_num_atoms(&mut self) -> Result<usize> {
         self.num_atoms
             .get_or_create(|| {
-                let mut num_atoms: i32 = 0;
+                let mut num_atoms: c_int = 0;
 
                 unsafe {
                     let path = path_to_cstring(&self.handle.path)?;
                     let path_p = path.into_raw();
-                    let code =
-                        xdrfile_xtc::read_xtc_natoms(path_p, &mut num_atoms as *const i32) as u32;
+                    let code = xdrfile_xtc::read_xtc_natoms(path_p, &mut num_atoms);
                     // Reconstitute the CString so it is deallocated correctly
                     let _ = CString::from_raw(path_p);
 
                     if let Some(err) = check_code(code, ErrorTask::ReadNumAtoms) {
                         Err(err)
                     } else {
-                        Ok(num_atoms as usize)
+                        to!(num_atoms, ErrorTask::ReadNumAtoms)
                     }
                 }
             })
@@ -341,7 +362,7 @@ impl io::Seek for XTCTrajectory {
     }
 }
 
-/// Read/Write TRR Trajectories
+/// Handle to Read/Write TRR Trajectories
 pub struct TRRTrajectory {
     handle: XDRFile,
     num_atoms: Lazy<Result<usize>>,
@@ -374,8 +395,8 @@ impl TRRTrajectory {
 
 impl Trajectory for TRRTrajectory {
     fn read(&mut self, frame: &mut Frame) -> Result<()> {
-        let mut step: i32 = 0;
-        let mut lambda: f32 = 0.0;
+        let mut step: c_int = 0;
+        let mut lambda: c_float = 0.0;
 
         let num_atoms = self
             .get_num_atoms()
@@ -385,13 +406,9 @@ impl Trajectory for TRRTrajectory {
         }
 
         unsafe {
-            // C lib requires an i32 to be passed, but step is exposed it as u32
-            // (A step cannot be negative, can it?). So we need to create a step
-            // variable to pass to read_trr and cast it afterwards to u32.
-            // Similar for lambda.
             let code = xdrfile_trr::read_trr(
                 self.handle.xdrfile,
-                num_atoms as i32,
+                to!(num_atoms, ErrorTask::Read)?,
                 &mut step,
                 &mut frame.time,
                 &mut lambda,
@@ -399,14 +416,12 @@ impl Trajectory for TRRTrajectory {
                 frame.coords.as_mut_ptr(),
                 std::ptr::null_mut(),
                 std::ptr::null_mut(),
-            ) as u32;
-
-            frame.step = step as usize;
+            );
             if let Some(err) = check_code(code, ErrorTask::Read) {
-                Err(err)
-            } else {
-                Ok(())
+                return Err(err);
             }
+            frame.step = to!(step, ErrorTask::Read)?;
+            Ok(())
         }
     }
 
@@ -414,15 +429,15 @@ impl Trajectory for TRRTrajectory {
         unsafe {
             let code = xdrfile_trr::write_trr(
                 self.handle.xdrfile,
-                frame.len() as i32,
-                frame.step as i32,
+                to!(frame.len(), ErrorTask::Write)?,
+                to!(frame.step, ErrorTask::Write)?,
                 frame.time,
                 0.0,
-                frame.box_vector.as_ptr() as *mut [[f32; 3]; 3],
-                frame.coords[..].as_ptr() as *mut [f32; 3],
+                &frame.box_vector,
+                frame.coords[..].as_ptr(),
                 std::ptr::null_mut(),
                 std::ptr::null_mut(),
-            ) as u32;
+            );
             if let Some(err) = check_code(code, ErrorTask::Write) {
                 Err(err)
             } else {
@@ -433,7 +448,7 @@ impl Trajectory for TRRTrajectory {
 
     fn flush(&mut self) -> Result<()> {
         unsafe {
-            let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32;
+            let code = xdr_seek::xdr_flush(self.handle.xdrfile);
             if let Some(err) = check_code(code, ErrorTask::Flush) {
                 Err(err)
             } else {
@@ -445,19 +460,18 @@ impl Trajectory for TRRTrajectory {
     fn get_num_atoms(&mut self) -> Result<usize> {
         self.num_atoms
             .get_or_create(|| {
-                let mut num_atoms: i32 = 0;
+                let mut num_atoms: c_int = 0;
                 unsafe {
                     let path = path_to_cstring(&self.handle.path)?;
                     let path_p = path.into_raw();
-                    let code =
-                        xdrfile_trr::read_trr_natoms(path_p, &mut num_atoms as *const i32) as u32;
+                    let code = xdrfile_trr::read_trr_natoms(path_p, &mut num_atoms);
                     // Reconstitute the CString so it is deallocated correctly
                     let _ = CString::from_raw(path_p);
 
                     if let Some(err) = check_code(code, ErrorTask::ReadNumAtoms) {
                         Err(err)
                     } else {
-                        Ok(num_atoms as usize)
+                        to!(num_atoms, ErrorTask::ReadNumAtoms)
                     }
                 }
             })
@@ -482,9 +496,9 @@ impl io::Seek for TRRTrajectory {
 mod tests {
 
     use super::*;
-    use tempfile::NamedTempFile;
     use std::io::Seek;
     use std::io::Write;
+    use tempfile::NamedTempFile;
 
     #[test]
     fn test_read_write_xtc() -> Result<()> {
@@ -530,7 +544,7 @@ mod tests {
         let tempfile = NamedTempFile::new().expect("Could not create temporary file");
         let tmp_path = tempfile.path();
 
-        let natoms: u32 = 2;
+        let natoms = 2;
         let frame = Frame {
             step: 5,
             time: 2.0,
@@ -545,7 +559,7 @@ mod tests {
         }
         f.flush()?;
 
-        let mut new_frame = Frame::with_len(natoms as usize);
+        let mut new_frame = Frame::with_len(natoms);
         let mut f = TRRTrajectory::open_read(tmp_path)?;
         // let num_atoms = f.get_num_atoms()?;
         // assert_eq!(num_atoms, natoms);
@@ -611,21 +625,28 @@ mod tests {
 
     #[test]
     fn test_path_to_cstring() -> Result<(), Box<dyn std::error::Error>> {
-        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(())
     }
 
@@ -744,7 +765,7 @@ mod tests {
         let tempfile = NamedTempFile::new()?;
         let tmp_path = tempfile.path();
 
-        let natoms: u32 = 2;
+        let natoms = 2;
         let frame = Frame {
             step: 5,
             time: 2.0,
@@ -755,7 +776,7 @@ mod tests {
         f.write(&frame)?;
         f.flush()?;
 
-        let mut new_frame = Frame::with_len(natoms as usize);
+        let mut new_frame = Frame::with_len(natoms);
         let mut f = XTCTrajectory::open_read(tmp_path)?;
 
         f.read(&mut new_frame)?;
@@ -792,4 +813,56 @@ mod tests {
             assert!(check_code(code, ErrorTask::Read).is_some());
         }
     }
+
+    #[test]
+    fn test_to() -> Result<()> {
+        assert_eq!(24234_i32, to!(24234_usize, ErrorTask::Write)?);
+
+        let big_number = 3_294_967_295_usize;
+        let expected: Result<i32> = Err(Error::OutOfRange {
+            name: "big_number",
+            task: ErrorTask::Write,
+            value: "3294967295".to_string(),
+            target: "i32",
+        });
+        assert_eq!(expected, to!(big_number, ErrorTask::Write));
+
+        let num_atoms: usize = 304;
+        let res: Result<u8, _> = to!(num_atoms, ErrorTask::Write);
+        assert_eq!(
+            format!("{}", res.unwrap_err()),
+            "Illegal num_atoms while writing trajectory: Failed to cast 304 to u8"
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_write_outofrange_step() -> Result<(), Box<dyn std::error::Error>> {
+        let tempfile = NamedTempFile::new()?;
+        let tmp_path = tempfile.path();
+        let mut traj = XTCTrajectory::open_write(tmp_path)?;
+
+        let frame = Frame {
+            step: usize::MAX,
+            time: 0.0,
+            box_vector: [[0.0; 3]; 3],
+            coords: vec![[1.0; 3]],
+        };
+        let expected = Error::OutOfRange {
+            name: "frame.step",
+            value: usize::MAX.to_string(),
+            target: "i32",
+            task: ErrorTask::Write,
+        };
+
+        if let Err(e) = traj.write(&frame) {
+            print!("{:?}", e);
+            assert_eq!(expected, e);
+        } else {
+            panic!("Writing frame with step=usize::MAX should not succeed.")
+        }
+
+        Ok(())
+    }
 }

tests/integration.rs

@@ -8,7 +8,7 @@ mod integration {
     fn test_use_library() -> Result<()> {
         let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
         let num_atoms = trj.get_num_atoms()?;
-        let mut frame = Frame::with_len(num_atoms as usize);
+        let mut frame = Frame::with_len(num_atoms);
 
         trj.read(&mut frame)?;
         trj.read(&mut frame)?;
@@ -21,7 +21,7 @@ mod integration {
         let trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
         let frames: Result<Vec<Rc<Frame>>> = trj.into_iter().collect();
         for (idx, frame) in frames?.iter().enumerate() {
-            assert_eq!(frame.step as usize, idx + 1);
+            assert_eq!(frame.step, idx + 1);
         }
         Ok(())
     }