redo error handling

src/error.rs

@@ -1,15 +1,60 @@
+use core::fmt::{self, Display};
+use alloc::String;
+
 #[cfg(any(test, not(feature = "no_std")))]
 use std::io;
 
 /// The set of all possible errors
 #[derive(Debug)]
 pub enum Error {
-    BadMagic(u16),
+    Other(String),
-    OutOfBounds(usize),
+    BadMagic {
-    AddressOutOfBounds(u32, u32, usize),
+        magic: u16,
-    BadBlockGroupCount(u32, u32),
+    },
+    OutOfBounds {
+        index: usize,
+    },
+    AddressOutOfBounds {
+        sector: u32,
+        offset: u32,
+        size: usize,
+    },
+    BadBlockGroupCount {
+        by_blocks: u32,
+        by_inodes: u32,
+    },
     #[cfg(any(test, not(feature = "no_std")))]
-    Io(io::Error),
+    Io {
+        inner: io::Error,
+    },
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            Error::Other(ref msg) => write!(f, "{}", msg),
+            Error::BadMagic {
+                magic,
+            } => write!(f, "invalid magic value: {}", magic),
+            Error::OutOfBounds {
+                index,
+            } => write!(f, "index ouf of bounds: {}", index),
+            Error::AddressOutOfBounds {
+                sector,
+                offset,
+                size,
+            } => write!(f, "address ouf of bounds: {}:{} with a block size of: {}",
+                   sector, offset, size),
+            Error::BadBlockGroupCount {
+                by_blocks,
+                by_inodes,
+            } => write!(f, "conflicting block group count data; by blocks: {}, by inodes: {}", by_blocks, by_inodes),
+            #[cfg(any(test, not(feature = "no_std")))]
+            Error::Io {
+                ref inner,
+            } => write!(f, "io error: {}", inner),
+        }
+    }
 }
 
 impl From<Infallible> for Error {
@@ -20,34 +65,8 @@ impl From<Infallible> for Error {
 
 #[cfg(any(test, not(feature = "no_std")))]
 impl From<io::Error> for Error {
-    fn from(err: io::Error) -> Error {
+    fn from(inner: io::Error) -> Error {
-        Error::Io(err)
+        Error::Io { inner }
-    }
-}
-
-impl PartialEq for Error {
-    fn eq(&self, rhs: &Error) -> bool {
-        match (self, rhs) {
-            (&Error::BadMagic(a), &Error::BadMagic(b)) => a == b,
-            (&Error::OutOfBounds(a), &Error::OutOfBounds(b)) => a == b,
-            (
-                &Error::BadBlockGroupCount(a1, a2),
-                &Error::BadBlockGroupCount(b1, b2),
-            ) => a1 == b1 && a2 == b2,
-            _ => false,
-        }
-    }
-
-    fn ne(&self, rhs: &Error) -> bool {
-        match (self, rhs) {
-            (&Error::BadMagic(a), &Error::BadMagic(b)) => a != b,
-            (&Error::OutOfBounds(a), &Error::OutOfBounds(b)) => a != b,
-            (
-                &Error::BadBlockGroupCount(a1, a2),
-                &Error::BadBlockGroupCount(b1, b2),
-            ) => a1 != b1 || a2 != b2,
-            _ => false,
-        }
     }
 }
 

src/fs.rs

@@ -30,10 +30,7 @@ pub struct Ext2<S: Size, V: Volume<u8, Address<S>>> {
     block_groups: Struct<Vec<BlockGroupDescriptor>, S>,
 }
 
-impl<S: Size, V: Volume<u8, Address<S>>> Ext2<S, V>
+impl<S: Size, V: Volume<u8, Address<S>>> Ext2<S, V> {
-where
-    Error: From<V::Error>,
-{
     pub fn new(volume: V) -> Result<Ext2<S, V>, Error> {
         let superblock = unsafe { Struct::from(Superblock::find(&volume)?) };
         let block_groups_offset = Address::with_block_size(
@@ -45,7 +42,10 @@ where
             .inner
             .block_group_count()
             .map(|count| count as usize)
-            .map_err(|(a, b)| Error::BadBlockGroupCount(a, b))?;
+            .map_err(|(a, b)| Error::BadBlockGroupCount {
+                by_blocks: a,
+                by_inodes: b,
+            })?;
         let block_groups = unsafe {
             BlockGroupDescriptor::find_descriptor_table(
                 &volume,
@@ -70,7 +70,7 @@ where
                 self.superblock.offset,
             );
             let commit = slice.commit();
-            self.volume.commit(commit).map_err(|err| Error::from(err))?;
+            self.volume.commit(commit).map_err(|err| err.into())?;
         }
 
         // block group descriptors
@@ -78,7 +78,7 @@ where
         for descr in &self.block_groups.inner {
             let slice = VolumeSlice::from_cast(descr, offset);
             let commit = slice.commit();
-            self.volume.commit(commit).map_err(|err| Error::from(err))?;
+            self.volume.commit(commit).map_err(|err| err.into())?;
             offset =
                 offset + Address::from(mem::size_of::<BlockGroupDescriptor>());
         }
@@ -96,7 +96,9 @@ where
         let block_size = self.block_size();
         let offset = 0;
 
-        for (data, _) in InodeBlocks::new(&inode) {
+        for block in InodeBlocks::new(&inode) {
+            match block {
+                Ok((data, _)) => {
                     let data_size = block_size
                         .min(total_size - read_size)
                         .min(buf.len() - offset);
@@ -104,6 +106,9 @@ where
                     buf[offset..end].copy_from_slice(&data[..data_size]);
                     read_size += data_size;
                 }
+                Err(err) => return Err(err.into()),
+            }
+        }
 
         Ok(read_size)
     }
@@ -180,7 +185,10 @@ impl<S: Size, V: Volume<u8, Address<S>>> Ext2<S, V> {
         self.superblock()
             .block_group_count()
             .map(|count| count as usize)
-            .map_err(|(a, b)| Error::BadBlockGroupCount(a, b))
+            .map_err(|(a, b)| Error::BadBlockGroupCount {
+                by_blocks: a,
+                by_inodes: b,
+            })
     }
 
     pub fn total_block_count(&self) -> usize {
@@ -224,9 +232,8 @@ pub struct Inodes<'a, S: 'a + Size, V: 'a + Volume<u8, Address<S>>> {
     index: usize,
 }
 
-impl<'a, S: Size, V: 'a + Volume<u8, Address<S>>> Iterator for Inodes<'a, S, V>
+impl<'a, S: Size, V: 'a + Volume<u8, Address<S>>> Iterator
-where
+    for Inodes<'a, S, V>
-    Error: From<V::Error>,
 {
     type Item = (Inode<'a, S, V>, Address<S>);
 
@@ -293,7 +300,7 @@ impl<'a, S: 'a + Size, V: 'a + Volume<u8, Address<S>>> Inode<'a, S, V> {
             );
             let size = Address::from(4_u64);
             let slice = self.fs.volume.slice(addr..addr + size);
-            slice.and_then(|slice| unsafe {
+            slice.ok().and_then(|slice| unsafe {
                 NonZero::new(u32::from_le(slice.dynamic_cast::<u32>().0))
             })
         } else if index < bs4 * bs4 + bs4 + 12 {
@@ -344,9 +351,8 @@ pub struct InodeBlocks<'a: 'b, 'b, S: 'a + Size, V: 'a + Volume<u8, Address<S>>>
     index: usize,
 }
 
-impl<'a, 'b, S: Size, V: 'a + Volume<u8, Address<S>>> InodeBlocks<'a, 'b, S, V>
+impl<'a, 'b, S: Size, V: 'a + Volume<u8, Address<S>>>
-where
+    InodeBlocks<'a, 'b, S, V>
-    Error: From<V::Error>,
 {
     pub fn new(inode: &'b Inode<'a, S, V>) -> InodeBlocks<'a, 'b, S, V> {
         InodeBlocks { inode, index: 0 }
@@ -355,10 +361,8 @@ where
 
 impl<'a, 'b, S: Size, V: 'a + Volume<u8, Address<S>>> Iterator
     for InodeBlocks<'a, 'b, S, V>
-where
-    Error: From<V::Error>,
 {
-    type Item = (VolumeSlice<'a, u8, Address<S>>, Address<S>);
+    type Item = Result<(VolumeSlice<'a, u8, Address<S>>, Address<S>), V::Error>;
 
     fn next(&mut self) -> Option<Self::Item> {
         let block = self.inode.block(self.index);
@@ -377,7 +381,7 @@ where
                         self.inode.fs.log_block_size(),
                     )
             })
-            .and_then(|block| {
+            .map(|block| {
                 let offset = block.start;
                 self.inode
                     .fs
@@ -467,7 +471,7 @@ mod tests {
             println!("{:?}", inode.0);
             let size = inode.0.size();
             for block in InodeBlocks::new(&inode.0) {
-                let (data, _) = block;
+                let (data, _) = block.unwrap();
                 assert_eq!(data.len(), fs.block_size());
                 println!("{:?}", &data[..size]);
                 let _ = str::from_utf8(&data[..size])
@@ -490,9 +494,11 @@ mod tests {
                 buf.set_len(inode.size());
             }
             let size = fs.read_inode(&mut buf[..], &inode);
-            assert_eq!(size, Ok(inode.size()));
+            assert!(size.is_ok());
+            let size = size.unwrap();
+            assert_eq!(size, inode.size());
             unsafe {
-                buf.set_len(size.unwrap());
+                buf.set_len(size);
             }
         }
     }

src/lib.rs

@@ -5,15 +5,16 @@
 #![feature(const_fn)]
 #![feature(step_trait)]
 #![feature(nonzero)]
+#![feature(associated_type_defaults)]
 #![cfg_attr(all(not(test), feature = "no_std"), no_std)]
 
 #[macro_use]
 extern crate alloc;
 #[macro_use]
 extern crate bitflags;
+
 #[cfg(any(test, not(feature = "no_std")))]
 extern crate core;
-extern crate spin;
 
 pub mod error;
 pub mod sys;

src/sys/block_group.rs

@@ -55,18 +55,15 @@ impl BlockGroupDescriptor {
     pub unsafe fn find_descriptor<S: Size, V: Volume<u8, Address<S>>>(
         haystack: &V,
         offset: Address<S>,
-    ) -> Result<(BlockGroupDescriptor, Address<S>), Error>
+    ) -> Result<(BlockGroupDescriptor, Address<S>), Error> {
-    where
-        Error: From<V::Error>,
-    {
         let end =
             offset + Address::from(mem::size_of::<BlockGroupDescriptor>());
         if haystack.size() < end {
-            return Err(Error::AddressOutOfBounds(
+            return Err(Error::AddressOutOfBounds {
-                end.sector(),
+                sector: end.sector(),
-                end.offset(),
+                offset: end.offset(),
-                end.sector_size(),
+                size: end.sector_size(),
-            ));
+            });
         }
 
         let descr = haystack
@@ -80,18 +77,15 @@ impl BlockGroupDescriptor {
         haystack: &V,
         offset: Address<S>,
         count: usize,
-    ) -> Result<(Vec<BlockGroupDescriptor>, Address<S>), Error>
+    ) -> Result<(Vec<BlockGroupDescriptor>, Address<S>), Error> {
-    where
-        Error: From<V::Error>,
-    {
         let end = offset
             + Address::from(count * mem::size_of::<BlockGroupDescriptor>());
         if haystack.size() < end {
-            return Err(Error::AddressOutOfBounds(
+            return Err(Error::AddressOutOfBounds {
-                end.sector(),
+                sector: end.sector(),
-                end.offset(),
+                offset: end.offset(),
-                end.sector_size(),
+                size: end.sector_size(),
-            ));
+            });
         }
 
         let mut vec = Vec::with_capacity(count);

src/sys/inode.rs

@@ -101,21 +101,18 @@ impl Inode {
         haystack: &V,
         offset: Address<S>,
         size: usize,
-    ) -> Result<(Inode, Address<S>), Error>
+    ) -> Result<(Inode, Address<S>), Error> {
-    where
-        Error: From<V::Error>,
-    {
         if size != mem::size_of::<Inode>() {
             unimplemented!("inodes with a size != 128");
         }
 
         let end = offset + Address::from(size);
         if haystack.size() < end {
-            return Err(Error::AddressOutOfBounds(
+            return Err(Error::AddressOutOfBounds {
-                end.sector(),
+                sector: end.sector(),
-                end.offset(),
+                offset: end.offset(),
-                end.sector_size(),
+                size: end.sector_size(),
-            ));
+            });
         }
 
         let inode = haystack

src/sys/superblock.rs

@@ -197,18 +197,15 @@ impl Debug for Superblock {
 impl Superblock {
     pub unsafe fn find<S: Size, V: Volume<u8, Address<S>>>(
         haystack: &V,
-    ) -> Result<(Superblock, Address<S>), Error>
+    ) -> Result<(Superblock, Address<S>), Error> {
-    where
-        Error: From<V::Error>,
-    {
         let offset = Address::from(1024_usize);
         let end = offset + Address::from(mem::size_of::<Superblock>());
         if haystack.size() < end {
-            return Err(Error::AddressOutOfBounds(
+            return Err(Error::AddressOutOfBounds {
-                end.sector(),
+                sector: end.sector(),
-                end.offset(),
+                offset: end.offset(),
-                end.sector_size(),
+                size: end.sector_size(),
-            ));
+            });
         }
 
         let superblock = {
@@ -218,7 +215,9 @@ impl Superblock {
         };
 
         if superblock.0.magic != EXT2_MAGIC {
-            Err(Error::BadMagic(superblock.0.magic))
+            Err(Error::BadMagic {
+                magic: superblock.0.magic,
+            })
         } else {
             Ok(superblock)
         }

src/volume/mod.rs

@@ -6,7 +6,7 @@ use alloc::Vec;
 use alloc::boxed::Box;
 use alloc::borrow::{Cow, ToOwned};
 
-use error::Infallible;
+use error::Error;
 use sector::{Address, Size};
 
 pub mod length;
@@ -17,7 +17,7 @@ where
     [T]: ToOwned,
     Idx: PartialEq + PartialOrd,
 {
-    type Error;
+    type Error: Into<Error>;
 
     fn size(&self) -> Length<Idx>;
     fn commit(
@@ -32,13 +32,7 @@ where
     fn slice<'a>(
         &'a self,
         range: Range<Idx>,
-    ) -> Option<VolumeSlice<'a, T, Idx>> {
+    ) -> Result<VolumeSlice<'a, T, Idx>, Self::Error>;
-        if self.size() >= range.end && self.size() > range.start {
-            unsafe { Some(self.slice_unchecked(range)) }
-        } else {
-            None
-        }
-    }
 }
 
 pub struct VolumeSlice<'a, T: 'a, Idx>
@@ -233,7 +227,7 @@ macro_rules! impl_slice {
             T: Clone,
             [T]: ToOwned,
         {
-            type Error = Infallible;
+            type Error = Error;
 
             fn size(&self) -> Length<Address<S>> {
                 Length::Bounded(
@@ -244,7 +238,7 @@ macro_rules! impl_slice {
             fn commit(
                 &mut self,
                 slice: Option<VolumeCommit<T, Address<S>>>,
-            ) -> Result<(), Infallible> {
+            ) -> Result<(), Self::Error> {
                 slice.map(|slice| {
                     let index = slice.at_index().into_index() as usize;
                     let end = index + slice.as_ref().len();
@@ -269,6 +263,21 @@ macro_rules! impl_slice {
                     index,
                 )
             }
+
+            fn slice<'a>(
+                &'a self,
+                range: Range<Address<S>>,
+            ) -> Result<VolumeSlice<'a, T, Address<S>>, Self::Error> {
+                if self.size() >= range.end {
+                    unsafe { Ok(self.slice_unchecked(range)) }
+                } else {
+                    Err(Error::AddressOutOfBounds {
+                        sector: range.end.sector(),
+                        offset: range.end.offset(),
+                        size: range.end.sector_size()
+                    })
+                }
+            }
         }
     };
     ($volume:ty) => {
@@ -344,7 +353,7 @@ mod file {
         fn slice<'a>(
             &'a self,
             range: Range<Address<S>>,
-        ) -> Option<VolumeSlice<'a, u8, Address<S>>> {
+        ) -> Result<VolumeSlice<'a, u8, Address<S>>, Self::Error> {
             let index = range.start;
             let mut vec = Vec::with_capacity((range.end - range.start)
                 .into_index()
@@ -357,7 +366,6 @@ mod file {
                 .seek(SeekFrom::Start(index.into_index()))
                 .and_then(|_| refmut.read_exact(&mut vec[..]))
                 .map(move |_| VolumeSlice::new_owned(vec, index))
-                .ok()
         }
     }
 }