From 39a16fc1175178278ebc6e8daadcbd156a29a994 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 11 Jul 2025 16:49:35 +0100 Subject: [PATCH] Destructure arrays to avoid unnecessarily fallible conversions This partly relies on slice::as_chunks(), which was added in Rust v1.88.0. --- src/archive/ba2.rs | 31 ++++------ src/archive/bsa.rs | 137 +++++++++++++++++++++++++++---------------- src/archive/error.rs | 12 ---- src/archive/parse.rs | 34 ----------- 4 files changed, 100 insertions(+), 114 deletions(-) diff --git a/src/archive/ba2.rs b/src/archive/ba2.rs index e9990802..11956555 100644 --- a/src/archive/ba2.rs +++ b/src/archive/ba2.rs @@ -4,9 +4,7 @@ use std::{ io::{BufRead, Seek}, }; -use super::error::{ArchiveParsingError, slice_too_small}; - -use super::parse::{to_u32, to_u64}; +use super::error::ArchiveParsingError; pub(super) const TYPE_ID: [u8; 4] = *b"BTDX"; const HEADER_SIZE: usize = 24; @@ -26,12 +24,20 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { type Error = ArchiveParsingError; fn try_from(value: [u8; HEADER_SIZE - TYPE_ID.len()]) -> Result { + // LIMITATION: There's no syntax to infallibly split an array into sub-arrays. + #[rustfmt::skip] + let [ + v0, v1, v2, v3, // version + a0, a1, a2, a3, // archive_type + c0, c1, c2, c3, // file_count + file_paths_offset @ .., + ] = value; let header = Self { type_id: TYPE_ID, - version: to_u32(&value, 0)?, - archive_type: to_archive_type(&value)?, - file_count: to_u32(&value, 8)?, - file_paths_offset: to_u64(&value, 12)?, + version: u32::from_le_bytes([v0, v1, v2, v3]), + archive_type: [a0, a1, a2, a3], + file_count: u32::from_le_bytes([c0, c1, c2, c3]), + file_paths_offset: u64::from_le_bytes(file_paths_offset), }; // The header version is 1, 7 or 8 for Fallout 4 and 2 or 3 for Starfield. @@ -51,17 +57,6 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { } } -fn to_archive_type( - array: &[u8; HEADER_SIZE - TYPE_ID.len()], -) -> Result<[u8; 4], ArchiveParsingError> { - let slice = &array[4..8]; - - slice - .try_into() - // This should be impossible, but it can't be asserted at compile time. - .map_err(|_e| slice_too_small(slice, 4)) -} - pub(super) fn read_assets( mut reader: T, ) -> Result>, ArchiveParsingError> { diff --git a/src/archive/bsa.rs b/src/archive/bsa.rs index 04c7d07e..4e1f724b 100644 --- a/src/archive/bsa.rs +++ b/src/archive/bsa.rs @@ -5,8 +5,6 @@ use std::{ use super::error::ArchiveParsingError; -use super::parse::{to_u32, to_u64, to_usize}; - pub(super) const TYPE_ID: [u8; 4] = *b"BSA\0"; const HEADER_SIZE: usize = 36; const FILE_RECORD_SIZE: usize = 16; @@ -28,16 +26,48 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { type Error = ArchiveParsingError; fn try_from(value: [u8; HEADER_SIZE - TYPE_ID.len()]) -> Result { + // LIMITATION: There's no syntax to infallibly split an array into sub-arrays. + #[rustfmt::skip] + let [ + v0, v1, v2, v3, // version + r0, r1, r2, r3, // records_offset + a0, a1, a2, a3, // archive_flags + folder_count0, folder_count1, folder_count2, folder_count3, + file_count0, file_count1, file_count2, file_count3, + folder_names0, folder_names1, folder_names2, folder_names3, + file_names0, file_names1, file_names2, file_names3, + content_type_flags @ .., + ] = value; let header = Self { type_id: TYPE_ID, - version: to_u32(&value, 0)?, - records_offset: to_u32(&value, 4)?, - archive_flags: to_u32(&value, 8)?, - folder_count: to_u32(&value, 12)?, - total_file_count: to_u32(&value, 16)?, - total_folder_names_length: to_u32(&value, 20)?, - total_file_names_length: to_u32(&value, 24)?, - content_type_flags: to_u32(&value, 28)?, + version: u32::from_le_bytes([v0, v1, v2, v3]), + records_offset: u32::from_le_bytes([r0, r1, r2, r3]), + archive_flags: u32::from_le_bytes([a0, a1, a2, a3]), + folder_count: u32::from_le_bytes([ + folder_count0, + folder_count1, + folder_count2, + folder_count3, + ]), + total_file_count: u32::from_le_bytes([ + file_count0, + file_count1, + file_count2, + file_count3, + ]), + total_folder_names_length: u32::from_le_bytes([ + folder_names0, + folder_names1, + folder_names2, + folder_names3, + ]), + total_file_names_length: u32::from_le_bytes([ + file_names0, + file_names1, + file_names2, + file_names3, + ]), + content_type_flags: u32::from_le_bytes(content_type_flags), }; if to_usize(header.records_offset) != HEADER_SIZE { @@ -62,54 +92,43 @@ struct FolderRecord { // Also used for v104 BSAs. mod v103 { - use crate::archive::{ - error::ArchiveParsingError, - parse::{to_u32, to_u64}, - }; - use super::FolderRecord; pub(super) const FOLDER_RECORD_SIZE: usize = 16; - pub(super) fn read_folder_record(value: &[u8]) -> Result { - if value.len() < FOLDER_RECORD_SIZE { - return Err(ArchiveParsingError::SliceTooSmall { - expected: FOLDER_RECORD_SIZE, - actual: value.len(), - }); - } + pub(super) fn read_folder_record(value: &[u8; FOLDER_RECORD_SIZE]) -> FolderRecord { + // LIMITATION: There's no syntax to infallibly split an array into sub-arrays. + let [name_hash @ .., c0, c1, c2, c3, o0, o1, o2, o3] = *value; - Ok(FolderRecord { - name_hash: to_u64(value, 0)?, - file_count: to_u32(value, 8)?, - file_records_offset: to_u32(value, 12)?, - }) + FolderRecord { + name_hash: u64::from_le_bytes(name_hash), + file_count: u32::from_le_bytes([c0, c1, c2, c3]), + file_records_offset: u32::from_le_bytes([o0, o1, o2, o3]), + } } } mod v105 { - use crate::archive::{ - error::ArchiveParsingError, - parse::{to_u32, to_u64}, - }; - use super::FolderRecord; pub(super) const FOLDER_RECORD_SIZE: usize = 24; - pub(super) fn read_folder_record(value: &[u8]) -> Result { - if value.len() < FOLDER_RECORD_SIZE { - return Err(ArchiveParsingError::SliceTooSmall { - expected: FOLDER_RECORD_SIZE, - actual: value.len(), - }); - } + pub(super) fn read_folder_record(value: &[u8; FOLDER_RECORD_SIZE]) -> FolderRecord { + // LIMITATION: There's no syntax to infallibly split an array into sub-arrays. + #[rustfmt::skip] + let [ + name_hash @ .., + c0, c1, c2, c3, // file_count + _, _, _, _, + o0, o1, o2, o3, // file_records_offset + _, _, _, _, + ] = *value; - Ok(FolderRecord { - name_hash: to_u64(value, 0)?, - file_count: to_u32(value, 8)?, - file_records_offset: to_u32(value, 16)?, - }) + FolderRecord { + name_hash: u64::from_le_bytes(name_hash), + file_count: u32::from_le_bytes([c0, c1, c2, c3]), + file_records_offset: u32::from_le_bytes([o0, o1, o2, o3]), + } } } @@ -142,7 +161,7 @@ pub(super) fn read_assets( fn read_assets_with_header( mut reader: T, header: &Header, - read_folder_record: impl Fn(&[u8]) -> Result, + read_folder_record: impl Fn(&[u8; U]) -> FolderRecord, ) -> Result>, ArchiveParsingError> { let mut folders_buffer: Vec = vec![0; U * to_usize(header.folder_count)]; @@ -160,8 +179,8 @@ fn read_assets_with_header( HEADER_SIZE + folders_buffer.len() + to_usize(header.total_file_names_length); let mut assets = BTreeMap::new(); - for chunk in folders_buffer.chunks_exact(U) { - let folder_record = read_folder_record(chunk)?; + for chunk in folders_buffer.as_chunks::().0 { + let folder_record = read_folder_record(chunk); let entry = assets.entry(folder_record.name_hash); if let Entry::Occupied(_) = entry { @@ -177,7 +196,7 @@ fn read_assets_with_header( to_usize(folder_record.file_records_offset) - folder_record_offset_baseline; if let Some(folder_name_length) = file_records_buffer.get(folder_name_length_offset) { - folder_name_length_offset + 1 + to_usize(u32::from(*folder_name_length)) + folder_name_length_offset + 1 + usize::from(*folder_name_length) } else { return Err(ArchiveParsingError::InvalidFolderNameLengthOffset( folder_name_length_offset, @@ -194,10 +213,12 @@ fn read_assets_with_header( let file_hashes: &mut BTreeSet = entry.or_default(); for file_chunk in file_records_buffer - .chunks_exact(FILE_RECORD_SIZE) + .as_chunks::() + .0 + .iter() .take(to_usize(folder_record.file_count)) { - let file_hash = to_u64(file_chunk, 0)?; + let file_hash = file_record_hash(file_chunk); if !file_hashes.insert(file_hash) { return Err(ArchiveParsingError::HashCollision { @@ -210,3 +231,19 @@ fn read_assets_with_header( Ok(assets) } + +fn file_record_hash(file_record: &[u8; FILE_RECORD_SIZE]) -> u64 { + // LIMITATION: There's no syntax to infallibly split an array into sub-arrays. + let [hash_bytes @ .., _, _, _, _, _, _, _, _] = file_record; + u64::from_le_bytes(*hash_bytes) +} + +#[expect( + clippy::as_conversions, + reason = "A compile-time assertion ensures that this conversion will be lossless on all relevant target platforms" +)] +const fn to_usize(value: u32) -> usize { + // Error at compile time if this conversion isn't lossless. + const _: () = assert!(u32::BITS <= usize::BITS, "cannot fit a u32 into a usize!"); + value as usize +} diff --git a/src/archive/error.rs b/src/archive/error.rs index ca5caa05..69d9ac6f 100644 --- a/src/archive/error.rs +++ b/src/archive/error.rs @@ -49,7 +49,6 @@ pub(crate) enum ArchiveParsingError { UsesBigEndianNumbers, FolderHashCollision(u64), HashCollision { folder_hash: u64, file_hash: u64 }, - SliceTooSmall { expected: usize, actual: usize }, } impl std::fmt::Display for ArchiveParsingError { @@ -81,10 +80,6 @@ impl std::fmt::Display for ArchiveParsingError { f, "unexpected collision for file name hash {file_hash:x} in set for folder name hash {folder_hash:x}" ), - Self::SliceTooSmall { expected, actual } => write!( - f, - "byte slice was unexpectedly too small: expected {expected} bytes, got {actual} bytes" - ), } } } @@ -103,10 +98,3 @@ impl From for ArchiveParsingError { ArchiveParsingError::IoError(value) } } - -pub(super) fn slice_too_small(slice: &[u8], expected_size: usize) -> ArchiveParsingError { - ArchiveParsingError::SliceTooSmall { - expected: expected_size, - actual: slice.len(), - } -} diff --git a/src/archive/parse.rs b/src/archive/parse.rs index 49e4538a..2497198c 100644 --- a/src/archive/parse.rs +++ b/src/archive/parse.rs @@ -7,7 +7,6 @@ use std::{ use super::error::{ArchiveParsingError, ArchivePathParsingError}; use crate::{ - archive::error::slice_too_small, escape_ascii, logging::{self, format_details}, plugin::has_ascii_extension, @@ -95,39 +94,6 @@ fn get_assets_in_archive( } } -pub(super) fn to_u32(bytes: &[u8], start_index: usize) -> Result { - const ARRAY_SIZE: usize = to_usize(u32::BITS >> 3); - subarray::(bytes, start_index).map(u32::from_le_bytes) -} - -pub(super) fn to_u64(bytes: &[u8], start_index: usize) -> Result { - const ARRAY_SIZE: usize = to_usize(u64::BITS >> 3); - subarray::(bytes, start_index).map(u64::from_le_bytes) -} - -fn subarray( - bytes: &[u8], - start_index: usize, -) -> Result<[u8; SIZE], ArchiveParsingError> { - let stop_index = start_index + SIZE; - - let bytes = bytes - .get(start_index..stop_index) - .ok_or_else(|| slice_too_small(bytes, stop_index))?; - - <[u8; SIZE]>::try_from(bytes).map_err(|_e| slice_too_small(bytes, SIZE)) -} - -#[expect( - clippy::as_conversions, - reason = "A compile-time assertion ensures that this conversion will be lossless on all relevant target platforms" -)] -pub(super) const fn to_usize(value: u32) -> usize { - // Error at compile time if this conversion isn't lossless. - const _: () = assert!(u32::BITS <= usize::BITS, "cannot fit a u32 into a usize!"); - value as usize -} - #[cfg(test)] mod tests { use super::*;