Merge pull request #4 from Yoshanuikabundi/no-unwrap

Removed unwrap()s, refactored iterator, removed memleak

README.md

@@ -12,29 +12,27 @@ files with a safe api.
 ```rust
 use xdrfile::*;
 
-// get a handle to the file
+fn main() -> Result<()> {
-let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+    // get a handle to the file
+    let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
 
-// find number of atoms in the file
+    // find number of atoms in the file
-let num_atoms = trj.get_num_atoms().unwrap();
+    let num_atoms = trj.get_num_atoms()?;
 
-// a frame object is used to get to read or write from a trajectory
+    // a frame object is used to get to read or write from a trajectory
-// without instantiating data arrays for every step
+    // without instantiating data arrays for every step
-let mut frame = Frame::with_capacity(num_atoms);
+    let mut frame = Frame::with_capacity(num_atoms);
+
+    // read the first frame of the trajectory
+    trj.read(&mut frame)?;
 
-// read the first frame of the trajectory
-let result = trj.read(&mut frame);
-match result {
-   Ok(_) => {
     assert_eq!(frame.step, 1);
     assert_eq!(frame.num_atoms, num_atoms);
 
     let first_atom_coords = frame.coords[0];
     assert_eq!(first_atom_coords, [-0.8901, 0.4127, -0.055499997]);
-   }
+
-   Err(msg) => {
+    Ok(())
-       panic!("Something went wrong: {}", msg);    
-   }
 }
 ```
 
@@ -47,13 +45,17 @@ Rc is required)
 ```rust
 use xdrfile::*;
 
-// get a handle to the file
+fn main() -> Result<()> {
-let trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+    // get a handle to the file
+    let trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
 
-// iterate over all frames
+    // iterate over all frames
-for (idx, frame) in trj.into_iter().filter_map(Result::ok).enumerate() {
+    for (idx, result) in trj.into_iter().enumerate() {
+        let frame = result?;
         println!("{}", frame.time);
         assert_eq!(idx+1, frame.step as usize);
+    }
+    Ok(())
 }
 ```
 

build.rs

@@ -1,17 +1,17 @@
 extern crate cc;
 
 use std::fs;
+use std::io::Result;
 
