From b88027c29284ff2b72b1efc76a67aba833f1a2ae Mon Sep 17 00:00:00 2001 From: Alex Badics Date: Mon, 28 Oct 2024 10:25:08 +0100 Subject: [PATCH 1/3] tests: add fsck test --- tests/fsck.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/fsck.rs diff --git a/tests/fsck.rs b/tests/fsck.rs new file mode 100644 index 0000000..c4d966f --- /dev/null +++ b/tests/fsck.rs @@ -0,0 +1,48 @@ +#![cfg(target_os="linux")] +use fatfs::Write; + +const KB: u32 = 1024; +const MB: u32 = KB * 1024; + +#[test] +fn test_fsck_1mb() { + let _ = env_logger::builder().is_test(true).try_init(); + + let mut image = std::fs::OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("/tmp/test.img") + .expect("open temporary image file"); + image.set_len(MB as u64).expect("set_len on temp file"); + + fatfs::format_volume( + &mut fatfs::StdIoWrapper::from(image.try_clone().expect("clone tempfile")), + fatfs::FormatVolumeOptions::new().total_sectors(MB / 512), + ) + .expect("format volume"); + + let fs = fatfs::FileSystem::new(image, fatfs::FsOptions::new()).expect("open fs"); + fs.root_dir().create_dir("dir1").expect("create dir1"); + fs.root_dir() + .create_file("root file.bin") + .expect("create root file") + .write_all(&[0xab; (16 * KB) as usize]) + .expect("root file write"); + let dir2 = fs.root_dir().create_dir("dir2").expect("create dir2"); + dir2.create_dir("subdir").expect("subdir"); + dir2.create_file("file1") + .expect("file1") + .write_all(b"testing 1 2 1 2") + .expect("file 1 write"); + core::mem::drop(dir2); + core::mem::drop(fs); + + let fsck_status = std::process::Command::new("fsck.vfat") + .args(&["-n", "/tmp/test.img"]) + .spawn() + .expect("spawn fsck") + .wait() + .expect("wait on fsck"); + assert!(fsck_status.success(), "fsck was not successful ({fsck_status:?})"); +} From 34c83e30e3c34dcf7eb1e4c8c5cbd83638e9804e Mon Sep 17 00:00:00 2001 From: Alex Badics Date: Wed, 23 Oct 2024 21:00:18 +0200 Subject: [PATCH 2/3] Don't create LFNs for . and .. The current code makes invalid dir entries, because the first two slots should be . and .., but with LFNs, it's LFN for . . LFN for .. .. We should probably not create LFN's when not needed anyway, but that's not implemented in this patch. --- CHANGELOG.md | 1 + src/dir.rs | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8127e..0e302cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Bug fixes: * Fix formatting volumes with size in range 4096-4199 KB * Always respect `fat_type` from `FormatVolumeOptions` * Fill FAT32 root directory clusters with zeros after allocation to avoid interpreting old data as directory entries +* Put '.' and '..' in the first two directory entries. (fixes "Expected a valid '.' entry in this slot." fsck error) 0.3.4 (2020-07-20) ------------------ diff --git a/src/dir.rs b/src/dir.rs index 670225b..6bd6cd5 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -529,6 +529,12 @@ impl<'a, IO: ReadWriteSeek, TP: TimeProvider, OCC: OemCpConverter> Dir<'a, IO, T Ok((stream, start_pos)) } + fn alloc_sfn_entry(&self) -> Result<(DirRawStream<'a, IO, TP, OCC>, u64), Error> { + let mut stream = self.find_free_entries(1)?; + let start_pos = stream.seek(io::SeekFrom::Current(0))?; + Ok((stream, start_pos)) + } + fn write_entry( &self, name: &str, @@ -539,8 +545,13 @@ impl<'a, IO: ReadWriteSeek, TP: TimeProvider, OCC: OemCpConverter> Dir<'a, IO, T validate_long_name(name)?; // convert long name to UTF-16 let lfn_utf16 = Self::encode_lfn_utf16(name); - // write LFN entries - let (mut stream, start_pos) = self.alloc_and_write_lfn_entries(&lfn_utf16, raw_entry.name())?; + // write LFN entries, except for . and .., which need to be at + // the first two slots and don't need LFNs anyway + let (mut stream, start_pos) = if name == "." || name == ".." { + self.alloc_sfn_entry()? + } else { + self.alloc_and_write_lfn_entries(&lfn_utf16, raw_entry.name())? + }; // write short name entry raw_entry.serialize(&mut stream)?; // Get position directory stream after entries were written From 8becd5f9a9324cfd15522c0ec9720f04c8656053 Mon Sep 17 00:00:00 2001 From: Alex Badics Date: Wed, 23 Oct 2024 20:52:07 +0200 Subject: [PATCH 3/3] Fix .. cluster number for first-level dirs On FAT32, the cluster number of / is 0 in dir entries (and not the actually used cluster) --- CHANGELOG.md | 1 + src/dir.rs | 16 ++++++++++++++-- src/file.rs | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e302cb..9617bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Bug fixes: * Always respect `fat_type` from `FormatVolumeOptions` * Fill FAT32 root directory clusters with zeros after allocation to avoid interpreting old data as directory entries * Put '.' and '..' in the first two directory entries. (fixes "Expected a valid '.' entry in this slot." fsck error) +* Set the cluster number to 0 in the ".." directory entry if it points to the root dir 0.3.4 (2020-07-20) ------------------ diff --git a/src/dir.rs b/src/dir.rs index 6bd6cd5..e2038e8 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -38,6 +38,13 @@ impl DirRawStream<'_, IO, TP, OCC> { DirRawStream::Root(_) => None, } } + + pub(crate) fn is_root_dir(&self) -> bool { + match self { + DirRawStream::File(file) => file.is_root_dir(), + DirRawStream::Root(_) => true, + } + } } // Note: derive cannot be used because of invalid bounds. See: https://github.com/rust-lang/rust/issues/26925 @@ -304,8 +311,13 @@ impl<'a, IO: ReadWriteSeek, TP: TimeProvider, OCC: OemCpConverter> Dir<'a, IO, T let sfn_entry = self.create_sfn_entry(dot_sfn, FileAttributes::DIRECTORY, entry.first_cluster()); dir.write_entry(".", sfn_entry)?; let dotdot_sfn = ShortNameGenerator::generate_dotdot(); - let sfn_entry = - self.create_sfn_entry(dotdot_sfn, FileAttributes::DIRECTORY, self.stream.first_cluster()); + // cluster of the root dir shall be set to 0 in directory entries. + let dotdot_cluster = if self.stream.is_root_dir() { + None + } else { + self.stream.first_cluster() + }; + let sfn_entry = self.create_sfn_entry(dotdot_sfn, FileAttributes::DIRECTORY, dotdot_cluster); dir.write_entry("..", sfn_entry)?; Ok(dir) } diff --git a/src/file.rs b/src/file.rs index 1a411f4..533351d 100644 --- a/src/file.rs +++ b/src/file.rs @@ -214,6 +214,10 @@ impl<'a, IO: ReadWriteSeek, TP, OCC> File<'a, IO, TP, OCC> { disk.flush()?; Ok(()) } + + pub(crate) fn is_root_dir(&self) -> bool { + self.entry.is_none() + } } impl File<'_, IO, TP, OCC> {