Merge pull request #123 from peterkrull/always-closing

Always closing (and more)

Author: thejpster

src/filesystem/directory.rs

@@ -72,9 +72,12 @@ impl RawDirectory {
 
 /// Represents an open directory on disk.
 ///
-/// If you drop a value of this type, it closes the directory automatically. However,
-/// it holds a mutable reference to its parent `VolumeManager`, which restricts
-/// which operations you can perform.
+/// In contrast to a `RawDirectory`, a `Directory` holds a mutable reference to
+/// its parent `VolumeManager`, which restricts which operations you can perform.
+///
+/// If you drop a value of this type, it closes the directory automatically, but
+/// any error that may occur will be ignored. To handle potential errors, use
+/// the [`Directory::close`] method.
 pub struct Directory<
     'a,
     D,
@@ -188,6 +191,16 @@ where
         core::mem::forget(self);
         d
     }
+
+    /// Consume the `Directory` handle and close it. The behavior of this is similar
+    /// to using [`core::mem::drop`] or letting the `Directory` go out of scope,
+    /// except this lets the user handle any errors that may occur in the process,
+    /// whereas when using drop, any errors will be discarded silently.
+    pub fn close(self) -> Result<(), Error<D::Error>> {
+        let result = self.volume_mgr.close_dir(self.raw_directory);
+        core::mem::forget(self);
+        result
+    }
 }
 
 impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop
@@ -197,9 +210,7 @@ where
     T: crate::TimeSource,
 {
     fn drop(&mut self) {
-        self.volume_mgr
-            .close_dir(self.raw_directory)
-            .expect("Failed to close directory");
+        _ = self.volume_mgr.close_dir(self.raw_directory)
     }
 }
 

src/filesystem/files.rs

@@ -39,9 +39,12 @@ impl RawFile {
 
 /// Represents an open file on disk.
 ///
-/// If you drop a value of this type, it closes the file automatically. However,
-/// it holds a mutable reference to its parent `VolumeManager`, which restricts
-/// which operations you can perform.
+/// In contrast to a `RawFile`, a `File`  holds a mutable reference to its
+/// parent `VolumeManager`, which restricts which operations you can perform.
+///
+/// If you drop a value of this type, it closes the file automatically, and but
+/// error that may occur will be ignored. To handle potential errors, use
+/// the [`File::close`] method.
 pub struct File<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize>
 where
     D: crate::BlockDevice,
@@ -128,6 +131,16 @@ where
     pub fn flush(&mut self) -> Result<(), Error<D::Error>> {
         self.volume_mgr.flush_file(self.raw_file)
     }
+
+    /// Consume the `File` handle and close it. The behavior of this is similar
+    /// to using [`core::mem::drop`] or letting the `File` go out of scope,
+    /// except this lets the user handle any errors that may occur in the process,
+    /// whereas when using drop, any errors will be discarded silently.
+    pub fn close(self) -> Result<(), Error<D::Error>> {
+        let result = self.volume_mgr.close_file(self.raw_file);
+        core::mem::forget(self);
+        result
+    }
 }
 
 impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop
@@ -137,9 +150,7 @@ where
     T: crate::TimeSource,
 {
     fn drop(&mut self) {
-        self.volume_mgr
-            .close_file(self.raw_file)
-            .expect("Failed to close file");
+        _ = self.volume_mgr.close_file(self.raw_file);
     }
 }
 

src/lib.rs

@@ -239,9 +239,12 @@ impl RawVolume {
 
 /// Represents an open volume on disk.
 ///
-/// If you drop a value of this type, it closes the volume automatically. However,
-/// it holds a mutable reference to its parent `VolumeManager`, which restricts
-/// which operations you can perform.
+/// In contrast to a `RawVolume`, a `Volume` holds a mutable reference to its
+/// parent `VolumeManager`, which restricts which operations you can perform.
+///
+/// If you drop a value of this type, it closes the volume automatically, but
+/// any error that may occur will be ignored. To handle potential errors, use
+/// the [`Volume::close`] method.
 pub struct Volume<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize>
 where
     D: crate::BlockDevice,
@@ -285,6 +288,16 @@ where
         core::mem::forget(self);
         v
     }
