From 2ec0a90e5882a2e917fc46dc2e767aa3691887a3 Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Fri, 11 Sep 2020 13:25:22 +0100 Subject: [PATCH] Simplified `Read`, `Write` and `Seek` implementations of `GameFile`. Added some unit tests for `game_file`. Added `Data::remaining_bytes`. Fixed bug in `Real::try_to_data`. Renamed `Real::data_section_end` to `cur_sector_data_section_end`. Added `Real::next_sector_data_section_start`. --- src/io/address/data.rs | 11 +++ src/io/address/real.rs | 26 ++++--- src/io/game_file.rs | 152 ++++++++++++++++++--------------------- src/io/game_file/test.rs | 45 ++++++++++++ 4 files changed, 141 insertions(+), 93 deletions(-) create mode 100644 src/io/game_file/test.rs diff --git a/src/io/address/data.rs b/src/io/address/data.rs index aeb669a..a2f2ef9 100644 --- a/src/io/address/data.rs +++ b/src/io/address/data.rs @@ -1,6 +1,8 @@ //! File data-only addresses // Imports +use std::convert::TryInto; + use crate::{ io::address::Real, util::{abs_diff, signed_offset}, @@ -44,6 +46,15 @@ impl Data { ) } + /// Returns the remaining bytes in this data section + #[must_use] + pub fn remaining_bytes(self) -> u64 { + // Note: This can't panic, as we know it's positive. + (self.to_real().cur_sector_data_section_end() - self.to_real()) + .try_into() + .expect("Offset was negative") + } + /// Returns the sector associated with this address #[must_use] pub const fn sector(self) -> u64 { diff --git a/src/io/address/real.rs b/src/io/address/real.rs index e2a582c..629146b 100644 --- a/src/io/address/real.rs +++ b/src/io/address/real.rs @@ -69,7 +69,7 @@ impl Real { // real offset to get the data offset #[rustfmt::skip] Ok(Data::from_u64( - Self::SECTOR_BYTE_SIZE * real_sector + // Base of data sector + Self::DATA_BYTE_SIZE * real_sector + // Base of data sector real_sector_offset - Self::HEADER_BYTE_SIZE, // Data offset (skipping header) )) } @@ -86,18 +86,26 @@ impl Real { self.as_u64() % Self::SECTOR_BYTE_SIZE } - /// Returns the address of the end of the data section in this sector. + /// Returns the address of the end of the data section in the current sector. #[must_use] - pub const fn data_section_end(self) -> Self { - // Get the sector - let real_sector = self.sector(); - + pub const fn cur_sector_data_section_end(self) -> Self { // The end of the real data section is after the header and data sections #[rustfmt::skip] Self::from_u64( - Self::SECTOR_BYTE_SIZE * real_sector + // Beginning of sector - Self::HEADER_BYTE_SIZE + // Skip Header - Self:: DATA_BYTE_SIZE, // Skip Data + Self::SECTOR_BYTE_SIZE * self.sector() + // Beginning of sector + Self::HEADER_BYTE_SIZE + // Skip Header + Self:: DATA_BYTE_SIZE, // Skip Data + ) + } + + /// Returns the start address of the data section on the next sector + #[must_use] + pub const fn next_sector_data_section_start(self) -> Self { + // The start is after the header in the next sector + #[rustfmt::skip] + Self::from_u64( + Self::SECTOR_BYTE_SIZE * (self.sector() + 1) + // Beginning of sector + Self::HEADER_BYTE_SIZE // Skip header ) } diff --git a/src/io/game_file.rs b/src/io/game_file.rs index 542f34d..3659229 100644 --- a/src/io/game_file.rs +++ b/src/io/game_file.rs @@ -2,6 +2,10 @@ //! //! See [`GameFile`] for details +// Modules +#[cfg(test)] +mod test; + // Imports use crate::io::address::{real, Data as DataAddress, Real as RealAddress}; use std::{ @@ -36,6 +40,9 @@ use std::{ /// This will require an `io` call for every single 2048 byte block instead /// of an unique call for all of the block, but due to the invariants required, /// this is the strategy employed. +/// +/// # Seek +/// All seeks are done in data addresses, the stream position is also in data addresses. #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash, Debug)] pub struct GameFile { /// The type to read and write from @@ -53,6 +60,8 @@ pub enum NewGameFileError { // Constructors impl GameFile { /// Constructs a `GameFile` given a reader + /// + /// This seeks the reader to the start of the data section on the first sector pub fn from_reader(mut reader: R) -> Result { // Seek the reader to the beginning of the data section reader @@ -74,49 +83,34 @@ impl Read for GameFile { // Total length of the buffer to fill let total_buf_len = buf.len(); + // Current address + let mut cur_address = RealAddress::from_u64(self.reader.stream_position()?); + // While the buffer isn't empty while !buf.is_empty() { - // Get the current real address we're at in the reader - // Note: If we can't get the position, we return immediately - let cur_real_address = RealAddress::from(self.reader.stream_position()?); - - // Get the data section end - let data_section_end = cur_real_address.data_section_end(); - - // If we're at the end of the data section, seek to the next data section - if cur_real_address == data_section_end { - // Seek ahead by skipping the footer and next header - self.reader.seek(SeekFrom::Current( - (RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE) - .try_into() - .expect("Sector offset didn't fit into `u64`"), - ))?; - - // And restart this loop + // If we're at the end of the current data section, seek to the start of the next data data section and restart + if cur_address == cur_address.cur_sector_data_section_end() { + cur_address = RealAddress::from_u64( + self.reader.seek(SeekFrom::Start( + cur_address + .next_sector_data_section_start() + .try_into() + .expect("Sector offset didn't fit into `i64`"), + ))?, + ); continue; } - // We always guarantee that the current address lies within the data sections - // Note: We only check it here, because `cur_real_address` may be `data_section_end` - // during seeking. - assert!( - cur_real_address.in_data_section(), - "Real offset {} [Sector {}, Offset {}] could not be read as it was not in the data section", - cur_real_address, - cur_real_address.sector(), - cur_real_address.offset() - ); - - // Check how many bytes we can read - // Note: Cannot overflow, max is `2048`, so an error means `cur_real_address` was past the data section end - let mut bytes_to_read = (data_section_end - cur_real_address) + // Get how many bytes we can read, The minimum between the end of the data section and the size of the buffer + // Note: Can't overflow, max is `2048` + // Note: At this point, `cur_address` must be within the data section + let bytes_to_read = cur_address + .try_to_data() + .expect("Address wasn't in data section") + .remaining_bytes() + .min(buf.len().try_into().expect("Unable to convert `usize` to `u64`")) .try_into() - .expect("Current address is past end of data"); - - // If we were to read more bytes than the buffer has, read less - if bytes_to_read > buf.len() { - bytes_to_read = buf.len(); - } + .expect("Unable to convert number 0..2048 to `i64`"); // Read either until the end of the data section or until buffer is full // Note: If any fail, we immediately return Err @@ -127,8 +121,10 @@ impl Read for GameFile { return Ok(total_buf_len - buf.len()); } - // Discard what we've already read - buf = &mut buf[bytes_read..]; // If `bytes_to_read == buf.len()` this does not panic + // And discard what we've already read + // Note: This slice can't panic, as `bytes_to_read` is at most `buf.len()` + buf = &mut buf[bytes_read..]; + cur_address = cur_address.next_sector_data_section_start(); } // And return the bytes we read @@ -147,60 +143,48 @@ impl Write for GameFile { // Total length of the buffer to write let total_buf_len = buf.len(); + // Current address + let mut cur_address = RealAddress::from_u64(self.reader.stream_position()?); + // While the buffer isn't empty while !buf.is_empty() { - // Get the current real address we're at in the reader - // Note: If we can't get the position, we return immediately - let cur_real_address = RealAddress::from(self.reader.stream_position()?); - - // Get the data section end - let data_section_end = cur_real_address.data_section_end(); - - // If we're at the end of the data section, seek to the next data section - if cur_real_address == data_section_end { - // Seek ahead by skipping the footer and next header - self.reader.seek(SeekFrom::Current( - (RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE) - .try_into() - .expect("Sector offset didn't fit into `u64`"), - ))?; - - // And restart this loop + // If we're at the end of the current data section, seek to the start of the next data data section and restart + if cur_address == cur_address.cur_sector_data_section_end() { + cur_address = RealAddress::from_u64( + self.reader.seek(SeekFrom::Start( + cur_address + .next_sector_data_section_start() + .try_into() + .expect("Sector offset didn't fit into `i64`"), + ))?, + ); continue; } - // We always guarantee that the current address lies within the data sections - // Note: We only check it here, because `cur_real_address` may be `data_section_end` - // during seeking. - assert!( - cur_real_address.in_data_section(), - "Real offset {} [Sector {}, Offset {}] could not be written as it was not in the data section", - cur_real_address, - cur_real_address.sector(), - cur_real_address.offset() - ); - - // Check how many bytes we can write, up to the buffer's len - let mut bytes_to_write = (data_section_end - cur_real_address) + // Get how many bytes we can write, The minimum between the end of the data section and the size of the buffer + // Note: Can't overflow, max is `2048` + // Note: At this point, `cur_address` must be within the data section + let bytes_to_write = cur_address + .try_to_data() + .expect("Address wasn't in data section") + .remaining_bytes() + .min(buf.len().try_into().expect("Unable to convert `usize` to `u64`")) .try_into() - .expect("Current address is past end of data"); + .expect("Unable to convert number 0..2048 to `i64`"); - // If we were to write more bytes than the buffer has, write less - if bytes_to_write > buf.len() { - bytes_to_write = buf.len(); - } - - // Write either until the end of the data section or until buffer runs out - // Note: If this fails, we immediately return Err + // Write either until the end of the data section or until buffer is full + // Note: If any fail, we immediately return Err let bytes_written = self.reader.write(&buf[0..bytes_to_write])?; - // If 0 bytes were written, EOF was reached, so return with however many we've read + // If 0 bytes were read, EOF was reached, so return with however many we've read if bytes_written == 0 { return Ok(total_buf_len - buf.len()); } - // Discard what we've already written - buf = &buf[bytes_to_write..]; // If `bytes_to_write == buf.len()` this does not panic + // And discard what we've already read + // Note: This slice can't panic, as `bytes_to_read` is at most `buf.len()` + buf = &buf[bytes_written..]; + cur_address = cur_address.next_sector_data_section_start(); } // And return the bytes we read @@ -219,12 +203,12 @@ impl Write for GameFile { pub struct SeekNonDataError(#[source] real::ToDataError); impl Seek for GameFile { - fn seek(&mut self, data_pos: SeekFrom) -> std::io::Result { + fn seek(&mut self, data_seek: SeekFrom) -> std::io::Result { // Imports use std::ops::Add; - // Calculate the real position - let real_pos = match data_pos { + // Calculate the real seek + let real_seek = match data_seek { SeekFrom::Start(data_address) => SeekFrom::Start( // Parse the address as data, then convert it to real DataAddress::from(data_address).to_real().as_u64(), @@ -244,7 +228,7 @@ impl Seek for GameFile { }; // Seek to the real position and get where we are right now - let cur_real_address = RealAddress::from(self.reader.seek(real_pos)?); + let cur_real_address = RealAddress::from(self.reader.seek(real_seek)?); // Get the data address let data_address = cur_real_address diff --git a/src/io/game_file/test.rs b/src/io/game_file/test.rs new file mode 100644 index 0000000..f5cfac4 --- /dev/null +++ b/src/io/game_file/test.rs @@ -0,0 +1,45 @@ +//! Tests + +// Imports +use super::*; +use std::{convert::TryFrom, io::Cursor}; + +#[test] +fn seek_start() { + let buf = vec![0; 10_000]; + let cursor = Cursor::new(buf); + let mut game_file = GameFile::from_reader(cursor).expect("Unable to create game file"); + + // Check initial position + assert_eq!(game_file.stream_position().expect("Unable to get stream position"), 0); + + // Check other seeks + for &seek_pos in &[1, 2047, 2048, 2049, 4095, 4096, 4097] { + game_file.seek(SeekFrom::Start(seek_pos)).expect("Unable to seek"); + + assert_eq!(game_file.stream_position().expect("Unable to get stream position"), seek_pos); + } +} + +#[test] +fn seek_cur() { + let buf = vec![0; 10_000]; + let cursor = Cursor::new(buf); + let mut game_file = GameFile::from_reader(cursor).expect("Unable to create game file"); + + // Check initial position + assert_eq!(game_file.stream_position().expect("Unable to get stream position"), 0); + + // Check other seeks + let mut cur_pos = 0u64; + for &seek_offset in &[0, 1, 2046, 1, 0, 1, 2048, 2047, 2049] { + game_file + .seek(SeekFrom::Current(i64::try_from(seek_offset).expect("Unable to convert to `i64`"))) + .expect("Unable to seek"); + cur_pos += seek_offset; + + assert_eq!(game_file.stream_position().expect("Unable to get stream position"), cur_pos); + } +} + +// TODO: Read and write tests.