Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rid into storage trait #74

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions examples/cat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ use std::env;
use std::fs::File;
use std::io::{self, prelude::*};

use fatfs::{FileSystem, FsOptions};
use fatfs::{FsOptions, StdIoWrapper, DefaultTimeProvider, LossyOemCpConverter};
use fscommon::BufStream;

type FileSystem =
fatfs::FileSystem<StdIoWrapper<BufStream<File>>, DefaultTimeProvider, LossyOemCpConverter>;

fn main() -> io::Result<()> {
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(buf_rdr.into(), FsOptions::new())?;
let root_dir = fs.root_dir();
let mut file = root_dir.open_file(&env::args().nth(1).expect("filename expected"))?;
let mut buf = vec![];
Expand Down
4 changes: 2 additions & 2 deletions examples/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fs::File;
use std::io;

use chrono::{DateTime, Local};
use fatfs::{FileSystem, FsOptions};
use fatfs::{FileSystem, FsOptions, StdIoWrapper};
use fscommon::BufStream;

fn format_file_size(size: u64) -> String {
Expand All @@ -24,7 +24,7 @@ fn format_file_size(size: u64) -> String {
fn main() -> io::Result<()> {
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())?;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do something like pub fn new(disk: impl Into<IO>, ...) to keep making this work.

let root_dir = fs.root_dir();
let dir = match env::args().nth(1) {
None => root_dir,
Expand Down
4 changes: 2 additions & 2 deletions examples/partition.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{fs, io};

use fatfs::{FileSystem, FsOptions};
use fatfs::{FileSystem, FsOptions, StdIoWrapper};
use fscommon::{BufStream, StreamSlice};

fn main() -> io::Result<()> {
Expand All @@ -14,7 +14,7 @@ fn main() -> io::Result<()> {
// Create buffered stream to optimize file access
let buf_rdr = BufStream::new(partition);
// Finally initialize filesystem struct using provided partition
let fs = FileSystem::new(buf_rdr, FsOptions::new())?;
let fs = FileSystem::new(StdIoWrapper::new(buf_rdr), FsOptions::new())?;
// Read and display volume label
println!("Volume Label: {}", fs.volume_label());
// other operations...
Expand Down
7 changes: 4 additions & 3 deletions examples/write.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::fs::OpenOptions;
use std::io::{self, prelude::*};

use fatfs::{FileSystem, FsOptions};
use fatfs::{FileSystem, FsOptions, StdIoWrapper};
use fscommon::BufStream;

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") {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Ok(file) => file,
Err(err) => {
println!("Failed to open image!");
Expand All @@ -14,7 +14,8 @@ fn main() -> io::Result<()> {
};
let buf_stream = BufStream::new(img_file);
let options = FsOptions::new().update_accessed_date(true);
let fs = FileSystem::new(buf_stream, options)?;
let mut disk = StdIoWrapper::new(buf_stream);
let fs = FileSystem::new(&mut disk, options).unwrap();
let mut file = fs.root_dir().create_file("hello.txt")?;
file.write_all(b"Hello World!")?;
Ok(())
Expand Down
3 changes: 2 additions & 1 deletion src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ impl LfnBuffer {
};
for (i, usc2_unit) in usc2_units.enumerate() {
lfn.ucs2_units[i] = usc2_unit;
lfn.len += 1;
}
lfn
}
Expand All @@ -807,7 +808,7 @@ impl LfnBuffer {
}

pub(crate) fn as_ucs2_units(&self) -> &[u16] {
&self.ucs2_units
&self.ucs2_units[..self.len]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please rebase so those changes are not visible?

}
}

Expand Down
59 changes: 24 additions & 35 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,24 +323,14 @@ pub struct FileSystem<IO: ReadWriteSeek, TP, OCC> {
current_status_flags: Cell<FsStatusFlags>,
}

pub trait IntoStorage<T: Read + Write + Seek> {
fn into_storage(self) -> T;
}

impl<T: Read + Write + Seek> IntoStorage<T> for T {
fn into_storage(self) -> Self {
self
}
}

#[cfg(feature = "std")]
impl<T: std::io::Read + std::io::Write + std::io::Seek> IntoStorage<io::StdIoWrapper<T>> for T {
fn into_storage(self) -> io::StdIoWrapper<Self> {
io::StdIoWrapper::new(self)
}
}

impl<IO: Read + Write + Seek, TP, OCC> FileSystem<IO, TP, OCC> {
// #[cfg(feature = "std")]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be removed?

// impl<T: std::io::Read + std::io::Write + std::io::Seek> Into<io::StdIoWrapper<T>> for T {
// fn into(self) -> io::StdIoWrapper<Self> {
// io::StdIoWrapper::new(self)
// }
// }

impl<IO: ReadWriteSeek, TP, OCC> FileSystem<IO, TP, OCC> {
/// Creates a new filesystem object instance.
///
/// Supplied `storage` parameter cannot be seeked. If there is a need to read a fragment of disk
Expand All @@ -361,11 +351,10 @@ impl<IO: Read + Write + Seek, TP, OCC> FileSystem<IO, TP, OCC> {
/// # Panics
///
/// Panics in non-optimized build if `storage` position returned by `seek` is not zero.
pub fn new<T: IntoStorage<IO>>(storage: T, options: FsOptions<TP, OCC>) -> Result<Self, Error<IO::Error>> {
pub fn new(mut disk: IO, options: FsOptions<TP, OCC>) -> Result<Self, Error<IO::Error>> {
// Make sure given image is not seeked
let mut disk = storage.into_storage();
trace!("FileSystem::new");
debug_assert!(disk.seek(SeekFrom::Current(0))? == 0);
disk.seek(SeekFrom::Start(0))?;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// read boot sector
let bpb = {
Expand Down Expand Up @@ -1113,9 +1102,9 @@ impl FormatVolumeOptions {
///
/// Panics in non-optimized build if `storage` position returned by `seek` is not zero.
#[allow(clippy::needless_pass_by_value)]
pub fn format_volume<S: ReadWriteSeek>(storage: &mut S, options: FormatVolumeOptions) -> Result<(), Error<S::Error>> {
pub fn format_volume<S: ReadWriteSeek>(mut storage: S, options: FormatVolumeOptions) -> Result<(), Error<S::Error>> {
trace!("format_volume");
debug_assert!(storage.seek(SeekFrom::Current(0))? == 0);
storage.seek(SeekFrom::Start(0))?;

let bytes_per_sector = options.bytes_per_sector.unwrap_or(512);
let total_sectors = if let Some(total_sectors) = options.total_sectors {
Expand All @@ -1136,10 +1125,10 @@ pub fn format_volume<S: ReadWriteSeek>(storage: &mut S, options: FormatVolumeOpt
if boot.validate::<S::Error>().is_err() {
return Err(Error::InvalidInput);
}
boot.serialize(storage)?;
boot.serialize(&mut storage)?;
// Make sure entire logical sector is updated (serialize method always writes 512 bytes)
let bytes_per_sector = boot.bpb.bytes_per_sector;
write_zeros_until_end_of_sector(storage, bytes_per_sector)?;
write_zeros_until_end_of_sector(&mut storage, bytes_per_sector)?;

let bpb = &boot.bpb;
if bpb.is_fat32() {
Expand All @@ -1150,23 +1139,23 @@ pub fn format_volume<S: ReadWriteSeek>(storage: &mut S, options: FormatVolumeOpt
dirty: false,
};
storage.seek(SeekFrom::Start(bpb.bytes_from_sectors(bpb.fs_info_sector())))?;
fs_info_sector.serialize(storage)?;
write_zeros_until_end_of_sector(storage, bytes_per_sector)?;
fs_info_sector.serialize(&mut storage)?;
write_zeros_until_end_of_sector(&mut storage, bytes_per_sector)?;

// backup boot sector
storage.seek(SeekFrom::Start(bpb.bytes_from_sectors(bpb.backup_boot_sector())))?;
boot.serialize(storage)?;
write_zeros_until_end_of_sector(storage, bytes_per_sector)?;
boot.serialize(&mut storage)?;
write_zeros_until_end_of_sector(&mut storage, bytes_per_sector)?;
}

// format File Allocation Table
let reserved_sectors = bpb.reserved_sectors();
let fat_pos = bpb.bytes_from_sectors(reserved_sectors);
let sectors_per_all_fats = bpb.sectors_per_all_fats();
storage.seek(SeekFrom::Start(fat_pos))?;
write_zeros(storage, bpb.bytes_from_sectors(sectors_per_all_fats))?;
write_zeros(&mut storage, bpb.bytes_from_sectors(sectors_per_all_fats))?;
{
let mut fat_slice = fat_slice::<S, &mut S>(storage, bpb);
let mut fat_slice = fat_slice::<S, &mut S>(&mut storage, bpb);
let sectors_per_fat = bpb.sectors_per_fat();
let bytes_per_fat = bpb.bytes_from_sectors(sectors_per_fat);
format_fat(&mut fat_slice, fat_type, bpb.media, bytes_per_fat, bpb.total_clusters())?;
Expand All @@ -1177,10 +1166,10 @@ pub fn format_volume<S: ReadWriteSeek>(storage: &mut S, options: FormatVolumeOpt
let root_dir_sectors = bpb.root_dir_sectors();
let root_dir_pos = bpb.bytes_from_sectors(root_dir_first_sector);
storage.seek(SeekFrom::Start(root_dir_pos))?;
write_zeros(storage, bpb.bytes_from_sectors(root_dir_sectors))?;
write_zeros(&mut storage, bpb.bytes_from_sectors(root_dir_sectors))?;
if fat_type == FatType::Fat32 {
let root_dir_first_cluster = {
let mut fat_slice = fat_slice::<S, &mut S>(storage, bpb);
let mut fat_slice = fat_slice::<S, &mut S>(&mut storage, bpb);
alloc_cluster(&mut fat_slice, fat_type, None, None, 1)?
};
assert!(root_dir_first_cluster == bpb.root_dir_first_cluster);
Expand All @@ -1189,14 +1178,14 @@ pub fn format_volume<S: ReadWriteSeek>(storage: &mut S, options: FormatVolumeOpt
let fat32_root_dir_first_sector = first_data_sector + data_sectors_before_root_dir;
let fat32_root_dir_pos = bpb.bytes_from_sectors(fat32_root_dir_first_sector);
storage.seek(SeekFrom::Start(fat32_root_dir_pos))?;
write_zeros(storage, u64::from(bpb.cluster_size()))?;
write_zeros(&mut storage, u64::from(bpb.cluster_size()))?;
}

// Create volume label directory entry if volume label is specified in options
if let Some(volume_label) = options.volume_label {
storage.seek(SeekFrom::Start(root_dir_pos))?;
let volume_entry = DirFileEntryData::new(volume_label, FileAttributes::VOLUME_ID);
volume_entry.serialize(storage)?;
volume_entry.serialize(&mut storage)?;
}

storage.seek(SeekFrom::Start(0))?;
Expand Down
41 changes: 41 additions & 0 deletions src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,47 @@ impl<T> From<T> for StdIoWrapper<T> {
}
}

#[cfg(feature = "std")]
impl<T: std::fmt::Debug> std::fmt::Debug for StdIoWrapper<T> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("StdIoWrapper").field("inner", &self.inner).finish()
}
}


impl<T: IoBase> IoBase for &mut T {
type Error = T::Error;
}

impl<T: Read> Read for &mut T {
fn read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error> {
T::read(self, buf)
}
fn read_exact(&mut self, buf: &mut [u8]) -> Result<(), Self::Error> {
T::read_exact(self, buf)
}
}

impl<T: Write> Write for &mut T {
fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
T::write(self, buf)
}

fn write_all(&mut self, buf: &[u8]) -> Result<(), Self::Error> {
T::write_all(self, buf)
}

fn flush(&mut self) -> Result<(), Self::Error> {
T::flush(self, )
}
}

impl<T: Seek> Seek for &mut T {
fn seek(&mut self, pos: SeekFrom) -> Result<u64, Self::Error> {
T::seek(self, pos.into())
}
}

pub(crate) trait ReadLeExt {
type Error;
fn read_u8(&mut self) -> Result<u8, Self::Error>;
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
//! let img_file = std::fs::OpenOptions::new().read(true).write(true)
//! .open("tmp/fat.img")?;
//! let buf_stream = fscommon::BufStream::new(img_file);
//! let fs = fatfs::FileSystem::new(buf_stream, fatfs::FsOptions::new())?;
//! let fs = fatfs::FileSystem::new(fatfs::StdIoWrapper::new(buf_stream), fatfs::FsOptions::new())?;
//! let root_dir = fs.root_dir();
//!
//! // Write a file
Expand Down
16 changes: 15 additions & 1 deletion tests/format.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::io;
use std::io::prelude::*;

use fatfs::{DefaultTimeProvider, LossyOemCpConverter, StdIoWrapper};
use fatfs::{DefaultTimeProvider, LossyOemCpConverter, StdIoWrapper, FsOptions};
use fscommon::BufStream;

const KB: u64 = 1024;
Expand Down Expand Up @@ -73,6 +73,20 @@ fn test_format_fs(opts: fatfs::FormatVolumeOptions, total_bytes: u64) -> FileSys
fs
}


#[test]
fn test_recover_fs() {
let _ = env_logger::builder().is_test(true).try_init();
let storage_vec: Vec<u8> = vec![0xD1_u8; 1024 * 1024];
let storage_cur = std::io::Cursor::new(storage_vec);
let mut disk = fatfs::StdIoWrapper::from(BufStream::new(storage_cur));
assert!(fatfs::FileSystem::new(&mut disk, FsOptions::new()).is_err());

fatfs::format_volume(&mut disk, fatfs::FormatVolumeOptions::new()).expect("format volume");

fatfs::FileSystem::new(&mut disk, fatfs::FsOptions::new()).expect("open fs");
}

#[test]
fn test_format_1mb() {
let total_bytes = MB;
Expand Down
5 changes: 3 additions & 2 deletions tests/read.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::fs;
use std::io::prelude::*;
use std::io::SeekFrom;
use std::str;

use fatfs::Seek;
use fatfs::SeekFrom;
use fatfs::{DefaultTimeProvider, FatType, FsOptions, LossyOemCpConverter, StdIoWrapper};
use fscommon::BufStream;

Expand All @@ -17,7 +18,7 @@ fn call_with_fs<F: Fn(FileSystem) -> ()>(f: F, filename: &str) {
let _ = env_logger::builder().is_test(true).try_init();
let file = fs::File::open(filename).unwrap();
let buf_file = BufStream::new(file);
let fs = FileSystem::new(buf_file, FsOptions::new()).unwrap();
let fs = FileSystem::new(buf_file.into(), FsOptions::new()).unwrap();
f(fs);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn open_filesystem_rw(tmp_path: &str) -> FileSystem {
let file = fs::OpenOptions::new().read(true).write(true).open(&tmp_path).unwrap();
let buf_file = BufStream::new(file);
let options = FsOptions::new().update_accessed_date(true);
FileSystem::new(buf_file, options).unwrap()
FileSystem::new(buf_file.into(), options).unwrap()
}

fn call_with_fs<F: Fn(FileSystem) -> ()>(f: F, filename: &str, test_seq: u32) {
Expand Down