Mark volume dirty on first write and not-dirty on unmount

TODO.md

@@ -1,6 +1,5 @@
 TODO
 ====
-* marking volume dirty on first write and not-dirty on unmount
 * support for a volume label file in the root directory
 * format volume API
 * add method for getting `DirEntry` from a path (possible names: metadata, lookup)

src/file.rs

@@ -229,6 +229,8 @@ impl<'a, T: ReadWriteSeek> Write for File<'a, T> {
         if write_size == 0 {
             return Ok(0);
         }
+        // Mark the volume 'dirty'
+        self.fs.set_dirty_flag(true)?;
         // Get cluster for write possibly allocating new one
         let current_cluster = if self.offset % cluster_size == 0 {
             // next cluster

src/fs.rs

@@ -1,6 +1,6 @@
 #[cfg(all(not(feature = "std"), feature = "alloc"))]
 use alloc::String;
-use core::cell::RefCell;
+use core::cell::{Cell, RefCell};
 use core::char;
 use core::cmp;
 use core::fmt::Debug;
@@ -66,6 +66,24 @@ impl FsStatusFlags {
     pub fn io_error(&self) -> bool {
         self.io_error
     }
+
+    fn encode(&self) -> u8 {
+        let mut res = 0u8;
+        if self.dirty {
+            res |= 1;
+        }
+        if self.io_error {
+            res |= 2;
+        }
+        res
+    }
+
+    fn decode(flags: u8) -> Self {
+        FsStatusFlags {
+            dirty: flags & 1 != 0,
+            io_error: flags & 2 != 0,
+        }
+    }
 }
 
 /// A sum of `Read` and `Seek` traits.
@@ -178,10 +196,7 @@ impl BiosParameterBlock {
     }
 
     fn status_flags(&self) -> FsStatusFlags {
-        FsStatusFlags {
-            dirty: self.reserved_1 & 1 != 0,
-            io_error: self.reserved_1 & 2 != 0,
-        }
+        FsStatusFlags::decode(self.reserved_1)
     }
 }
 
@@ -374,6 +389,7 @@ pub struct FileSystem<T: ReadWriteSeek> {
     root_dir_sectors: u32,
     total_clusters: u32,
     fs_info: RefCell<FsInfoSector>,
+    current_status_flags: Cell<FsStatusFlags>,
 }
 
 impl<T: ReadWriteSeek> FileSystem<T> {
@@ -434,6 +450,7 @@ impl<T: ReadWriteSeek> FileSystem<T> {
         }
 
         // return FileSystem struct
+        let status_flags = bpb.status_flags();
         Ok(FileSystem {
             disk: RefCell::new(disk),
             options,
@@ -443,6 +460,7 @@ impl<T: ReadWriteSeek> FileSystem<T> {
             root_dir_sectors,
             total_clusters,
             fs_info: RefCell::new(fs_info),
+            current_status_flags: Cell::new(status_flags),
         })
     }
 
@@ -605,6 +623,7 @@ impl<T: ReadWriteSeek> FileSystem<T> {
 
     fn unmount_internal(&self) -> io::Result<()> {
         self.flush_fs_info()?;
+        self.set_dirty_flag(false)?;
         Ok(())
     }
 
@@ -618,6 +637,27 @@ impl<T: ReadWriteSeek> FileSystem<T> {
         }
         Ok(())
     }
+
+    pub(crate) fn set_dirty_flag(&self, dirty: bool) -> io::Result<()> {
+        // Do not overwrite flags read from BPB on mount
+        let mut flags = self.bpb.status_flags();
+        flags.dirty |= dirty;
+        // Check if flags has changed
+        let current_flags = self.current_status_flags.get();
+        if flags == current_flags {
+            // Nothing to do
+            return Ok(());
+        }
+        let encoded = flags.encode();
+        // Note: only one field is written to avoid rewriting entire boot-sector which could be dangerous
+        // Compute reserver_1 field offset and write new flags
+        let offset = if self.fat_type() == FatType::Fat32 { 0x041 } else { 0x025 };
+        let mut disk = self.disk.borrow_mut();
+        disk.seek(io::SeekFrom::Start(offset))?;
+        disk.write_u8(encoded)?;
+        self.current_status_flags.set(flags);
+        Ok(())
+    }
 }
 
 /// `Drop` implementation tries to unmount the filesystem when dropping.
@@ -692,8 +732,14 @@ impl<'a, T: ReadWriteSeek> Write for DiskSlice<'a, T> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         let offset = self.begin + self.offset;
         let write_size = cmp::min((self.size - self.offset) as usize, buf.len());
+        if write_size == 0 {
+            return Ok(0);
+        }
+        // Mark the volume 'dirty'
+        self.fs.set_dirty_flag(true)?;
+        // Write data
+        let mut disk = self.fs.disk.borrow_mut();
         for i in 0..self.mirrors {
-            let mut disk = self.fs.disk.borrow_mut();
             disk.seek(SeekFrom::Start(offset + i as u64 * self.size))?;
             disk.write_all(&buf[..write_size])?;
         }

tests/write.rs

@@ -5,6 +5,7 @@ extern crate fscommon;
 use std::fs;
 use std::io;
 use std::io::prelude::*;
+use std::mem;
 use std::str;
 
 use fatfs::FsOptions;
@@ -20,22 +21,30 @@ const TEST_STR2: &str = "Rust is cool!\n";
 
 type FileSystem = fatfs::FileSystem<BufStream<fs::File>>;
 
-fn call_with_fs(f: &Fn(FileSystem) -> (), filename: &str, test_seq: u32) {
+fn call_with_tmp_img(f: &Fn(&str) -> (), filename: &str, test_seq: u32) {
     let _ = env_logger::try_init();
     let img_path = format!("{}/{}", IMG_DIR, filename);
     let tmp_path = format!("{}/{}-{}", TMP_DIR, test_seq, filename);
     fs::create_dir(TMP_DIR).ok();
     fs::copy(&img_path, &tmp_path).unwrap();
-    {
-        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);
-        let fs = FileSystem::new(buf_file, options).unwrap();
-        f(fs);
-    }
+    f(tmp_path.as_str());
     fs::remove_file(tmp_path).unwrap();
 }
 
+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()
+}
+
+fn call_with_fs(f: &Fn(FileSystem) -> (), filename: &str, test_seq: u32) {
+    call_with_tmp_img(&|tmp_path| {
+        let fs = open_filesystem_rw(tmp_path);
+        f(fs);
+    }, filename, test_seq);
+}
+
 fn test_write_short_file(fs: FileSystem) {
     let root_dir = fs.root_dir();
     let mut file = root_dir.open_file("short.txt").expect("open file");
@@ -296,3 +305,27 @@ fn test_rename_file_fat16() {
 fn test_rename_file_fat32() {
     call_with_fs(&test_rename_file, FAT32_IMG, 6)
 }
+
+#[test]
+fn test_dirty_flag() {
+    call_with_tmp_img(&|tmp_path| {
+        // Open filesystem, make change, and forget it - should become dirty
+        let fs = open_filesystem_rw(tmp_path);
+        let status_flags = fs.read_status_flags().unwrap();
+        assert_eq!(status_flags.dirty(), false);
+        assert_eq!(status_flags.io_error(), false);
+        fs.root_dir().create_file("abc.txt").unwrap();
+        mem::forget(fs);
+        // Check if volume is dirty now
+        let fs = open_filesystem_rw(tmp_path);
+        let status_flags = fs.read_status_flags().unwrap();
+        assert_eq!(status_flags.dirty(), true);
+        assert_eq!(status_flags.io_error(), false);
+        fs.unmount().unwrap();
+        // Make sure remounting does not clear the dirty flag
+        let fs = open_filesystem_rw(tmp_path);
+        let status_flags = fs.read_status_flags().unwrap();
+        assert_eq!(status_flags.dirty(), true);
+        assert_eq!(status_flags.io_error(), false);
+    }, FAT32_IMG, 7);
+}