Skip to content

Commit

Permalink
use enum instead of bool
Browse files Browse the repository at this point in the history 
Reason: makes the funktionality more clear and less error prone
  • Loading branch information
@Murmele
Murmele committed Jun 3, 2024
1 parent 1e714bd commit 2fd18cb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 36 deletions.
3 changes: 2 additions & 1 deletion src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
79 changes: 44 additions & 35 deletions src/volume_mgr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -103,7 +109,7 @@ where
&mut self,
volume_idx: VolumeIdx,
) -> Result<Volume<D, T, MAX_DIRS, MAX_FILES, MAX_VOLUMES>, Error<D::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.
Expand All @@ -115,20 +121,19 @@ where
&mut self,
volume_idx: VolumeIdx,
) -> Result<Volume<D, T, MAX_DIRS, MAX_FILES, MAX_VOLUMES>, Error<D::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
/// concept of drive letters - that is for a higher layer to handle.
fn _open_volume(
&mut self,
volume_idx: VolumeIdx,
read_only: bool,
open_mode: VolumeOpenMode,
) -> Result<Volume<D, T, MAX_DIRS, MAX_FILES, MAX_VOLUMES>, Error<D::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)?;
Expand All @@ -146,7 +151,7 @@ where
pub fn open_raw_volume(
&mut self,
volume_idx: VolumeIdx,
read_only: bool,
open_mode: VolumeOpenMode,
) -> Result<RawVolume, Error<D::Error>> {
const PARTITION1_START: usize = 446;
const PARTITION2_START: usize = PARTITION1_START + PARTITION_INFO_LENGTH;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -1691,7 +1696,7 @@ mod tests {
DUMMY,
DUMMY,
FAT32_PARTITION0_FAT_TABLE,
])
]),
}
}

Expand All @@ -1707,7 +1712,7 @@ mod tests {
DUMMY,
DUMMY,
FAT32_PARTITION0_FAT_TABLE_DIRTY,
])
]),
}
}

Expand All @@ -1724,7 +1729,7 @@ mod tests {
DUMMY,
DUMMY,
FAT32_PARTITION0_FAT_TABLE,
])
]),
}
}
}
Expand All @@ -1745,8 +1750,6 @@ mod tests {
start_block_idx: BlockIdx,
_reason: &str,
) -> Result<(), Self::Error> {


println!(
"Reading block {} to {}",
start_block_idx.0,
Expand Down Expand Up @@ -1780,21 +1783,19 @@ mod tests {
#[test]
fn partition0() {
let mut c: VolumeManager<DummyBlockDevice, Clock, 2, 2> =
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!(
&c.open_volumes[0],
&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),
Expand All @@ -1816,23 +1817,24 @@ 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]
fn partition0_dirty() {
let mut c: VolumeManager<DummyBlockDevice, Clock, 2, 2> =
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!(
&c.open_volumes[0],
&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),
Expand All @@ -1858,22 +1860,23 @@ mod tests {

#[test]
fn partition0_set_dirty() {
let mut c: VolumeManager<DummyBlockDevice, Clock, 2, 2> =
VolumeManager::new_with_limits(
DummyBlockDevice::new_not_dirty_fattable_size_5(),
Clock,
0xAA00_0000,
);
let mut c: VolumeManager<DummyBlockDevice, Clock, 2, 2> = 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!(
&c.open_volumes[0],
&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),
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 2fd18cb

Please sign in to comment.