From 2fd18cbaa92ca31ceac2922de05e50e3511ca88b Mon Sep 17 00:00:00 2001 From: Martin Marmsoler Date: Mon, 3 Jun 2024 21:21:56 +0200 Subject: [PATCH] use enum instead of bool Reason: makes the funktionality more clear and less error prone --- src/lib.rs | 3 +- src/volume_mgr.rs | 79 ++++++++++++++++++++++++++--------------------- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3d32ddf..aa98fb6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,6 +75,7 @@ pub mod filesystem; pub mod sdcard; use filesystem::SearchId; +use volume_mgr::VolumeOpenMode; #[doc(inline)] pub use crate::blockdevice::{Block, BlockCount, BlockDevice, BlockIdx}; @@ -347,7 +348,7 @@ pub(crate) struct VolumeInfo { /// What kind of volume this is volume_type: VolumeType, /// Flag to indicate if the volume was opened as read only. If read only, files cannot be opened in write mode! - read_only: bool, + open_mode: VolumeOpenMode, } /// This enum holds the data for the various different types of filesystems we diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index 3e62fd4..86e087c 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -18,6 +18,12 @@ use crate::{ }; use heapless::Vec; +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum VolumeOpenMode { + ReadOnly, + ReadWrite, +} + /// A `VolumeManager` wraps a block device and gives access to the FAT-formatted /// volumes within it. #[derive(Debug)] @@ -103,7 +109,7 @@ where &mut self, volume_idx: VolumeIdx, ) -> Result, Error> { - return self._open_volume(volume_idx, false); + return self._open_volume(volume_idx, VolumeOpenMode::ReadWrite); } /// Get a read only volume (or partition) based on entries in the Master Boot Record. @@ -115,9 +121,8 @@ where &mut self, volume_idx: VolumeIdx, ) -> Result, Error> { - return self._open_volume(volume_idx, true); + return self._open_volume(volume_idx, VolumeOpenMode::ReadOnly); } - /// Get a volume (or partition) based on entries in the Master Boot Record. /// /// We do not support GUID Partition Table disks. Nor do we support any @@ -125,10 +130,10 @@ where fn _open_volume( &mut self, volume_idx: VolumeIdx, - read_only: bool, + open_mode: VolumeOpenMode, ) -> Result, Error> { - let v = self.open_raw_volume(volume_idx, read_only)?; - if !read_only { + let v = self.open_raw_volume(volume_idx, open_mode)?; + if open_mode != VolumeOpenMode::ReadOnly { let idx = self.get_volume_by_id(v)?; let VolumeType::Fat(volume_type) = &self.open_volumes[idx].volume_type; self.set_volume_status_dirty(volume_type, true)?; @@ -146,7 +151,7 @@ where pub fn open_raw_volume( &mut self, volume_idx: VolumeIdx, - read_only: bool, + open_mode: VolumeOpenMode, ) -> Result> { const PARTITION1_START: usize = 446; const PARTITION2_START: usize = PARTITION1_START + PARTITION_INFO_LENGTH; @@ -225,7 +230,7 @@ where volume_id: id, idx: volume_idx, volume_type: volume, - read_only: read_only, + open_mode: open_mode, }; // We already checked for space self.open_volumes.push(info).unwrap(); @@ -354,7 +359,7 @@ where } } let volume_idx = self.get_volume_by_id(volume)?; - if !self.open_volumes[volume_idx].read_only { + if self.open_volumes[volume_idx].open_mode != VolumeOpenMode::ReadOnly { let VolumeType::Fat(volume_type) = &self.open_volumes[volume_idx].volume_type; self.set_volume_status_dirty(volume_type, false)?; } @@ -555,7 +560,7 @@ where let volume_info = &self.open_volumes[volume_idx]; let sfn = name.to_short_filename().map_err(Error::FilenameError)?; - if volume_info.read_only && mode != Mode::ReadOnly { + if volume_info.open_mode == VolumeOpenMode::ReadOnly && mode != Mode::ReadOnly { return Err(Error::VolumeReadOnly); } @@ -1691,7 +1696,7 @@ mod tests { DUMMY, DUMMY, FAT32_PARTITION0_FAT_TABLE, - ]) + ]), } } @@ -1707,7 +1712,7 @@ mod tests { DUMMY, DUMMY, FAT32_PARTITION0_FAT_TABLE_DIRTY, - ]) + ]), } } @@ -1724,7 +1729,7 @@ mod tests { DUMMY, DUMMY, FAT32_PARTITION0_FAT_TABLE, - ]) + ]), } } } @@ -1745,8 +1750,6 @@ mod tests { start_block_idx: BlockIdx, _reason: &str, ) -> Result<(), Self::Error> { - - println!( "Reading block {} to {}", start_block_idx.0, @@ -1780,13 +1783,11 @@ mod tests { #[test] fn partition0() { let mut c: VolumeManager = - VolumeManager::new_with_limits( - DummyBlockDevice::new_not_dirty(), - Clock, - 0xAA00_0000, - ); + VolumeManager::new_with_limits(DummyBlockDevice::new_not_dirty(), Clock, 0xAA00_0000); - let v = c.open_raw_volume(VolumeIdx(0), false).unwrap(); + let v = c + .open_raw_volume(VolumeIdx(0), VolumeOpenMode::ReadWrite) + .unwrap(); let expected_id = RawVolume(SearchId(0xAA00_0000)); assert_eq!(v, expected_id); assert_eq!( @@ -1794,7 +1795,7 @@ mod tests { &VolumeInfo { volume_id: expected_id, idx: VolumeIdx(0), - read_only: false, + open_mode: VolumeOpenMode::ReadWrite, volume_type: VolumeType::Fat(crate::FatVolume { lba_start: BlockIdx(1), num_blocks: BlockCount(0x0011_2233), @@ -1816,7 +1817,6 @@ mod tests { ); let VolumeType::Fat(fat_info) = &c.open_volumes[0].volume_type; assert_eq!(c.volume_status_dirty(fat_info).unwrap(), false); - c.set_volume_status_dirty(fat_info, true).unwrap(); } #[test] @@ -1824,7 +1824,9 @@ mod tests { let mut c: VolumeManager = VolumeManager::new_with_limits(DummyBlockDevice::new_dirty(), Clock, 0xAA00_0000); - let v = c.open_raw_volume(VolumeIdx(0), false).unwrap(); + let v = c + .open_raw_volume(VolumeIdx(0), VolumeOpenMode::ReadWrite) + .unwrap(); let expected_id = RawVolume(SearchId(0xAA00_0000)); assert_eq!(v, expected_id); assert_eq!( @@ -1832,7 +1834,7 @@ mod tests { &VolumeInfo { volume_id: expected_id, idx: VolumeIdx(0), - read_only: false, + open_mode: VolumeOpenMode::ReadWrite, volume_type: VolumeType::Fat(crate::FatVolume { lba_start: BlockIdx(1), num_blocks: BlockCount(0x0011_2233), @@ -1858,14 +1860,15 @@ mod tests { #[test] fn partition0_set_dirty() { - let mut c: VolumeManager = - VolumeManager::new_with_limits( - DummyBlockDevice::new_not_dirty_fattable_size_5(), - Clock, - 0xAA00_0000, - ); + let mut c: VolumeManager = VolumeManager::new_with_limits( + DummyBlockDevice::new_not_dirty_fattable_size_5(), + Clock, + 0xAA00_0000, + ); - let v = c.open_raw_volume(VolumeIdx(0), false).unwrap(); + let v = c + .open_raw_volume(VolumeIdx(0), VolumeOpenMode::ReadWrite) + .unwrap(); let expected_id = RawVolume(SearchId(0xAA00_0000)); assert_eq!(v, expected_id); assert_eq!( @@ -1873,7 +1876,7 @@ mod tests { &VolumeInfo { volume_id: expected_id, idx: VolumeIdx(0), - read_only: false, + open_mode: VolumeOpenMode::ReadWrite, volume_type: VolumeType::Fat(crate::FatVolume { lba_start: BlockIdx(1), num_blocks: BlockCount(0x0011_2233), @@ -1896,7 +1899,10 @@ mod tests { let VolumeType::Fat(fat_info) = &c.open_volumes[0].volume_type; assert_eq!(c.volume_status_dirty(fat_info).unwrap(), false); assert_eq!(c.block_device.blocks.borrow()[0], MBR_BLOCK); - assert_eq!(c.block_device.blocks.borrow()[1], FAT32_PARTITION0_BOOT_FAT_TABLE_SIZE_5); + assert_eq!( + c.block_device.blocks.borrow()[1], + FAT32_PARTITION0_BOOT_FAT_TABLE_SIZE_5 + ); assert_eq!(c.block_device.blocks.borrow()[2], FAT32_PARTITION0_FSINFO); assert_eq!(c.block_device.blocks.borrow()[3].contents[7] & (1 << 3), 8); assert_eq!(c.block_device.blocks.borrow()[4], DUMMY); @@ -1913,7 +1919,10 @@ mod tests { c.set_volume_status_dirty(fat_info, false).unwrap(); assert_eq!(c.volume_status_dirty(fat_info).unwrap(), false); assert_eq!(c.block_device.blocks.borrow()[0], MBR_BLOCK); - assert_eq!(c.block_device.blocks.borrow()[1], FAT32_PARTITION0_BOOT_FAT_TABLE_SIZE_5); + assert_eq!( + c.block_device.blocks.borrow()[1], + FAT32_PARTITION0_BOOT_FAT_TABLE_SIZE_5 + ); assert_eq!(c.block_device.blocks.borrow()[2], FAT32_PARTITION0_FSINFO); assert_eq!(c.block_device.blocks.borrow()[3].contents[7] & (1 << 3), 8); assert_eq!(c.block_device.blocks.borrow()[4], DUMMY);