-fn main() {
+fn main() -> Result<()> {
     // This builds gromacs' xdrfile library
-    let source_files: Vec<_> = fs::read_dir("external/xdrfile/src")
+    let source_files = fs::read_dir("external/xdrfile/src")?
-        .unwrap()
+        .map(|r| r.map(|f| f.path()))
-        .map(|f| f.unwrap())
+        .collect::<Result<Vec<_>>>()?;
-        .map(|f| f.path())
-        .collect();
     cc::Build::new()
         .files(source_files)
         .include("external/xdrfile/include")
         .warnings(false)
-        .compile("libxdrfile.a")
+        .compile("libxdrfile.a");
+    Ok(())
 }

src/c_abi/xdr_seek.rs

@@ -111,8 +111,8 @@ mod tests {
     use std::ffi::CString;
 
     #[test]
-    fn test_xdr_tell() {
+    fn test_xdr_tell() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.xtc").unwrap();
+        let path = CString::new("tests/1l2y.xtc")?;
         let num_atoms = 304;
         let mut time: f32 = 2.0;
         let mut step: i32 = 5;
@@ -121,7 +121,7 @@ mod tests {
         let mut prec: f32 = 0.0;
 
         unsafe {
-            let mode = CString::new("r").unwrap();
+            let mode = CString::new("r")?;
             let xdr = xdrfile_open(path.as_ptr(), mode.as_ptr());
             assert!(!xdr.is_null());
 
@@ -140,15 +140,16 @@ mod tests {
 
             let tell = xdr_tell(xdr);
             assert!(tell > 0, "{}", tell);
-        }
+        };
+        Ok(())
     }
 
     #[test]
-    fn test_xdr_seek() {
+    fn test_xdr_seek() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.xtc").unwrap();
+        let path = CString::new("tests/1l2y.xtc")?;
 
         unsafe {
-            let mode = CString::new("r").unwrap();
+            let mode = CString::new("r")?;
             let xdr = xdrfile_open(path.as_ptr(), mode.as_ptr());
             assert!(!xdr.is_null());
 
@@ -160,5 +161,6 @@ mod tests {
             let tell = xdr_tell(xdr);
             assert!(tell == 500, "{}", tell);
         }
+        Ok(())
     }
 }

src/c_abi/xdrfile_trr.rs

@@ -47,19 +47,20 @@ mod tests {
     use tempfile::NamedTempFile;
 
     #[test]
-    fn test_read_trr_natoms() {
+    fn test_read_trr_natoms() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.trr").unwrap();
+        let path = CString::new("tests/1l2y.trr")?;
 
         let mut natoms = 0;
         unsafe {
             read_trr_natoms(path.as_ptr() as *const i8, &mut natoms);
         }
         assert!(natoms == 304);
+        Ok(())
     }
 
     #[test]
-    fn test_read_trr_nframes() {
+    fn test_read_trr_nframes() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.trr").unwrap();
+        let path = CString::new("tests/1l2y.trr")?;
         let mut nframes: u64 = 0;
 
         unsafe {
@@ -67,12 +68,18 @@ mod tests {
             assert!(code as u32 == exdrOK);
         }
         assert!(nframes == 38, "{:?}", nframes);
+        Ok(())
     }
 
     #[test]
-    fn test_read_write_trr() {
+    fn test_read_write_trr() -> Result<(), Box<dyn std::error::Error>> {
-        let tempfile = NamedTempFile::new().unwrap();
+        let tempfile = NamedTempFile::new()?;
-        let tmp_path = CString::new(tempfile.path().to_str().unwrap()).unwrap();
+        let tmp_path = CString::new(
+            tempfile
+                .path()
+                .to_str()
+                .expect("Could not convert path to str"),
+        )?;
 
         // write atoms to tempfile
         let natoms: i32 = 2;
@@ -86,7 +93,7 @@ mod tests {
         let f: Vec<Rvec> = vec![[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]];
 
         unsafe {
-            let mode = CString::new("w").unwrap();
+            let mode = CString::new("w")?;
             let xdr = xdrfile_open(tmp_path.as_ptr(), mode.as_ptr());
             let write_code = write_trr(
                 xdr,
@@ -114,7 +121,7 @@ mod tests {
         let f2: Vec<Rvec> = vec![[0.0, 0.0, 0.0]; 2];
 
         unsafe {
-            let mode = CString::new("r").unwrap();
+            let mode = CString::new("r")?;
             let xdr = xdrfile_open(tmp_path.as_ptr(), mode.as_ptr());
             let read_code = read_trr(
                 xdr,
@@ -139,5 +146,6 @@ mod tests {
         assert!(x2 == x);
         assert!(v2 == v);
         assert!(f2 == f);
+        Ok(())
     }
 }

src/c_abi/xdrfile_xtc.rs

@@ -43,19 +43,20 @@ mod tests {
     use tempfile::NamedTempFile;
 
     #[test]
-    fn test_read_xtc_natoms() {
+    fn test_read_xtc_natoms() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.xtc").unwrap();
+        let path = CString::new("tests/1l2y.xtc")?;
 
         let mut natoms = 0;
         unsafe {
             read_xtc_natoms(path.as_ptr() as *mut i8, &mut natoms);
         }
         assert!(natoms == 304);
+        Ok(())
     }
 
     #[test]
-    fn test_read_xtc_nframes() {
+    fn test_read_xtc_nframes() -> Result<(), Box<dyn std::error::Error>> {
-        let path = CString::new("tests/1l2y.xtc").unwrap();
+        let path = CString::new("tests/1l2y.xtc")?;
         let mut nframes: u64 = 0;
 
         unsafe {
@@ -63,12 +64,18 @@ mod tests {
             assert!(code as u32 == exdrOK);
         }
         assert!(nframes == 38, "{:?}", nframes);
+        Ok(())
     }
 
     #[test]
-    fn test_read_write_xtc() {
+    fn test_read_write_xtc() -> Result<(), Box<dyn std::error::Error>> {
-        let tempfile = NamedTempFile::new().unwrap();
+        let tempfile = NamedTempFile::new()?;
-        let tmp_path = CString::new(tempfile.path().to_str().unwrap()).unwrap();
+        let tmp_path = CString::new(
+            tempfile
+                .path()
+                .to_str()
+                .expect("Could not convert path to str"),
+        )?;
 
         // write atoms to tempfile
         let natoms: i32 = 2;
@@ -78,7 +85,7 @@ mod tests {
         let x: Vec<Rvec> = vec![[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]];
 
         unsafe {
-            let mode = CString::new("w").unwrap();
+            let mode = CString::new("w")?;
             let xdr = xdrfile_open(tmp_path.as_ptr(), mode.as_ptr());
             let write_code = write_xtc(
                 xdr,
@@ -101,7 +108,7 @@ mod tests {
         let mut prec: f32 = 0.0;
 
         unsafe {
-            let mode = CString::new("r").unwrap();
+            let mode = CString::new("r")?;
             let xdr = xdrfile_open(tmp_path.as_ptr(), mode.as_ptr());
             let read_code = read_xtc(
                 xdr,
@@ -121,5 +128,6 @@ mod tests {
         assert!(time2 == time);
         assert!(box_vec2 == box_vec);
         assert!(x2 == x);
+        Ok(())
     }
 }

src/iterator.rs

@@ -1,74 +1,35 @@
-use crate::c_abi::xdrfile::exdrENDOFFILE;
 use crate::*;
 use std::rc::Rc;
 
 impl IntoIterator for XTCTrajectory {
     type Item = Result<Rc<Frame>>;
-    type IntoIter = XTCTrajectoryIterator;
+    type IntoIter = TrajectoryIterator<XTCTrajectory>;
 
     fn into_iter(mut self) -> Self::IntoIter {
-        // TODO this should be handled without the requirement of unwrap
+        let frame = match self.get_num_atoms() {
-        let num_atoms = self.get_num_atoms().unwrap();
+            Ok(num_atoms) => Frame::with_capacity(num_atoms),
-        XTCTrajectoryIterator {
+            Err(_) => Frame::new(),
-            trajectory: self,
-            item: Rc::new(Frame::with_capacity(num_atoms)),
-            has_error: false,
-        }
-    }
-}
-
-/*
-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
-*/
-pub struct XTCTrajectoryIterator {
-    trajectory: XTCTrajectory,
-    item: Rc<Frame>,
-    has_error: bool,
-}
-
-impl Iterator for XTCTrajectoryIterator {
-    type Item = Result<Rc<Frame>>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        // Reuse old frame
-        if self.has_error {
-            return None;
-        }
-
-        let item: &mut Frame = match Rc::get_mut(&mut self.item) {
-            Some(item) => item,
-            None => {
-                // caller kept frame. Create new one
-                self.item = Rc::new(Frame::with_capacity(self.item.num_atoms));
-                Rc::get_mut(&mut self.item).unwrap()
-            }
         };
-        match self.trajectory.read(item) {
+        TrajectoryIterator {
-            Ok(()) => Some(Ok(Rc::clone(&self.item))),
+            trajectory: self,
-            Err(msg) => {
+            item: Rc::new(frame),
-                if msg.to_string().contains(&exdrENDOFFILE.to_string()) {
+            has_error: false,
-                    None
-                } else {
-                    self.has_error = true;
-                    Some(Err(msg))
-                }
-            }
         }
     }
 }
 
 impl IntoIterator for TRRTrajectory {
     type Item = Result<Rc<Frame>>;
-    type IntoIter = TRRTrajectoryIterator;
+    type IntoIter = TrajectoryIterator<TRRTrajectory>;
 
     fn into_iter(mut self) -> Self::IntoIter {
-        // TODO this should be handled without the requirement of unwrap
+        let frame = match self.get_num_atoms() {
-        let num_atoms = self.get_num_atoms().unwrap();
+            Ok(num_atoms) => Frame::with_capacity(num_atoms),
-        TRRTrajectoryIterator {
+            Err(_) => Frame::new(),
+        };
+        TrajectoryIterator {
             trajectory: self,
-            item: Rc::new(Frame::with_capacity(num_atoms)),
+            item: Rc::new(frame),
             has_error: false,
         }
     }
@@ -79,13 +40,16 @@ 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
 */
-pub struct TRRTrajectoryIterator {
+pub struct TrajectoryIterator<T> {
-    trajectory: TRRTrajectory,
+    trajectory: T,
     item: Rc<Frame>,
     has_error: bool,
 }
 
-impl Iterator for TRRTrajectoryIterator {
+impl<T> Iterator for TrajectoryIterator<T>
+where
+    T: Trajectory,
+{
     type Item = Result<Rc<Frame>>;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -99,18 +63,15 @@ impl Iterator for TRRTrajectoryIterator {
             None => {
                 // caller kept frame. Create new one
                 self.item = Rc::new(Frame::with_capacity(self.item.num_atoms));
-                Rc::get_mut(&mut self.item).unwrap()
+                Rc::get_mut(&mut self.item).expect("Could not get mutable access to new Rc")
             }
         };
         match self.trajectory.read(item) {
             Ok(()) => Some(Ok(Rc::clone(&self.item))),
-            Err(msg) => {
+            Err(e) if e.is_eof() => None,
-                if msg.to_string().contains(&exdrENDOFFILE.to_string()) {
+            Err(e) => {
-                    None
-                } else {
                 self.has_error = true;
-                    Some(Err(msg))
+                Some(Err(e))
-                }
             }
         }
     }
@@ -121,20 +82,24 @@ mod tests {
     use super::*;
 
     #[test]
-    pub fn test_xtc_trajectory_iterator() {
+    pub fn test_xtc_trajectory_iterator() -> Result<()> {
-        let traj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+        let traj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
-        let frames: Vec<Rc<Frame>> = traj.into_iter().filter_map(Result::ok).collect();
+        let frames: Result<Vec<Rc<Frame>>> = traj.into_iter().collect();
+        let frames = frames?;
         assert!(frames.len() == 38);
         assert!(frames[0].step == 1, frames[0].step);
         assert!(frames[37].step == 38);
+        Ok(())
     }
 
     #[test]
-    pub fn test_trr_trajectory_iterator() {
+    pub fn test_trr_trajectory_iterator() -> Result<()> {
-        let traj = TRRTrajectory::open_read("tests/1l2y.trr").unwrap();
+        let traj = TRRTrajectory::open_read("tests/1l2y.trr")?;
-        let frames: Vec<Rc<Frame>> = traj.into_iter().filter_map(Result::ok).collect();
+        let frames: Result<Vec<Rc<Frame>>> = traj.into_iter().collect();
+        let frames = frames?;
         assert!(frames.len() == 38);
         assert!(frames[0].step == 1, frames[0].step);
         assert!(frames[37].step == 38);
+        Ok(())
     }
 }

src/lib.rs

@@ -9,29 +9,27 @@
 //! ```rust
 //! use xdrfile::*;
 //!
+//! fn main() -> Result<()> {
 //!     // get a handle to the file
-//! let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+//!     let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
 //!
 //!     // find number of atoms in the file
-//! let num_atoms = trj.get_num_atoms().unwrap();
+//!     let num_atoms = trj.get_num_atoms()?;
 //!
 //!     // a frame object is used to get to read or write from a trajectory
 //!     // without instantiating data arrays for every step
 //!     let mut frame = Frame::with_capacity(num_atoms);
 //!
 //!     // read the first frame of the trajectory
-//! let result = trj.read(&mut frame);
+//!     trj.read(&mut frame)?;
-//! match result {
+//!
-//!    Ok(_) => {
 //!     assert_eq!(frame.step, 1);
 //!     assert_eq!(frame.num_atoms, num_atoms);
 //!
 //!     let first_atom_coords = frame.coords[0];
 //!     assert_eq!(first_atom_coords, [-0.8901, 0.4127, -0.055499997]);
-//!    }
+//!
-//!    Err(msg) => {
+//!     Ok(())
-//!        panic!("Something went wrong: {}", msg);
-//!    }
 //! }
 //! ```
 //!
@@ -44,14 +42,18 @@
 //! ```rust
 //! use xdrfile::*;
 //!
+//! fn main() -> Result<()> {
 //!     // get a handle to the file
-//! let trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+//!     let trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
 //!
 //!     // iterate over all frames
-//! for (idx, frame) in trj.into_iter().filter_map(Result::ok).enumerate() {
+//!     for (idx, result) in trj.into_iter().enumerate() {
+//!         let frame = result?;
 //!         println!("{}", frame.time);
 //!         assert_eq!(idx+1, frame.step as usize);
 //!     }
+//!     Ok(())
+//! }
 //! ```
 
 #[cfg(test)]
@@ -74,15 +76,16 @@ use c_abi::xdrfile_xtc;
 use lazy_init::Lazy;
 use std::cell::Cell;
 use std::ffi::CString;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
-#[derive(Debug, Clone)]
+#[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 {
@@ -115,13 +118,34 @@ impl std::fmt::Display for Error {
                 "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 {}
 
-type Result<T> = std::result::Result<T, Error>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum FileMode {
@@ -131,17 +155,21 @@ pub enum FileMode {
 }
 
 impl FileMode {
-    pub fn value(&self) -> &str {
+    /// Get a CStr slice corresponding to the file mode
-        match *self {
+    fn to_cstr(&self) -> &'static std::ffi::CStr {
-            FileMode::Write => "w",
+        let bytes: &[u8; 2] = match *self {
-            FileMode::Append => "a",
+            FileMode::Write => b"w\0",
-            FileMode::Read => "r",
+            FileMode::Append => b"a\0",
-        }
+            FileMode::Read => b"r\0",
+        };
+
+        std::ffi::CStr::from_bytes_with_nul(bytes).expect("CStr::from_bytes_with_nul failed")
     }
 }
 
-fn path_to_cstring(path: &Path) -> CString {
+fn path_to_cstring(path: impl AsRef<Path>) -> Result<CString> {
-    CString::new(path.to_str().unwrap()).unwrap()
+    let s = path.as_ref().to_str().ok_or(Error::PathInvalidCstring)?;
+    CString::new(s).map_err(|_| Error::PathInvalidCstring)
 }
 
 /// A safe wrapper around the c implementation of an XDRFile
@@ -149,20 +177,24 @@ struct XDRFile {
     xdrfile: *mut XDRFILE,
     #[allow(dead_code)]
     filemode: FileMode,
-    path: String,
+    path: PathBuf,
 }
 
 impl XDRFile {
     pub fn open(path: impl AsRef<Path>, filemode: FileMode) -> Result<XDRFile> {
         let path = path.as_ref();
-        let path_p = path_to_cstring(path).into_raw();
-        let mode_p = CString::new(filemode.value()).unwrap().into_raw();
-
         unsafe {
+            let path_p = path_to_cstring(path)?.into_raw();
+            // SAFETY: mode_p must not be mutated by the C code
+            let mode_p = filemode.to_cstr().as_ptr();
+
             let xdrfile = xdrfile::xdrfile_open(path_p, mode_p);
 
+            // Reconstitute the CString so it is deallocated correctly
+            let _ = CString::from_raw(path_p);
+
             if !xdrfile.is_null() {
-                let path = String::from(path.to_str().unwrap());
+                let path = path.to_owned();
                 Ok(XDRFile {
                     xdrfile,
                     filemode,
@@ -290,10 +322,12 @@ impl Trajectory for XTCTrajectory {
             .get_or_create(|| {
                 let mut num_atoms: i32 = 0;
                 unsafe {
-                    let path = CString::new(self.handle.path.as_str()).unwrap();
+                    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;
+                    // 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)),
@@ -398,10 +432,12 @@ impl Trajectory for TRRTrajectory {
             .get_or_create(|| {
                 let mut num_atoms: i32 = 0;
                 unsafe {
-                    let path = CString::new(self.handle.path.as_str()).unwrap();
+                    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;
+                    // 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)),
@@ -419,8 +455,8 @@ mod tests {
     use tempfile::NamedTempFile;
 
     #[test]
-    fn test_read_write_xtc() {
+    fn test_read_write_xtc() -> Result<()> {
-        let tempfile = NamedTempFile::new().unwrap();
+        let tempfile = NamedTempFile::new().expect("Could not create temporary file");
         let tmp_path = tempfile.path();
 
         let natoms: u32 = 2;
@@ -431,17 +467,17 @@ mod tests {
             box_vector: [[1.0, 2.0, 3.0], [2.0, 1.0, 3.0], [3.0, 2.0, 1.0]],
             coords: vec![[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]],
         };
-        let mut f = XTCTrajectory::open_write(&tmp_path).unwrap();
+        let mut f = XTCTrajectory::open_write(&tmp_path)?;
         let write_status = f.write(&frame);
         match write_status {
             Err(_) => panic!("Failed"),
             Ok(()) => {}
         }
-        f.flush().unwrap();
+        f.flush()?;
 
         let mut new_frame = Frame::with_capacity(natoms);
-        let mut f = XTCTrajectory::open_read(tmp_path).unwrap();
+        let mut f = XTCTrajectory::open_read(tmp_path)?;
-        let num_atoms = f.get_num_atoms().unwrap();
+        let num_atoms = f.get_num_atoms()?;
         assert_eq!(num_atoms, natoms);
 
         let read_status = f.read(&mut new_frame);
@@ -455,11 +491,12 @@ mod tests {
         assert_approx_eq!(new_frame.time, frame.time);
         assert_eq!(new_frame.box_vector, frame.box_vector);
         assert_eq!(new_frame.coords, frame.coords);
+        Ok(())
     }
 
     #[test]
-    fn test_read_write_trr() {
+    fn test_read_write_trr() -> Result<()> {
-        let tempfile = NamedTempFile::new().unwrap();
+        let tempfile = NamedTempFile::new().expect("Could not create temporary file");
         let tmp_path = tempfile.path();
 
         let natoms: u32 = 2;
@@ -470,17 +507,17 @@ mod tests {
             box_vector: [[1.0, 2.0, 3.0], [2.0, 1.0, 3.0], [3.0, 2.0, 1.0]],
             coords: vec![[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]],
         };
-        let mut f = TRRTrajectory::open_write(tmp_path).unwrap();
+        let mut f = TRRTrajectory::open_write(tmp_path)?;
         let write_status = f.write(&frame);
         match write_status {
             Err(_) => panic!("Failed"),
             Ok(()) => {}
         }
-        f.flush().unwrap();
+        f.flush()?;
 
         let mut new_frame = Frame::with_capacity(natoms);
-        let mut f = TRRTrajectory::open_read(tmp_path).unwrap();
+        let mut f = TRRTrajectory::open_read(tmp_path)?;
-        // let num_atoms = f.get_num_atoms().unwrap();
+        // let num_atoms = f.get_num_atoms()?;
         // assert_eq!(num_atoms, natoms);
 
         let read_status = f.read(&mut new_frame);
@@ -494,6 +531,19 @@ mod tests {
         assert_eq!(new_frame.time, frame.time);
         assert_eq!(new_frame.box_vector, frame.box_vector);
         assert_eq!(new_frame.coords, frame.coords);
+        Ok(())
+    }
+
+    #[test]
+    fn test_path_to_cstring() -> Result<(), Box<dyn std::error::Error>> {
+        let result_invalid = path_to_cstring("invalid/\0path");
+
+        assert_eq!(result_invalid, Err(Error::PathInvalidCstring));
+
+        let result_valid = path_to_cstring("valid/path");
+
+        assert_eq!(result_valid, Ok(CString::new("valid/path")?));
+        Ok(())
     }
 
     #[test]
@@ -512,9 +562,9 @@ mod tests {
     }
 
     #[test]
-    fn test_err_could_not_read_atom_nr() {
+    fn test_err_could_not_read_atom_nr() -> Result<()> {
         let file_name = "README.md"; // not a trajectory
-        let mut trr = TRRTrajectory::open_read(file_name).unwrap();
+        let mut trr = TRRTrajectory::open_read(file_name)?;
         if let Err(e) = trr.get_num_atoms() {
             match e {
                 Error::CouldNotReadAtomNumber(code) => {
@@ -522,14 +572,15 @@ mod tests {
                 }
                 _ => panic!("Wrong Error type"),
             }
-        }
+        };
+        Ok(())
     }
 
     #[test]
-    fn test_err_could_not_read() {
+    fn test_err_could_not_read() -> Result<()> {
         let file_name = "README.md"; // not a trajectory
         let mut frame = Frame::with_capacity(1);
-        let mut trr = TRRTrajectory::open_read(file_name).unwrap();
+        let mut trr = TRRTrajectory::open_read(file_name)?;
         if let Err(e) = trr.read(&mut frame) {
             match e {
                 Error::CouldNotRead(code) => {
@@ -538,5 +589,69 @@ mod tests {
                 _ => panic!("Wrong Error type"),
             }
         }
+        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<dyn std::error::Error>> {
+        let tempfile = NamedTempFile::new()?;
+        let tmp_path = tempfile.path();
+
+        let natoms: u32 = 2;
+        let frame = Frame {
+            num_atoms: natoms,
+            step: 5,
+            time: 2.0,
+            box_vector: [[1.0, 2.0, 3.0], [2.0, 1.0, 3.0], [3.0, 2.0, 1.0]],
+            coords: vec![[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]],
+        };
+        let mut f = XTCTrajectory::open_write(&tmp_path)?;
+        f.write(&frame)?;
+        f.flush()?;
+
+        let mut new_frame = Frame::with_capacity(natoms);
+        let mut f = XTCTrajectory::open_read(tmp_path)?;
+
+        f.read(&mut new_frame)?;
+
+        let result = f.read(&mut new_frame); // Should be eof as we only wrote one frame
+        if let Err(e) = result {
+            assert!(e.is_eof());
+        } else {
+            panic!("read two frames after writing one");
+        }
+
+        let mut file = std::fs::File::create(tmp_path)?;
+        use std::io::Write as _;
+        file.write_all(&[0; 999])?;
+        file.flush()?;
+
+        let mut f = XTCTrajectory::open_read(tmp_path)?;
+        let result = f.read(&mut new_frame); // Should be an invalid XTC file
+        if let Err(e) = result {
+            assert!(!e.is_eof());
+        } else {
+            panic!("999 zero bytes was read as a valid XTC file");
+        }
+
+        Ok(())
     }
 }

tests/integration.rs

@@ -1,27 +1,28 @@
 #[cfg(test)]
 mod integration {
 
-    
     use std::rc::Rc;
     use xdrfile::*;
 
     #[test]
-    fn test_use_library() {
+    fn test_use_library() -> Result<()> {
-        let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+        let mut trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
-        let num_atoms = trj.get_num_atoms().unwrap();
+        let num_atoms = trj.get_num_atoms()?;
         let mut frame = Frame::with_capacity(num_atoms);
 
-        trj.read(&mut frame).unwrap();
+        trj.read(&mut frame)?;
-        trj.read(&mut frame).unwrap();
+        trj.read(&mut frame)?;
         assert_eq!(frame.step, 2);
+        Ok(())
     }
 
     #[test]
-    fn test_use_library_iterator() {
+    fn test_use_library_iterator() -> Result<()> {
-        let trj = XTCTrajectory::open_read("tests/1l2y.xtc").unwrap();
+        let trj = XTCTrajectory::open_read("tests/1l2y.xtc")?;
-        let frames: Vec<Rc<Frame>> = trj.into_iter().filter_map(Result::ok).collect();
+        let frames: Result<Vec<Rc<Frame>>> = trj.into_iter().collect();
-        for (idx, frame) in frames.iter().enumerate() {
+        for (idx, frame) in frames?.iter().enumerate() {
             assert_eq!(frame.step as usize, idx + 1);
         }
+        Ok(())
     }
 }