Rid into storage trait#74
Conversation
|
ping @rafalh |
rafalh
left a comment
rafalh
left a comment
There was a problem hiding this comment.
I made some comments but I still want to wait until I experiment with other options that would not require users to use StdIoWrapper before merging it.
| } | ||
|
|
||
| impl<IO: Read + Write + Seek, TP, OCC> FileSystem<IO, TP, OCC> { | ||
| // #[cfg(feature = "std")] |
|
|
||
| pub(crate) fn as_ucs2_units(&self) -> &[u16] { | ||
| &self.ucs2_units | ||
| &self.ucs2_units[..self.len] |
There was a problem hiding this comment.
could you please rebase so those changes are not visible?
|
|
||
| fn main() -> io::Result<()> { | ||
| let img_file = match OpenOptions::new().read(true).write(true).open("fat.img") { | ||
| let img_file = match OpenOptions::new().create(true).read(true).write(true).open("fat.img") { |
There was a problem hiding this comment.
create(true) does not make sense for me here. You are not formatting the filesystem. If fat.img does not exist it will fail trying to read BPB.
| let file = File::open("resources/fat32.img")?; | ||
| let buf_rdr = BufStream::new(file); | ||
| let fs = FileSystem::new(buf_rdr, FsOptions::new())?; | ||
| let fs = FileSystem::new(StdIoWrapper::new(buf_rdr), FsOptions::new())?; |
There was a problem hiding this comment.
This is exactly why I made IntoStorage trait - to make it possible to pass std::fs::File directly. Anyway why in some places you used into() and in some StdIoWrapper::new directly? IMHO into way is better
There was a problem hiding this comment.
Could do something like pub fn new(disk: impl Into<IO>, ...) to keep making this work.
| let mut disk = storage.into_storage(); | ||
| trace!("FileSystem::new"); | ||
| debug_assert!(disk.seek(SeekFrom::Current(0))? == 0); | ||
| disk.seek(SeekFrom::Start(0))?; |
There was a problem hiding this comment.
This is a change of behavior that requires changing the function comment.
My assertion was a sanity check that let people know that you can't open a disk image, seek to a partition and hope it will open a filesystem from on that partition. On the other hand it should fail anyway because in the first sector there will be MBR or GPT, so new behavior should be okay. I can see that it is useful for situation like formatting after error.
4 participants
This PR seeks to solve #54.
This is done by implementing
Read, Write, Seek, IoBasefor mutable references and removing theIntoStoragetrait.This enables one to give a mutable reference to
Filesystem::new(..). Which can be freed up when an error occurs.An example of recovering a corrupted filesystem can be seen in /tests/format.rs#L78.