+
+    /// Consume the `Volume` handle and close it. The behavior of this is similar
+    /// to using [`core::mem::drop`] or letting the `Volume` go out of scope,
+    /// except this lets the user handle any errors that may occur in the process,
+    /// whereas when using drop, any errors will be discarded silently.
+    pub fn close(self) -> Result<(), Error<D::Error>> {
+        let result = self.volume_mgr.close_volume(self.raw_volume);
+        core::mem::forget(self);
+        result
+    }
 }
 
 impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop
@@ -294,9 +307,7 @@ where
     T: crate::TimeSource,
 {
     fn drop(&mut self) {
-        self.volume_mgr
-            .close_volume(self.raw_volume)
-            .expect("Failed to close volume");
+        _ = self.volume_mgr.close_volume(self.raw_volume)
     }
 }
 

src/volume_mgr.rs

@@ -775,14 +775,36 @@ where
 
     /// Close a file with the given raw file handle.
     pub fn close_file(&mut self, file: RawFile) -> Result<(), Error<D::Error>> {
-        let file_idx = self.flush_file_get_index(file)?;
+        let flush_result = self.flush_file(file);
+        let file_idx = self.get_file_by_id(file)?;
         self.open_files.swap_remove(file_idx);
-        Ok(())
+        flush_result
     }
 
     /// Flush (update the entry) for a file with the given raw file handle.
     pub fn flush_file(&mut self, file: RawFile) -> Result<(), Error<D::Error>> {
-        self.flush_file_get_index(file).map(|_| ())
+        let file_info = self
+            .open_files
+            .iter()
+            .find(|info| info.file_id == file)
+            .ok_or(Error::BadHandle)?;
+
+        if file_info.dirty {
+            let volume_idx = self.get_volume_by_id(file_info.volume_id)?;
+            match self.open_volumes[volume_idx].volume_type {
+                VolumeType::Fat(ref mut fat) => {
+                    debug!("Updating FAT info sector");
+                    fat.update_info_sector(&self.block_device)?;
+                    debug!("Updating dir entry {:?}", file_info.entry);
+                    if file_info.entry.size != 0 {
+                        // If you have a length, you must have a cluster
+                        assert!(file_info.entry.cluster.0 != 0);
+                    }
+                    fat.write_entry_to_disk(&self.block_device, &file_info.entry)?;
+                }
+            };
+        }
+        Ok(())
     }
 
     /// Check if any files or folders are open.
@@ -1054,38 +1076,6 @@ where
         let available = Block::LEN - block_offset;
         Ok((block_idx, block_offset, available))
     }
-
-    /// Flush (update the entry) for a file with the given raw file handle and
-    /// get its index.
-    fn flush_file_get_index(&mut self, file: RawFile) -> Result<usize, Error<D::Error>> {
-        let mut found_idx = None;
-        for (idx, info) in self.open_files.iter().enumerate() {
-            if file == info.file_id {
-                found_idx = Some((info, idx));
-                break;
-            }
-        }
-
-        let (file_info, file_idx) = found_idx.ok_or(Error::BadHandle)?;
-
-        if file_info.dirty {
-            let volume_idx = self.get_volume_by_id(file_info.volume_id)?;
-            match self.open_volumes[volume_idx].volume_type {
-                VolumeType::Fat(ref mut fat) => {
-                    debug!("Updating FAT info sector");
-                    fat.update_info_sector(&self.block_device)?;
-                    debug!("Updating dir entry {:?}", file_info.entry);
-                    if file_info.entry.size != 0 {
-                        // If you have a length, you must have a cluster
-                        assert!(file_info.entry.cluster.0 != 0);
-                    }
-                    fat.write_entry_to_disk(&self.block_device, &file_info.entry)?;
-                }
-            };
-        }
-
-        Ok(file_idx)
-    }
 }
 
 /// Transform mode variants (ReadWriteCreate_Or_Append) to simple modes ReadWriteAppend or