From 117b8d4e7592546e95f452a0a1bc5fb790decdd7 Mon Sep 17 00:00:00 2001 From: Szymon Walter Date: Wed, 21 Mar 2018 20:40:39 +0100 Subject: [PATCH] redo error handling --- src/error.rs | 85 ++++++++++++++++++++++++++---------------- src/fs.rs | 64 +++++++++++++++++-------------- src/lib.rs | 3 +- src/sys/block_group.rs | 30 ++++++--------- src/sys/inode.rs | 15 +++----- src/sys/superblock.rs | 19 +++++----- src/volume/mod.rs | 34 ++++++++++------- 7 files changed, 137 insertions(+), 113 deletions(-) diff --git a/src/error.rs b/src/error.rs index 0c5b8be..94e5bea 100644 --- a/src/error.rs +++ b/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), - OutOfBounds(usize), - AddressOutOfBounds(u32, u32, usize), - BadBlockGroupCount(u32, u32), + Other(String), + BadMagic { + magic: u16, + }, + 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 for Error { @@ -20,34 +65,8 @@ impl From for Error { #[cfg(any(test, not(feature = "no_std")))] impl From for Error { - fn from(err: io::Error) -> Error { - Error::Io(err) - } -} - -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, - } + fn from(inner: io::Error) -> Error { + Error::Io { inner } } } diff --git a/src/fs.rs b/src/fs.rs index be333db..9199467 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -30,10 +30,7 @@ pub struct Ext2>> { block_groups: Struct, S>, } -impl>> Ext2 -where - Error: From, -{ +impl>> Ext2 { pub fn new(volume: V) -> Result, 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::()); } @@ -96,13 +96,18 @@ where let block_size = self.block_size(); let offset = 0; - for (data, _) in InodeBlocks::new(&inode) { - let data_size = block_size - .min(total_size - read_size) - .min(buf.len() - offset); - let end = offset + data_size; - buf[offset..end].copy_from_slice(&data[..data_size]); - read_size += data_size; + for block in InodeBlocks::new(&inode) { + match block { + Ok((data, _)) => { + let data_size = block_size + .min(total_size - read_size) + .min(buf.len() - offset); + let end = offset + data_size; + 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>> Ext2 { 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>> { index: usize, } -impl<'a, S: Size, V: 'a + Volume>> Iterator for Inodes<'a, S, V> -where - Error: From, +impl<'a, S: Size, V: 'a + Volume>> Iterator + for Inodes<'a, S, V> { type Item = (Inode<'a, S, V>, Address); @@ -293,7 +300,7 @@ impl<'a, S: 'a + Size, V: 'a + Volume>> 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::().0)) }) } else if index < bs4 * bs4 + bs4 + 12 { @@ -344,9 +351,8 @@ pub struct InodeBlocks<'a: 'b, 'b, S: 'a + Size, V: 'a + Volume>> index: usize, } -impl<'a, 'b, S: Size, V: 'a + Volume>> InodeBlocks<'a, 'b, S, V> -where - Error: From, +impl<'a, 'b, S: Size, V: 'a + Volume>> + InodeBlocks<'a, 'b, S, V> { 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>> Iterator for InodeBlocks<'a, 'b, S, V> -where - Error: From, { - type Item = (VolumeSlice<'a, u8, Address>, Address); + type Item = Result<(VolumeSlice<'a, u8, Address>, Address), V::Error>; fn next(&mut self) -> Option { 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); } } } diff --git a/src/lib.rs b/src/lib.rs index 1e36af7..3b3be42 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/sys/block_group.rs b/src/sys/block_group.rs index b280aeb..fc9b00d 100644 --- a/src/sys/block_group.rs +++ b/src/sys/block_group.rs @@ -55,18 +55,15 @@ impl BlockGroupDescriptor { pub unsafe fn find_descriptor>>( haystack: &V, offset: Address, - ) -> Result<(BlockGroupDescriptor, Address), Error> - where - Error: From, - { + ) -> Result<(BlockGroupDescriptor, Address), Error> { let end = offset + Address::from(mem::size_of::()); if haystack.size() < end { - return Err(Error::AddressOutOfBounds( - end.sector(), - end.offset(), - end.sector_size(), - )); + return Err(Error::AddressOutOfBounds { + sector: end.sector(), + offset: end.offset(), + size: end.sector_size(), + }); } let descr = haystack @@ -80,18 +77,15 @@ impl BlockGroupDescriptor { haystack: &V, offset: Address, count: usize, - ) -> Result<(Vec, Address), Error> - where - Error: From, - { + ) -> Result<(Vec, Address), Error> { let end = offset + Address::from(count * mem::size_of::()); if haystack.size() < end { - return Err(Error::AddressOutOfBounds( - end.sector(), - end.offset(), - end.sector_size(), - )); + return Err(Error::AddressOutOfBounds { + sector: end.sector(), + offset: end.offset(), + size: end.sector_size(), + }); } let mut vec = Vec::with_capacity(count); diff --git a/src/sys/inode.rs b/src/sys/inode.rs index f391cbf..ed4c5ee 100644 --- a/src/sys/inode.rs +++ b/src/sys/inode.rs @@ -101,21 +101,18 @@ impl Inode { haystack: &V, offset: Address, size: usize, - ) -> Result<(Inode, Address), Error> - where - Error: From, - { + ) -> Result<(Inode, Address), Error> { if size != mem::size_of::() { unimplemented!("inodes with a size != 128"); } let end = offset + Address::from(size); if haystack.size() < end { - return Err(Error::AddressOutOfBounds( - end.sector(), - end.offset(), - end.sector_size(), - )); + return Err(Error::AddressOutOfBounds { + sector: end.sector(), + offset: end.offset(), + size: end.sector_size(), + }); } let inode = haystack diff --git a/src/sys/superblock.rs b/src/sys/superblock.rs index 917d705..276c97a 100644 --- a/src/sys/superblock.rs +++ b/src/sys/superblock.rs @@ -197,18 +197,15 @@ impl Debug for Superblock { impl Superblock { pub unsafe fn find>>( haystack: &V, - ) -> Result<(Superblock, Address), Error> - where - Error: From, - { + ) -> Result<(Superblock, Address), Error> { let offset = Address::from(1024_usize); let end = offset + Address::from(mem::size_of::()); if haystack.size() < end { - return Err(Error::AddressOutOfBounds( - end.sector(), - end.offset(), - end.sector_size(), - )); + return Err(Error::AddressOutOfBounds { + sector: end.sector(), + offset: end.offset(), + 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) } diff --git a/src/volume/mod.rs b/src/volume/mod.rs index fa427b0..197fba1 100644 --- a/src/volume/mod.rs +++ b/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; fn size(&self) -> Length; fn commit( @@ -32,13 +32,7 @@ where fn slice<'a>( &'a self, range: Range, - ) -> Option> { - if self.size() >= range.end && self.size() > range.start { - unsafe { Some(self.slice_unchecked(range)) } - } else { - None - } - } + ) -> Result, Self::Error>; } 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> { Length::Bounded( @@ -244,7 +238,7 @@ macro_rules! impl_slice { fn commit( &mut self, slice: Option>>, - ) -> 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>, + ) -> Result>, 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>, - ) -> Option>> { + ) -> Result>, 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() } } }