diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index 08a50e8..462511a 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -211,7 +211,10 @@ pub(crate) struct FileInfo { pub(crate) file_id: RawFile, /// The unique ID for the volume this directory is on pub(crate) volume_id: RawVolume, - /// The current cluster, and how many bytes that short-cuts us + /// The last cluster we accessed, and how many bytes that short-cuts us. + /// + /// This saves us walking from the very start of the FAT chain when we move + /// forward through a file. pub(crate) current_cluster: (u32, ClusterId), /// How far through the file we've read (in bytes). pub(crate) current_offset: u32, @@ -236,41 +239,30 @@ impl FileInfo { /// Seek to a new position in the file, relative to the start of the file. pub fn seek_from_start(&mut self, offset: u32) -> Result<(), FileError> { - if offset <= self.entry.size { - self.current_offset = offset; - if offset < self.current_cluster.0 { - // Back to start - self.current_cluster = (0, self.entry.cluster); - } - Ok(()) - } else { - Err(FileError::InvalidOffset) + if offset > self.entry.size { + return Err(FileError::InvalidOffset); } + self.current_offset = offset; + Ok(()) } /// Seek to a new position in the file, relative to the end of the file. pub fn seek_from_end(&mut self, offset: u32) -> Result<(), FileError> { - if offset <= self.entry.size { - self.current_offset = self.entry.size - offset; - if offset < self.current_cluster.0 { - // Back to start - self.current_cluster = (0, self.entry.cluster); - } - Ok(()) - } else { - Err(FileError::InvalidOffset) + if offset > self.entry.size { + return Err(FileError::InvalidOffset); } + self.current_offset = self.entry.size - offset; + Ok(()) } /// Seek to a new position in the file, relative to the current position. pub fn seek_from_current(&mut self, offset: i32) -> Result<(), FileError> { let new_offset = i64::from(self.current_offset) + i64::from(offset); - if new_offset >= 0 && new_offset <= i64::from(self.entry.size) { - self.current_offset = new_offset as u32; - Ok(()) - } else { - Err(FileError::InvalidOffset) + if new_offset < 0 || new_offset > i64::from(self.entry.size) { + return Err(FileError::InvalidOffset); } + self.current_offset = new_offset as u32; + Ok(()) } /// Amount of file left to read. @@ -281,7 +273,6 @@ impl FileInfo { /// Update the file's length. pub(crate) fn update_length(&mut self, new: u32) { self.entry.size = new; - self.entry.size = new; } } diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index b5f3940..15e4f42 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -624,6 +624,7 @@ where let (block_idx, block_offset, block_avail) = self.find_data_on_disk( volume_idx, &mut current_cluster, + self.open_files[file_idx].entry.cluster, self.open_files[file_idx].current_offset, )?; self.open_files[file_idx].current_cluster = current_cluster; @@ -701,44 +702,45 @@ where written, bytes_to_write, current_cluster ); let current_offset = self.open_files[file_idx].current_offset; - let (block_idx, block_offset, block_avail) = - match self.find_data_on_disk(volume_idx, &mut current_cluster, current_offset) { - Ok(vars) => { - debug!( - "Found block_idx={:?}, block_offset={:?}, block_avail={}", - vars.0, vars.1, vars.2 - ); - vars - } - Err(Error::EndOfFile) => { - debug!("Extending file"); - match self.open_volumes[volume_idx].volume_type { - VolumeType::Fat(ref mut fat) => { - if fat - .alloc_cluster( - &self.block_device, - Some(current_cluster.1), - false, - ) - .is_err() - { - return Err(Error::DiskFull); - } - debug!("Allocated new FAT cluster, finding offsets..."); - let new_offset = self - .find_data_on_disk( - volume_idx, - &mut current_cluster, - self.open_files[file_idx].current_offset, - ) - .map_err(|_| Error::AllocationError)?; - debug!("New offset {:?}", new_offset); - new_offset + let (block_idx, block_offset, block_avail) = match self.find_data_on_disk( + volume_idx, + &mut current_cluster, + self.open_files[file_idx].entry.cluster, + current_offset, + ) { + Ok(vars) => { + debug!( + "Found block_idx={:?}, block_offset={:?}, block_avail={}", + vars.0, vars.1, vars.2 + ); + vars + } + Err(Error::EndOfFile) => { + debug!("Extending file"); + match self.open_volumes[volume_idx].volume_type { + VolumeType::Fat(ref mut fat) => { + if fat + .alloc_cluster(&self.block_device, Some(current_cluster.1), false) + .is_err() + { + return Err(Error::DiskFull); } + debug!("Allocated new FAT cluster, finding offsets..."); + let new_offset = self + .find_data_on_disk( + volume_idx, + &mut current_cluster, + self.open_files[file_idx].entry.cluster, + self.open_files[file_idx].current_offset, + ) + .map_err(|_| Error::AllocationError)?; + debug!("New offset {:?}", new_offset); + new_offset } } - Err(e) => return Err(e), - }; + } + Err(e) => return Err(e), + }; let mut blocks = [Block::new()]; let to_copy = core::cmp::min(block_avail, bytes_to_write - written); if block_offset != 0 { @@ -1043,18 +1045,33 @@ where /// This function turns `desired_offset` into an appropriate block to be /// read. It either calculates this based on the start of the file, or - /// from the last cluster we read - whichever is better. + /// from the given start point - whichever is better. + /// + /// Returns: + /// + /// * the index for the block on the disk that contains the data we want, + /// * the byte offset into that block for the data we want, and + /// * how many bytes remain in that block. fn find_data_on_disk( &self, volume_idx: usize, start: &mut (u32, ClusterId), + file_start: ClusterId, desired_offset: u32, ) -> Result<(BlockIdx, usize, usize), Error> { let bytes_per_cluster = match &self.open_volumes[volume_idx].volume_type { VolumeType::Fat(fat) => fat.bytes_per_cluster(), }; + // do we need to be before our start point? + if desired_offset < start.0 { + // user wants to go backwards - start from the beginning of the file + // because the FAT is only a singly-linked list. + start.0 = 0; + start.1 = file_start; + } // How many clusters forward do we need to go? let offset_from_cluster = desired_offset - start.0; + // walk through the FAT chain let num_clusters = offset_from_cluster / bytes_per_cluster; let mut block_cache = BlockCache::empty(); for _ in 0..num_clusters { @@ -1065,7 +1082,7 @@ where }; start.0 += bytes_per_cluster; } - // How many blocks in are we? + // How many blocks in are we now? let offset_from_cluster = desired_offset - start.0; assert!(offset_from_cluster < bytes_per_cluster); let num_blocks = BlockCount(offset_from_cluster / Block::LEN_U32); diff --git a/tests/read_file.rs b/tests/read_file.rs index b0e7628..b7c80c9 100644 --- a/tests/read_file.rs +++ b/tests/read_file.rs @@ -150,27 +150,31 @@ fn read_file_backwards() { const CHUNK_SIZE: u32 = 100; let length = volume_mgr.file_length(test_file).expect("file length"); - let mut offset = length - CHUNK_SIZE; let mut read = 0; + // go to end + volume_mgr.file_seek_from_end(test_file, 0).expect("seek"); + // We're going to read the file backwards in chunks of 100 bytes. This // checks we didn't make any assumptions about only going forwards. while read < length { + // go to start of next chunk volume_mgr - .file_seek_from_start(test_file, offset) + .file_seek_from_current(test_file, -(CHUNK_SIZE as i32)) .expect("seek"); + // read chunk let mut buffer = [0u8; CHUNK_SIZE as usize]; let len = volume_mgr.read(test_file, &mut buffer).expect("read"); assert_eq!(len, CHUNK_SIZE as usize); contents.push_front(buffer.to_vec()); read += CHUNK_SIZE; - if offset >= CHUNK_SIZE { - offset -= CHUNK_SIZE; - } + // go to start of chunk we just read + volume_mgr + .file_seek_from_current(test_file, -(CHUNK_SIZE as i32)) + .expect("seek"); } assert_eq!(read, length); - assert_eq!(offset, 0); let flat: Vec = contents.iter().flatten().copied().collect(); @@ -180,6 +184,27 @@ fn read_file_backwards() { assert_eq!(&hash[..], TEST_DAT_SHA256_SUM); } +#[test] +fn read_file_with_odd_seek() { + let time_source = utils::make_time_source(); + let disk = utils::make_block_device(utils::DISK_SOURCE).unwrap(); + let mut volume_mgr = embedded_sdmmc::VolumeManager::new(disk, time_source); + + let mut volume = volume_mgr + .open_volume(embedded_sdmmc::VolumeIdx(0)) + .unwrap(); + let mut root_dir = volume.open_root_dir().unwrap(); + let mut f = root_dir + .open_file_in_dir("64MB.DAT", embedded_sdmmc::Mode::ReadOnly) + .unwrap(); + f.seek_from_start(0x2c).unwrap(); + while f.offset() < 1000000 { + let mut buffer = [0u8; 2048]; + f.read(&mut buffer).unwrap(); + f.seek_from_current(-1024).unwrap(); + } +} + // **************************************************************************** // // End Of File