From f2fa73b28a80762c403ab39caf8c6404227a682c Mon Sep 17 00:00:00 2001 From: Ben Pig Chu Date: Fri, 16 Nov 2018 04:32:22 +0800 Subject: [PATCH] remove warning+add FsError, pt4 --- src/sfs.rs | 73 ++++++++++++++++++++++++++++++++++++++---------------- src/vfs.rs | 19 ++++++++------ 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/sfs.rs b/src/sfs.rs index dde25d7..2da44cc 100644 --- a/src/sfs.rs +++ b/src/sfs.rs @@ -26,10 +26,9 @@ impl Device { } } /// Load struct `T` from given block in device - fn load_struct(&mut self, id: BlockId) -> T { + fn load_struct(&mut self, id: BlockId) -> vfs::Result { let mut s: T = unsafe { uninitialized() }; - self.read_block(id, 0, s.as_buf_mut()).unwrap(); - s + self.read_block(id, 0, s.as_buf_mut()).map(|_|{s}) } } @@ -122,7 +121,7 @@ impl INodeImpl { /// remove a page in middle of file and insert the last page here, useful for dirent remove /// should be only used in unlink fn remove_dirent_page(&self, id: usize) -> vfs::Result<()> { - assert!(id < self.disk_inode.read().blocks as usize); + debug_assert!(id < self.disk_inode.read().blocks as usize); let to_remove = self.get_disk_block_id(id)?; let current_last = self.get_disk_block_id(self.disk_inode.read().blocks as usize - 1)?; self.set_disk_block_id(id, current_last)?; @@ -199,6 +198,7 @@ impl INodeImpl { _ => panic!("Unknown file type"), } as u32 } + // Note: the _\w*_at method always return begin>size?0:begin(&self, begin: usize, end: usize, mut f: F) -> vfs::Result where F: FnMut(&mut Box, &BlockRange, usize) -> vfs::Result<()> @@ -232,13 +232,11 @@ impl INodeImpl { }) } /// Clean content, no matter what type it is - fn _clean_at(&self, begin: usize, end: usize) -> vfs::Result<()> { + fn _clean_at(&self, begin: usize, end: usize) -> vfs::Result { static ZEROS: [u8; BLKSIZE] = [0; BLKSIZE]; - let size = self._io_at(begin, end, |device, range, _| { + self._io_at(begin, end, |device, range, _| { device.write_block(range.block, range.begin, &ZEROS[..range.len()]) - })?; - assert_eq!(size, end - begin); - Ok(()) + }) } fn nlinks_inc(&self) { self.disk_inode.write().nlinks += 1; @@ -297,7 +295,9 @@ impl vfs::INode for INodeImpl { if info.type_!=vfs::FileType::Dir { return Err(FsError::NotDir); } - assert!(info.nlinks > 0); + if info.nlinks <= 0 { + return Err(FsError::DirRemoved) + } // Ensure the name is not exist if !self.get_file_inode_id(name).is_none() { @@ -331,6 +331,9 @@ impl vfs::INode for INodeImpl { if info.type_!=vfs::FileType::Dir { return Err(FsError::NotDir) } + if info.nlinks <= 0 { + return Err(FsError::DirRemoved) + } if name == "." { return Err(FsError::IsDir) } @@ -344,7 +347,10 @@ impl vfs::INode for INodeImpl { let type_ = inode.disk_inode.read().type_; if type_ == FileType::Dir { // only . and .. - assert_eq!(inode.disk_inode.read().blocks, 2); + assert!(inode.disk_inode.read().blocks >= 2); + if inode.disk_inode.read().blocks > 2 { + return Err(FsError::DirNotEmpty) + } } inode.nlinks_dec(); if type_ == FileType::Dir { @@ -360,7 +366,9 @@ impl vfs::INode for INodeImpl { if info.type_!=vfs::FileType::Dir { return Err(FsError::NotDir) } - assert!(info.nlinks > 0); + if info.nlinks <= 0 { + return Err(FsError::DirRemoved) + } if !self.get_file_inode_id(name).is_none() { return Err(FsError::EntryExist); } @@ -386,7 +394,15 @@ impl vfs::INode for INodeImpl { if info.type_!=vfs::FileType::Dir { return Err(FsError::NotDir) } - assert!(info.nlinks > 0); + if info.nlinks <= 0 { + return Err(FsError::DirRemoved) + } + if old_name == "." { + return Err(FsError::IsDir) + } + if old_name == ".." { + return Err(FsError::IsDir) + } if !self.get_file_inode_id(new_name).is_none() { return Err(FsError::EntryExist); @@ -408,7 +424,16 @@ impl vfs::INode for INodeImpl { if info.type_!=vfs::FileType::Dir { return Err(FsError::NotDir) } - assert!(info.nlinks > 0); + if info.nlinks <= 0 { + return Err(FsError::DirRemoved) + } + if old_name == "." { + return Err(FsError::IsDir) + } + if old_name == ".." { + return Err(FsError::IsDir) + } + let dest = target.downcast_ref::().ok_or(FsError::NotSameFs)?; if !Arc::ptr_eq(&self.fs, &dest.fs) { return Err(FsError::NotSameFs); @@ -416,7 +441,9 @@ impl vfs::INode for INodeImpl { if dest.info()?.type_ != vfs::FileType::Dir { return Err(FsError::NotDir) } - assert!(dest.info()?.nlinks > 0); + if dest.info()?.nlinks <= 0 { + return Err(FsError::DirRemoved) + } if !self.get_file_inode_id(new_name).is_none() { return Err(FsError::EntryExist); @@ -472,7 +499,7 @@ impl vfs::INode for INodeImpl { impl Drop for INodeImpl { /// Auto sync when drop fn drop(&mut self) { - self.sync().expect("failed to sync"); + self.sync().expect("Failed to sync when dropping the SimpleFileSystem Inode"); if self.disk_inode.read().nlinks <= 0 { self._resize(0).unwrap(); self.disk_inode.write().sync(); @@ -504,9 +531,11 @@ pub struct SimpleFileSystem { impl SimpleFileSystem { /// Load SFS from device pub fn open(mut device: Box) -> vfs::Result> { - let super_block = device.load_struct::(BLKN_SUPER); - assert!(super_block.check(), "not a valid SFS"); - let free_map = device.load_struct::<[u8; BLKSIZE]>(BLKN_FREEMAP); + let super_block = device.load_struct::(BLKN_SUPER).unwrap(); + if !super_block.check() { + return Err(FsError::WrongFs); + } + let free_map = device.load_struct::<[u8; BLKSIZE]>(BLKN_FREEMAP).unwrap(); Ok(SimpleFileSystem { super_block: RwLock::new(Dirty::new(super_block)), @@ -574,7 +603,7 @@ impl SimpleFileSystem { free_map.set(block_id, true); return None } - super_block.unused_blocks -= 1; // will panic if underflow + super_block.unused_blocks -= 1; // will not underflow } id } @@ -609,7 +638,7 @@ impl SimpleFileSystem { } } // Load if not in set, or is weak ref. - let disk_inode = Dirty::new(self.device.lock().load_struct::(id)); + let disk_inode = Dirty::new(self.device.lock().load_struct::(id).unwrap()); self._new_inode(id, disk_inode) } /// Create a new INode file @@ -675,7 +704,7 @@ impl Drop for SimpleFileSystem { /// Auto sync when drop fn drop(&mut self) { use vfs::FileSystem; - self.sync().expect("failed to sync"); + self.sync().expect("Failed to sync when dropping the SimpleFileSystem"); } } diff --git a/src/vfs.rs b/src/vfs.rs index b73a978..ce27f2d 100644 --- a/src/vfs.rs +++ b/src/vfs.rs @@ -10,7 +10,7 @@ pub trait Device: Send { fn write_at(&mut self, offset: usize, buf: &[u8]) -> Option; } -/// Abstract operations on a inode. +/// Abstract operations on a inode. pub trait INode: Debug + Any + Sync + Send { fn read_at(&self, offset: usize, buf: &mut [u8]) -> Result; fn write_at(&self, offset: usize, buf: &[u8]) -> Result; @@ -45,9 +45,9 @@ impl INode { if info.type_ != FileType::Dir { return Err(FsError::NotDir); } - Ok((0..info.size).map(|i| { - self.get_entry(i).unwrap() - }).collect()) + (0..info.size).map(|i| { + self.get_entry(i) + }).collect() } pub fn lookup(&self, path: &str) -> Result> { if self.info()?.type_ != FileType::Dir { @@ -103,10 +103,11 @@ pub struct FsInfo { pub max_file_size: usize, } - +// Note: IOError/NoMemory always lead to a panic since it's hard to recover from it. +// We also panic when we can not parse the fs on disk normally #[derive(Debug)] pub enum FsError { - NotSupported,//E_UNIMP + NotSupported,//E_UNIMP, or E_INVAL NotFile,//E_ISDIR IsDir,//E_ISDIR, used only in link NotDir,//E_NOTDIR @@ -115,12 +116,14 @@ pub enum FsError { NotSameFs,//E_XDEV InvalidParam,//E_INVAL NoDeviceSpace,//E_NOSPC, but is defined and not used in the original ucore, which uses E_NO_MEM - //and something else + DirRemoved,//E_NOENT, when the current dir was remove by a previous unlink + DirNotEmpty,//E_NOTEMPTY + WrongFs,//E_INVAL, when we find the content on disk is wrong when opening the device } pub type Result = result::Result; -/// Abstract filesystem +/// Abstract filesystem pub trait FileSystem: Sync { fn sync(&self) -> Result<()>; fn root_inode(&self) -> Arc;