From 42894bf93b4685945bc3401f5e13811ab231facc Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 16:13:01 -0500 Subject: [PATCH] format: v2 B-trees, dense groups and group listings over Storage BTreeV2Header::parse_in, collect_btree_v2_records_in and find_btree_v2_records_in read one bounded window per node (its size is known from the parent before the node is read; a count stretched past node_size is checked against the end of the file first), with the whole-file bounds errors unchanged. With them, dense attributes, a SOHM B-tree index and huge fractal-heap objects no longer answer ContiguousStorageRequired, and group_v1/group_v2 listings, lookups and path resolution get *_in cores (resolve_group_children_in, resolve_child_in, resolve_path_any_in, ...). The &[u8] functions are thin wrappers, as in M1. The equivalence harness now fails on any ContiguousStorageRequired and compares v2 B-tree headers, records and descents, group listings, child lookups and paths; a unit test compares a two-level tree through a read_at-only storage truncated at every length and with every node byte flipped. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/addr.rs | 9 + crates/clawhdf5-format/src/attribute.rs | 55 ++-- crates/clawhdf5-format/src/btree_v2.rs | 306 ++++++++++++------ crates/clawhdf5-format/src/btree_v2_write.rs | 51 +++ crates/clawhdf5-format/src/fractal_heap.rs | 36 +-- crates/clawhdf5-format/src/group_v1.rs | 85 +++-- crates/clawhdf5-format/src/group_v2.rs | 138 +++++--- crates/clawhdf5-format/src/shared_message.rs | 23 +- .../tests/storage_equivalence.rs | 138 ++++---- 9 files changed, 544 insertions(+), 297 deletions(-) diff --git a/crates/clawhdf5-format/src/addr.rs b/crates/clawhdf5-format/src/addr.rs index 3198f1f..e6471d0 100644 --- a/crates/clawhdf5-format/src/addr.rs +++ b/crates/clawhdf5-format/src/addr.rs @@ -23,6 +23,15 @@ pub fn to_usize(value: u64) -> Result { to_index::(value) } +/// A file address for a [`crate::storage::Storage`] read, checked as +/// [`to_usize`] checks it: the parsers read through 64-bit offsets, but an +/// address that could not index an in-memory file on this platform is the +/// same [`FormatError::Overflow`] the slice parsers gave for it. +#[inline] +pub fn checked_addr(value: u64) -> Result { + to_usize(value).map(|_| value) +} + /// [`to_usize`] for an index type of any width. `usize` is 64 bits wide on /// the hosts CI tests on, where the error path cannot be reached through /// `usize`; tests run the same code with `u32` in its place, as on a 32-bit diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 78aaf12..4b30fac 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -7,7 +7,7 @@ use std::borrow::Cow; use crate::addr::to_usize; use crate::attribute_info::AttributeInfoMessage; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records, find_btree_v2_records}; +use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records_in, find_btree_v2_records_in}; use crate::checksum::jenkins_lookup3; use crate::data_read; use crate::dataspace::Dataspace; @@ -17,7 +17,7 @@ use crate::fractal_heap::FractalHeapHeader; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; use crate::shared_message; -use crate::storage::{Storage, require_contiguous}; +use crate::storage::Storage; use crate::vl_data; /// A parsed HDF5 attribute message. @@ -578,9 +578,12 @@ pub fn find_attribute_in( .find(|a| a.name == name), ); }; - let contiguous = require_contiguous(file_data, "dense attribute storage (a v2 B-tree)")?; - let btree_hdr = - BTreeV2Header::parse(contiguous, to_usize(btree_addr)?, offset_size, length_size)?; + let btree_hdr = BTreeV2Header::parse_in( + file_data, + to_usize(btree_addr)? as u64, + offset_size, + length_size, + )?; let fh = FractalHeapHeader::parse_in(file_data, fh_addr, offset_size, length_size)?; if btree_hdr.tree_type != ATTRIBUTE_NAME_INDEX || btree_hdr.record_size < 4 { return Ok( @@ -610,7 +613,7 @@ pub fn find_attribute_in( // hash is the last field. let hash = jenkins_lookup3(name.as_bytes()); let hash_at = usize::from(btree_hdr.record_size) - 4; - let records = find_btree_v2_records(contiguous, &btree_hdr, offset_size, &mut |r| match r + let records = find_btree_v2_records_in(file_data, &btree_hdr, offset_size, &mut |r| match r .get(hash_at..hash_at + 4) { Some(h) => u32::from_le_bytes([h[0], h[1], h[2], h[3]]).cmp(&hash), @@ -722,10 +725,13 @@ fn extract_dense_attributes( expected: 1, available: 0, })?; - let contiguous = require_contiguous(file_data, "dense attribute storage (a v2 B-tree)")?; - let btree_hdr = - BTreeV2Header::parse(contiguous, to_usize(btree_addr)?, offset_size, length_size)?; - let records = collect_btree_v2_records(contiguous, &btree_hdr, offset_size, length_size)?; + let btree_hdr = BTreeV2Header::parse_in( + file_data, + to_usize(btree_addr)? as u64, + offset_size, + length_size, + )?; + let records = collect_btree_v2_records_in(file_data, &btree_hdr, offset_size, length_size)?; for record in &records { // Per HDF5 spec, both type 8 and type 9 records start with heap_id: @@ -1156,11 +1162,9 @@ mod tests { } /// Every object's attributes in h5py-written files read identically - /// through a read_at-only CountingStorage — compact ones, shared ones - /// and those behind an Attribute Info message — except dense storage, - /// whose v2 B-tree index is not read over Storage yet: that is the clean - /// ContiguousStorageRequired error, never a partial list. Through a - /// slice as Storage every object matches. + /// through a read_at-only CountingStorage — compact ones, shared ones, + /// those behind an Attribute Info message and dense storage (its v2 + /// B-tree name index included) — and through a slice as Storage. #[test] fn storage_reads_match_slice_reads() { use crate::storage::CountingStorage; @@ -1206,18 +1210,17 @@ mod tests { .unwrap() .is_some_and(|i| i.fractal_heap_address.is_some()); if is_dense { - let e = FormatError::ContiguousStorageRequired( - "dense attribute storage (a v2 B-tree)", - ); - assert_eq!(got.unwrap_err(), e, "{name}"); - assert_eq!(got_t.unwrap_err(), e, "{name}"); dense += 1; - } else { - attrs += want.as_ref().map_or(0, Vec::len); - assert_eq!(format!("{got:?}"), format!("{want:?}"), "{name}"); - let want_t = extract_attributes_tolerant(file, &header, os, ls); - assert_eq!(format!("{got_t:?}"), format!("{want_t:?}"), "{name}"); - same += 1; + } + attrs += want.as_ref().map_or(0, Vec::len); + assert_eq!(format!("{got:?}"), format!("{want:?}"), "{name}"); + let want_t = extract_attributes_tolerant(file, &header, os, ls); + assert_eq!(format!("{got_t:?}"), format!("{want_t:?}"), "{name}"); + same += 1; + for a in want.iter().flatten() { + let one = find_attribute_in(&storage, &header, &a.name, os, ls); + let want_one = find_attribute_in_file(file, &header, &a.name, os, ls); + assert_eq!(format!("{one:?}"), format!("{want_one:?}"), "{name}"); } } } diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index d3f6250..5641eb7 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -9,6 +9,7 @@ use byteorder::{ByteOrder, LittleEndian}; use crate::addr::to_usize; use crate::error::FormatError; +use crate::storage::{Storage, Window, len_usize}; /// Parsed B-tree v2 header (signature "BTHD"). #[derive(Debug, Clone)] @@ -99,38 +100,52 @@ impl BTreeV2Header { offset_size: u8, length_size: u8, ) -> Result { - ensure_len(file_data, offset, 4)?; - if &file_data[offset..offset + 4] != b"BTHD" { + Self::parse_in(file_data, offset as u64, offset_size, length_size) + } + + /// [`Self::parse`] over any [`Storage`]: one bounded read of the + /// header. + pub fn parse_in( + file: &S, + offset: u64, + offset_size: u8, + length_size: u8, + ) -> Result { + // Every field and the checksum; the window holds all of it or ends + // at the end of the file, so its bounds checks are the whole-file + // ones. + let full = 16 + usize::from(offset_size) + 2 + usize::from(length_size) + 4; + let w = Window::read(file, offset, full)?; + let d = &w.bytes; + w.ensure(0, 4)?; + if &d[..4] != b"BTHD" { return Err(FormatError::InvalidBTreeV2Signature); } - ensure_len(file_data, offset, 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1)?; - let version = file_data[offset + 4]; + w.ensure(0, 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1)?; + let version = d[4]; if version != 0 { return Err(FormatError::InvalidBTreeV2Version(version)); } - let tree_type = file_data[offset + 5]; - let node_size = u32::from_le_bytes([ - file_data[offset + 6], - file_data[offset + 7], - file_data[offset + 8], - file_data[offset + 9], - ]); - let record_size = u16::from_le_bytes([file_data[offset + 10], file_data[offset + 11]]); - let depth = u16::from_le_bytes([file_data[offset + 12], file_data[offset + 13]]); - let _split_percent = file_data[offset + 14]; - let _merge_percent = file_data[offset + 15]; + let tree_type = d[5]; + let node_size = u32::from_le_bytes([d[6], d[7], d[8], d[9]]); + let record_size = u16::from_le_bytes([d[10], d[11]]); + let depth = u16::from_le_bytes([d[12], d[13]]); + let _split_percent = d[14]; + let _merge_percent = d[15]; - let mut pos = offset + 16; - let root_node_address = read_offset(file_data, pos, offset_size)?; + let mut pos = 16; + w.ensure(pos, usize::from(offset_size))?; + let root_node_address = read_offset(d, pos, offset_size)?; pos += offset_size as usize; - ensure_len(file_data, pos, 2)?; - let num_records_in_root = u16::from_le_bytes([file_data[pos], file_data[pos + 1]]); + w.ensure(pos, 2)?; + let num_records_in_root = u16::from_le_bytes([d[pos], d[pos + 1]]); pos += 2; - let total_records = read_offset(file_data, pos, length_size)?; + w.ensure(pos, usize::from(length_size))?; + let total_records = read_offset(d, pos, length_size)?; #[allow(unused_assignments)] { pos += length_size as usize; @@ -139,9 +154,9 @@ impl BTreeV2Header { // Validate header checksum #[cfg(feature = "checksum")] { - ensure_len(file_data, pos, 4)?; - let stored = LittleEndian::read_u32(&file_data[pos..pos + 4]); - let computed = crate::checksum::jenkins_lookup3(&file_data[offset..pos]); + w.ensure(pos, 4)?; + let stored = LittleEndian::read_u32(&d[pos..pos + 4]); + let computed = crate::checksum::jenkins_lookup3(&d[..pos]); if computed != stored { return Err(FormatError::ChecksumMismatch { expected: stored, @@ -191,6 +206,17 @@ pub fn collect_btree_v2_records( header: &BTreeV2Header, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + collect_btree_v2_records_in(file_data, header, offset_size, length_size) +} + +/// [`collect_btree_v2_records`] over any [`Storage`]: one bounded read per +/// node. +pub fn collect_btree_v2_records_in( + file: &S, + header: &BTreeV2Header, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { if header.total_records == 0 || header.num_records_in_root == 0 { return Ok(Vec::new()); @@ -210,23 +236,24 @@ pub fn collect_btree_v2_records( // millions of records from a few kilobytes. Counting against what the // file could physically contain bounds that without trusting the // header's own `total_records`. - let mut budget = file_data.len() / usize::from(header.record_size.max(1)); + let mut budget = len_usize(file) / usize::from(header.record_size.max(1)); let max_leaf_nrec = max_records_leaf(header.node_size, header.record_size); if header.depth == 0 { // Root is a leaf parse_leaf_records( - file_data, + file, to_usize(header.root_node_address)?, header.num_records_in_root, header.record_size, + header.node_size, ) } else { // Root is internal; traverse recursively let mut records = Vec::new(); collect_internal_records( - file_data, + file, to_usize(header.root_node_address)?, header.num_records_in_root, header.depth, @@ -242,36 +269,72 @@ pub fn collect_btree_v2_records( } } +/// A node's bytes: `want` bytes at `offset` (fewer only at the end of the +/// file), after checking its 4-byte signature. A node is read in one piece +/// when it fits in `node_size` (every valid node does); a larger claimed +/// extent — record counts from a damaged parent — is first checked against +/// the end of the file, so it costs a read only of bytes the file has. +/// Bounds errors are the whole-file ones: the signature check needs the +/// first 6 bytes, then `checks` — `(position, length)` pairs relative to +/// the node, in the order the parser checks them — must lie in the file. +fn read_node<'a, S: Storage + ?Sized>( + file: &'a S, + offset: usize, + want: usize, + node_size: u32, + signature: &[u8; 4], + checks: &[(usize, usize)], +) -> Result, FormatError> { + let one_read = usize::try_from(node_size).unwrap_or(usize::MAX).max(6); + let w = Window::read(file, offset as u64, want.min(one_read))?; + w.ensure(0, 6)?; + if &w.bytes[..4] != signature { + return Err(FormatError::InvalidBTreeV2Signature); + } + if want <= one_read { + return Ok(w); + } + for &(rel, len) in checks { + Window::check_extent(file, offset as u64, rel, len)?; + } + Window::read(file, offset as u64, want) +} + /// Parse records from a leaf node (signature "BTLF"). -fn parse_leaf_records( - file_data: &[u8], +fn parse_leaf_records( + file: &S, offset: usize, num_records: u16, record_size: u16, + node_size: u32, ) -> Result, FormatError> { // signature(4) + version(1) + type(1) = 6 bytes header - ensure_len(file_data, offset, 6)?; - if &file_data[offset..offset + 4] != b"BTLF" { - return Err(FormatError::InvalidBTreeV2Signature); - } - - let pos = offset + 6; + let pos = 6; let rs = record_size as usize; let total = (num_records as usize) .checked_mul(rs) .ok_or(FormatError::UnexpectedEof { expected: usize::MAX, - available: file_data.len(), + available: len_usize(file), })?; - ensure_len(file_data, pos, total)?; + let w = read_node( + file, + offset, + pos + total + 4, + node_size, + b"BTLF", + &[(pos, total)], + )?; + let d = &w.bytes; + w.ensure(pos, total)?; // Validate checksum: 4 bytes after records + padding #[cfg(feature = "checksum")] { let checksum_pos = pos + total; - if file_data.len() >= checksum_pos + 4 { - let stored = LittleEndian::read_u32(&file_data[checksum_pos..checksum_pos + 4]); - let computed = crate::checksum::jenkins_lookup3(&file_data[offset..checksum_pos]); + if d.len() >= checksum_pos + 4 { + let stored = LittleEndian::read_u32(&d[checksum_pos..checksum_pos + 4]); + let computed = crate::checksum::jenkins_lookup3(&d[..checksum_pos]); if computed != stored { return Err(FormatError::ChecksumMismatch { expected: stored, @@ -285,17 +348,41 @@ fn parse_leaf_records( for i in 0..num_records as usize { let start = pos + i * rs; records.push(BTreeV2Record { - data: file_data[start..start + rs].to_vec(), + data: d[start..start + rs].to_vec(), }); } Ok(records) } +/// An internal node read from the file: its bytes (from the signature on), +/// where its records start, and its children as `(address, record count)`. +struct InternalNode<'a> { + node: Window<'a>, + records_start: usize, + children: Vec<(u64, u16)>, +} + +impl InternalNode<'_> { + /// Record `i`, `rs` bytes long. + fn record(&self, i: usize, rs: usize) -> Result<&[u8], FormatError> { + let overflow = || FormatError::UnexpectedEof { + expected: usize::MAX, + available: usize::MAX, + }; + let rec_start = i + .checked_mul(rs) + .and_then(|o| self.records_start.checked_add(o)) + .ok_or_else(overflow)?; + self.node.ensure(rec_start, rs)?; + Ok(&self.node.bytes[rec_start..rec_start + rs]) + } +} + /// An internal node's layout: where its records start, and its children as /// `(address, record count)`. #[allow(clippy::too_many_arguments)] -fn read_internal_node( - file_data: &[u8], +fn read_internal_node( + file: &S, offset: usize, num_records: u16, depth: u16, @@ -303,25 +390,15 @@ fn read_internal_node( node_size: u32, offset_size: u8, max_leaf_nrec: u64, -) -> Result<(usize, Vec<(u64, u16)>), FormatError> { - // signature(4) + version(1) + type(1) = 6 - ensure_len(file_data, offset, 6)?; - if &file_data[offset..offset + 4] != b"BTIN" { - return Err(FormatError::InvalidBTreeV2Signature); - } - +) -> Result, FormatError> { let nr = num_records as usize; let rs = record_size as usize; - let mut pos = offset + 6; // Records first let records_total = nr.checked_mul(rs).ok_or(FormatError::UnexpectedEof { expected: usize::MAX, - available: file_data.len(), + available: len_usize(file), })?; - ensure_len(file_data, pos, records_total)?; - let records_start = pos; - pos += records_total; // Child pointer layout, as libhdf5 computes it (H5B2__hdr_init): the // child's record count is always encoded in the width needed for a @@ -344,13 +421,30 @@ fn read_internal_node( let num_children = nr + 1; let child_ptr_size = offset_size as usize + nrec_width + total_nrec_width; - ensure_len(file_data, pos, num_children * child_ptr_size)?; + let pointers = num_children * child_ptr_size; + + // signature(4) + version(1) + type(1) = 6, records, pointers, checksum. + let w = read_node( + file, + offset, + 6 + records_total + pointers + 4, + node_size, + b"BTIN", + &[(6, records_total), (6 + records_total, pointers)], + )?; + let d = &w.bytes; + let mut pos = 6; + w.ensure(pos, records_total)?; + let records_start = pos; + pos += records_total; + + w.ensure(pos, pointers)?; let mut children = Vec::with_capacity(num_children); for _ in 0..num_children { - let addr = read_offset(file_data, pos, offset_size)?; + let addr = read_offset(d, pos, offset_size)?; pos += offset_size as usize; - let child_nrec = read_var_uint(file_data, pos, nrec_width)? as u16; + let child_nrec = read_var_uint(d, pos, nrec_width)? as u16; pos += nrec_width; pos += total_nrec_width; // skip total records in subtree children.push((addr, child_nrec)); @@ -362,9 +456,9 @@ fn read_internal_node( // a mismatch here, and so does this. #[cfg(feature = "checksum")] { - ensure_len(file_data, pos, 4)?; - let stored = LittleEndian::read_u32(&file_data[pos..pos + 4]); - let computed = crate::checksum::jenkins_lookup3(&file_data[offset..pos]); + w.ensure(pos, 4)?; + let stored = LittleEndian::read_u32(&d[pos..pos + 4]); + let computed = crate::checksum::jenkins_lookup3(&d[..pos]); if computed != stored { return Err(FormatError::ChecksumMismatch { expected: stored, @@ -372,37 +466,17 @@ fn read_internal_node( }); } } - Ok((records_start, children)) -} - -/// Record `i` of an internal node whose records start at `records_start`. -fn internal_record( - file_data: &[u8], - records_start: usize, - i: usize, - rs: usize, -) -> Result<&[u8], FormatError> { - let overflow = || FormatError::UnexpectedEof { - expected: usize::MAX, - available: file_data.len(), - }; - let rec_start = i - .checked_mul(rs) - .and_then(|o| records_start.checked_add(o)) - .ok_or_else(overflow)?; - let rec_end = rec_start.checked_add(rs).ok_or_else(overflow)?; - file_data - .get(rec_start..rec_end) - .ok_or(FormatError::UnexpectedEof { - expected: rec_end, - available: file_data.len(), - }) + Ok(InternalNode { + node: w, + records_start, + children, + }) } /// Recursively collect records from an internal node. #[allow(clippy::too_many_arguments, clippy::only_used_in_recursion)] -fn collect_internal_records( - file_data: &[u8], +fn collect_internal_records( + file: &S, offset: usize, num_records: u16, depth: u16, @@ -416,8 +490,8 @@ fn collect_internal_records( ) -> Result<(), FormatError> { let nr = num_records as usize; let rs = record_size as usize; - let (records_start, children) = read_internal_node( - file_data, + let node = read_internal_node( + file, offset, num_records, depth, @@ -430,16 +504,21 @@ fn collect_internal_records( // Interleave: child[0], record[0], child[1], record[1], ..., child[nr] // We collect child[0] records, then record[0], then child[1], etc. - for (i, &(child_addr, child_nrec)) in children.iter().enumerate() { + for (i, &(child_addr, child_nrec)) in node.children.iter().enumerate() { if child_depth == 0 { // Before parsing, so a refused tree is not also a large allocation. spend(budget, usize::from(child_nrec))?; - let leaf_recs = - parse_leaf_records(file_data, to_usize(child_addr)?, child_nrec, record_size)?; + let leaf_recs = parse_leaf_records( + file, + to_usize(child_addr)?, + child_nrec, + record_size, + node_size, + )?; out.extend(leaf_recs); } else { collect_internal_records( - file_data, + file, to_usize(child_addr)?, child_nrec, child_depth, @@ -455,7 +534,7 @@ fn collect_internal_records( // Add record[i] (except after the last child) if i < nr { - let data = internal_record(file_data, records_start, i, rs)?; + let data = node.record(i, rs)?; spend(budget, 1)?; out.push(BTreeV2Record { data: data.to_vec(), @@ -481,6 +560,17 @@ pub fn find_btree_v2_records( header: &BTreeV2Header, offset_size: u8, cmp: &mut dyn FnMut(&[u8]) -> Ordering, +) -> Result, FormatError> { + find_btree_v2_records_in(file_data, header, offset_size, cmp) +} + +/// [`find_btree_v2_records`] over any [`Storage`]: one bounded read per +/// node visited. +pub fn find_btree_v2_records_in( + file: &S, + header: &BTreeV2Header, + offset_size: u8, + cmp: &mut dyn FnMut(&[u8]) -> Ordering, ) -> Result, FormatError> { if header.total_records == 0 || header.num_records_in_root == 0 { return Ok(Vec::new()); @@ -490,11 +580,11 @@ pub fn find_btree_v2_records( } // As in `collect_btree_v2_records`: a valid tree cannot hold more // records than the file has room for, however its children are shared. - let mut budget = file_data.len() / usize::from(header.record_size.max(1)); + let mut budget = len_usize(file) / usize::from(header.record_size.max(1)); let max_leaf_nrec = max_records_leaf(header.node_size, header.record_size); let mut out = Vec::new(); find_in_node( - file_data, + file, header, to_usize(header.root_node_address)?, header.num_records_in_root, @@ -509,8 +599,8 @@ pub fn find_btree_v2_records( } #[allow(clippy::too_many_arguments)] -fn find_in_node( - file_data: &[u8], +fn find_in_node( + file: &S, header: &BTreeV2Header, offset: usize, num_records: u16, @@ -523,7 +613,13 @@ fn find_in_node( ) -> Result<(), FormatError> { spend(budget, usize::from(num_records))?; if depth == 0 { - let records = parse_leaf_records(file_data, offset, num_records, header.record_size)?; + let records = parse_leaf_records( + file, + offset, + num_records, + header.record_size, + header.node_size, + )?; out.extend( records .into_iter() @@ -532,8 +628,8 @@ fn find_in_node( return Ok(()); } let rs = usize::from(header.record_size); - let (records_start, children) = read_internal_node( - file_data, + let node = read_internal_node( + file, offset, num_records, depth, @@ -545,17 +641,17 @@ fn find_in_node( let nr = usize::from(num_records); let mut order = Vec::with_capacity(nr); for i in 0..nr { - order.push(cmp(internal_record(file_data, records_start, i, rs)?)); + order.push(cmp(node.record(i, rs)?)); } // Child `i` holds the keys between record `i - 1` and record `i`: it can // hold a match unless the record before it is already past the range or // the record after it is still before it. - for (i, &(child_addr, child_nrec)) in children.iter().enumerate() { + for (i, &(child_addr, child_nrec)) in node.children.iter().enumerate() { let after_left = i == 0 || order[i - 1] != Ordering::Greater; let before_right = i == nr || order[i] != Ordering::Less; if after_left && before_right { find_in_node( - file_data, + file, header, to_usize(child_addr)?, child_nrec, @@ -569,7 +665,7 @@ fn find_in_node( } if i < nr && order[i] == Ordering::Equal { out.push(BTreeV2Record { - data: internal_record(file_data, records_start, i, rs)?.to_vec(), + data: node.record(i, rs)?.to_vec(), }); } } diff --git a/crates/clawhdf5-format/src/btree_v2_write.rs b/crates/clawhdf5-format/src/btree_v2_write.rs index 38fa7c1..dd8e9cf 100644 --- a/crates/clawhdf5-format/src/btree_v2_write.rs +++ b/crates/clawhdf5-format/src/btree_v2_write.rs @@ -441,6 +441,57 @@ mod tests { } } + /// A two-level tree read through a `read_at`-only storage gives what + /// the slice gives — records, descents and errors — whole, truncated + /// at every length, and with each byte of its nodes flipped, and each + /// node costs one read. + #[test] + fn storage_reads_match_slice_reads() { + use crate::btree_v2::{ + collect_btree_v2_records_in, find_btree_v2_records, find_btree_v2_records_in, + }; + use crate::storage::CountingStorage; + let (rs, n, base) = (11usize, 120usize, 64usize); + let recs = records(n, rs); + let tree = build_btree_v2(params(128, 11), &recs, base as u64, 8, 8).unwrap(); + let mut whole = vec![0u8; base]; + whole.extend_from_slice(&tree); + let hdr = BTreeV2Header::parse(&whole, base, 8, 8).unwrap(); + assert!(hdr.depth >= 1, "{hdr:?}"); + let key = |r: &[u8]| u64::from_be_bytes(r[..8].try_into().unwrap()); + let mut files = Vec::new(); + for cut in base..=whole.len() { + files.push(whole[..cut].to_vec()); + } + for at in base..whole.len() { + let mut bad = whole.clone(); + bad[at] ^= 0x5a; + files.push(bad); + } + let mut ok = 0; + for f in &files { + let st = CountingStorage::new(f.clone()); + let want_h = BTreeV2Header::parse(f, base, 8, 8); + let got_h = BTreeV2Header::parse_in(&st, base as u64, 8, 8); + assert_eq!(format!("{got_h:?}"), format!("{want_h:?}")); + // The nodes of the intact header, over each damaged file. + let want = collect_btree_v2_records(f, &hdr, 8, 8); + st.reset(); + let got = collect_btree_v2_records_in(&st, &hdr, 8, 8); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + if want.is_ok() { + ok += 1; + assert!(st.reads() <= 1 + n as u64 / 3, "{} reads", st.reads()); + } + for k in [0u64, 7, 60, 119, 500] { + let want = find_btree_v2_records(f, &hdr, 8, &mut |r: &[u8]| key(r).cmp(&k)); + let got = find_btree_v2_records_in(&st, &hdr, 8, &mut |r: &[u8]| key(r).cmp(&k)); + assert_eq!(format!("{got:?}"), format!("{want:?}")); + } + } + assert!(ok > 1); + } + #[test] fn a_node_too_small_or_too_big_is_an_error() { assert!(build_btree_v2(params(16, 11), &records(1, 11), 0, 8, 8).is_err()); diff --git a/crates/clawhdf5-format/src/fractal_heap.rs b/crates/clawhdf5-format/src/fractal_heap.rs index 7bdce6f..83cfd33 100644 --- a/crates/clawhdf5-format/src/fractal_heap.rs +++ b/crates/clawhdf5-format/src/fractal_heap.rs @@ -7,10 +7,10 @@ use alloc::{format, vec::Vec}; use byteorder::{ByteOrder, LittleEndian}; use crate::addr::to_usize; -use crate::btree_v2::{BTreeV2Header, find_btree_v2_records}; +use crate::btree_v2::{BTreeV2Header, find_btree_v2_records_in}; use crate::error::FormatError; use crate::filter_pipeline::FilterPipeline; -use crate::storage::{Storage, Window, len_usize, read_exact_at, require_contiguous}; +use crate::storage::{Storage, Window, len_usize, read_exact_at}; /// Parsed fractal heap header (signature "FRHP"). #[derive(Debug, Clone)] @@ -386,9 +386,7 @@ impl FractalHeapHeader { self.read_managed_object_in(file_data, id_bytes, offset_size) } - /// [`Self::read_managed_object`] over any [`Storage`]. A huge object - /// found through the huge-object v2 B-tree still needs the whole file - /// in memory ([`FormatError::ContiguousStorageRequired`] otherwise). + /// [`Self::read_managed_object`] over any [`Storage`]. pub fn read_managed_object_in( &self, file_data: &S, @@ -491,11 +489,9 @@ impl FractalHeapHeader { "huge object ID but the heap has no huge-object index", )); } - // The v2 B-tree is read from a slice until it is converted. - let file_data = require_contiguous(file, "a huge fractal-heap object's B-tree")?; - let hdr = BTreeV2Header::parse( - file_data, - to_usize(self.huge_btree_address)?, + let hdr = BTreeV2Header::parse_in( + file, + self.huge_btree_address, self.offset_size, self.length_size, )?; @@ -513,7 +509,7 @@ impl FractalHeapHeader { // Records are ordered by ID (the last field): descend to the ones // equal to `key` instead of reading the whole index. let id_at = rec_len - ls; - let records = find_btree_v2_records(file_data, &hdr, self.offset_size, &mut |r| { + let records = find_btree_v2_records_in(file, &hdr, self.offset_size, &mut |r| { le_uint(&r[id_at..id_at + ls]).cmp(&key) })?; for rec in &records { @@ -1317,22 +1313,20 @@ mod tests { assert_eq!(hdr.read_managed_object_in(&storage, &id, 8), want); } - /// A huge object found through the huge-object B-tree needs the whole - /// file in memory until the B-tree reader is converted: a clean error - /// on other storage. + /// A huge object found through the huge-object B-tree reads the + /// B-tree through Storage: the same result (here an error, there is no + /// B-tree at that address) as from the slice. #[test] - fn huge_object_btree_needs_contiguous_storage() { + fn huge_object_btree_reads_through_storage() { use crate::storage::CountingStorage; let (file, _) = build_simple_heap(8, 8); let mut hdr = FractalHeapHeader::parse(&file, 0, 8, 8).unwrap(); hdr.huge_btree_address = 700; + let id = [0x10, 1, 0, 0, 0, 0, 0]; + let want = hdr.read_managed_object(&file, &id, 8); + assert!(want.is_err()); let storage = CountingStorage::new(file); - assert_eq!( - hdr.read_managed_object_in(&storage, &[0x10, 1, 0, 0, 0, 0, 0], 8), - Err(FormatError::ContiguousStorageRequired( - "a huge fractal-heap object's B-tree" - )) - ); + assert_eq!(hdr.read_managed_object_in(&storage, &id, 8), want); } /// A header with an I/O filter pipeline (read in a second, longer diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index 3dff9cd..ebd4d32 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -3,12 +3,13 @@ #[cfg(not(feature = "std"))] use alloc::{string::String, vec::Vec}; -use crate::addr::to_usize; -use crate::btree_v1::collect_symbol_table_nodes; +use crate::addr::checked_addr; +use crate::btree_v1::collect_symbol_table_nodes_in; use crate::error::FormatError; use crate::local_heap::LocalHeap; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; +use crate::storage::Storage; use crate::symbol_table::{SymbolTableMessage, SymbolTableNode}; /// A resolved group entry (child name + object header address). @@ -36,6 +37,16 @@ pub fn resolve_v1_group_entries( sym_table_msg: &SymbolTableMessage, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + resolve_v1_group_entries_in(file_data, sym_table_msg, offset_size, length_size) +} + +/// [`resolve_v1_group_entries`] over any [`Storage`]. +pub fn resolve_v1_group_entries_in( + file_data: &S, + sym_table_msg: &SymbolTableMessage, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { let entries = v1_group_entries(file_data, sym_table_msg, offset_size, length_size)?; if entries.iter().any(|e| e.name.is_empty()) { @@ -46,22 +57,22 @@ pub fn resolve_v1_group_entries( /// Every entry of a v1 group, empty names included — for looking a name up, /// which never matches an empty name. -pub(crate) fn v1_group_entries( - file_data: &[u8], +pub(crate) fn v1_group_entries( + file_data: &S, sym_table_msg: &SymbolTableMessage, offset_size: u8, length_size: u8, ) -> Result, FormatError> { // Parse local heap - let heap = LocalHeap::parse( + let heap = LocalHeap::parse_in( file_data, - to_usize(sym_table_msg.local_heap_address)?, + checked_addr(sym_table_msg.local_heap_address)?, offset_size, length_size, )?; // Collect all SNOD addresses from B-tree - let snod_addrs = collect_symbol_table_nodes( + let snod_addrs = collect_symbol_table_nodes_in( file_data, sym_table_msg.btree_address, offset_size, @@ -71,15 +82,15 @@ pub(crate) fn v1_group_entries( let mut entries = Vec::new(); let mut heap_checked = false; for snod_addr in snod_addrs { - let snod = SymbolTableNode::parse(file_data, to_usize(snod_addr)?, offset_size)?; + let snod = SymbolTableNode::parse_in(file_data, checked_addr(snod_addr)?, offset_size)?; for entry in &snod.entries { // Like libhdf5, look at the heap's free list only once a name is // needed: an empty group with a damaged heap still lists. if !heap_checked { - heap.validate_free_list(file_data, length_size)?; + heap.validate_free_list_in(file_data, length_size)?; heap_checked = true; } - let name = heap.read_string(file_data, entry.link_name_offset)?; + let name = heap.read_string_in(file_data, entry.link_name_offset)?; entries.push(GroupEntry { name, object_header_address: entry.object_header_address, @@ -103,6 +114,17 @@ pub fn find_v1_soft_link( name: &str, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + find_v1_soft_link_in(file_data, sym_table_msg, name, offset_size, length_size) +} + +/// [`find_v1_soft_link`] over any [`Storage`]. +pub fn find_v1_soft_link_in( + file_data: &S, + sym_table_msg: &SymbolTableMessage, + name: &str, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { let mut found = None; for_each_v1_soft_link( @@ -125,6 +147,16 @@ pub fn v1_soft_links( sym_table_msg: &SymbolTableMessage, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + v1_soft_links_in(file_data, sym_table_msg, offset_size, length_size) +} + +/// [`v1_soft_links`] over any [`Storage`]. +pub fn v1_soft_links_in( + file_data: &S, + sym_table_msg: &SymbolTableMessage, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { let mut links = Vec::new(); for_each_v1_soft_link( @@ -143,21 +175,21 @@ pub fn v1_soft_links( /// Visit the soft links of a v1 group whose name passes `wanted`, with their /// target paths, until `visit` returns false. -fn for_each_v1_soft_link( - file_data: &[u8], +fn for_each_v1_soft_link( + file_data: &S, sym_table_msg: &SymbolTableMessage, offset_size: u8, length_size: u8, wanted: impl Fn(&str) -> bool, mut visit: impl FnMut(&str, String) -> bool, ) -> Result<(), FormatError> { - let heap = LocalHeap::parse( + let heap = LocalHeap::parse_in( file_data, - to_usize(sym_table_msg.local_heap_address)?, + checked_addr(sym_table_msg.local_heap_address)?, offset_size, length_size, )?; - let snod_addrs = collect_symbol_table_nodes( + let snod_addrs = collect_symbol_table_nodes_in( file_data, sym_table_msg.btree_address, offset_size, @@ -165,16 +197,16 @@ fn for_each_v1_soft_link( )?; let mut heap_checked = false; for snod_addr in snod_addrs { - let snod = SymbolTableNode::parse(file_data, to_usize(snod_addr)?, offset_size)?; + let snod = SymbolTableNode::parse_in(file_data, checked_addr(snod_addr)?, offset_size)?; for entry in &snod.entries { if entry.cache_type != CACHE_TYPE_SOFT_LINK { continue; } if !heap_checked { - heap.validate_free_list(file_data, length_size)?; + heap.validate_free_list_in(file_data, length_size)?; heap_checked = true; } - let name = heap.read_string(file_data, entry.link_name_offset)?; + let name = heap.read_string_in(file_data, entry.link_name_offset)?; if !wanted(&name) { continue; } @@ -184,7 +216,7 @@ fn for_each_v1_soft_link( entry.scratch_pad[2], entry.scratch_pad[3], ]); - let target = heap.read_string(file_data, u64::from(value_offset))?; + let target = heap.read_string_in(file_data, u64::from(value_offset))?; if !visit(&name, target) { return Ok(()); } @@ -222,6 +254,17 @@ pub fn resolve_path( path: &str, offset_size: u8, length_size: u8, +) -> Result { + resolve_path_in(file_data, root_sym_table, path, offset_size, length_size) +} + +/// [`resolve_path`] over any [`Storage`]. +pub fn resolve_path_in( + file_data: &S, + root_sym_table: &SymbolTableMessage, + path: &str, + offset_size: u8, + length_size: u8, ) -> Result { let components: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); if components.is_empty() { @@ -241,9 +284,9 @@ pub fn resolve_path( return Ok(entry.object_header_address); } // Not last — must be a group, parse its object header to get symbol table - let obj_header = ObjectHeader::parse( + let obj_header = ObjectHeader::parse_in( file_data, - to_usize(entry.object_header_address)?, + checked_addr(entry.object_header_address)?, offset_size, length_size, )?; diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index 0edf972..9dda843 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -11,8 +11,8 @@ use alloc::collections::BTreeSet; #[cfg(feature = "std")] use std::collections::BTreeSet; -use crate::addr::to_usize; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records, find_btree_v2_records}; +use crate::addr::checked_addr; +use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records_in, find_btree_v2_records_in}; use crate::checksum::jenkins_lookup3; use crate::error::FormatError; use crate::fractal_heap::FractalHeapHeader; @@ -21,6 +21,7 @@ use crate::link_info::LinkInfoMessage; use crate::link_message::{LinkMessage, LinkTarget}; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; +use crate::storage::Storage; use crate::superblock::Superblock; use crate::symbol_table::SymbolTableMessage; @@ -32,6 +33,16 @@ pub fn resolve_v2_group_entries( object_header: &ObjectHeader, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + resolve_v2_group_entries_in(file_data, object_header, offset_size, length_size) +} + +/// [`resolve_v2_group_entries`] over any [`Storage`]. +pub fn resolve_v2_group_entries_in( + file_data: &S, + object_header: &ObjectHeader, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { // Look for Link Info message to determine storage type let link_info = find_link_info(object_header, offset_size)?; @@ -91,8 +102,8 @@ fn resolve_compact_entries( } /// Visit every link in dense storage (fractal heap + B-tree v2 name index). -fn for_each_dense_link( - file_data: &[u8], +fn for_each_dense_link( + file_data: &S, link_info: &LinkInfoMessage, fh_addr: u64, offset_size: u8, @@ -100,15 +111,20 @@ fn for_each_dense_link( mut visit: impl FnMut(LinkMessage), ) -> Result<(), FormatError> { // Parse fractal heap - let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; + let fh = + FractalHeapHeader::parse_in(file_data, checked_addr(fh_addr)?, offset_size, length_size)?; // Parse B-tree v2 for name index let btree_addr = link_info .btree_name_index_address .ok_or_else(|| FormatError::PathNotFound(String::from("no B-tree v2 name index")))?; - let btree_hdr = - BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; - let records = collect_btree_v2_records(file_data, &btree_hdr, offset_size, length_size)?; + let btree_hdr = BTreeV2Header::parse_in( + file_data, + checked_addr(btree_addr)?, + offset_size, + length_size, + )?; + let records = collect_btree_v2_records_in(file_data, &btree_hdr, offset_size, length_size)?; for record in &records { // For type 5 (name index): hash(4) + heap_id(heap_id_length) @@ -125,7 +141,7 @@ fn for_each_dense_link( let id_bytes = &record.data[id_offset..id_offset + fh.heap_id_length as usize]; // Read managed object from fractal heap - let link_data = fh.read_managed_object(file_data, id_bytes, offset_size)?; + let link_data = fh.read_managed_object_in(file_data, id_bytes, offset_size)?; if let Some(link) = parse_link(&link_data, offset_size)? { visit(link); } @@ -134,8 +150,8 @@ fn for_each_dense_link( } /// Resolve entries from dense storage (fractal heap + B-tree v2). -fn resolve_dense_entries( - file_data: &[u8], +fn resolve_dense_entries( + file_data: &S, link_info: &LinkInfoMessage, fh_addr: u64, offset_size: u8, @@ -167,8 +183,8 @@ fn resolve_dense_entries( /// The soft link called `name` in a v1 (symbol table) group, if there is /// one. Hard links are what `resolve_group_entries` returns; this is /// consulted only when a path component isn't among them. -fn find_v1_symbolic_link( - file_data: &[u8], +fn find_v1_symbolic_link( + file_data: &S, object_header: &ObjectHeader, name: &str, offset_size: u8, @@ -182,7 +198,7 @@ fn find_v1_symbolic_link( return Ok(None); }; let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; - group_v1::find_v1_soft_link(file_data, &stm, name, offset_size, length_size) + group_v1::find_v1_soft_link_in(file_data, &stm, name, offset_size, length_size) .map(|target| target.map(|target_path| LinkTarget::Soft { target_path })) } @@ -199,8 +215,8 @@ const LINK_NAME_INDEX: u8 = 5; /// link. libhdf5 orders records with equal hashes by name; all of them are /// read and compared here, so that order does not matter. An index of /// another type is scanned in full. -fn links_named( - file_data: &[u8], +fn links_named( + file_data: &S, object_header: &ObjectHeader, name: &str, offset_size: u8, @@ -220,12 +236,17 @@ fn links_named( return Ok(found); }; - let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; + let fh = + FractalHeapHeader::parse_in(file_data, checked_addr(fh_addr)?, offset_size, length_size)?; let btree_addr = link_info .btree_name_index_address .ok_or_else(|| FormatError::PathNotFound(String::from("no B-tree v2 name index")))?; - let btree_hdr = - BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; + let btree_hdr = BTreeV2Header::parse_in( + file_data, + checked_addr(btree_addr)?, + offset_size, + length_size, + )?; if btree_hdr.tree_type != LINK_NAME_INDEX { for_each_dense_link( file_data, @@ -244,7 +265,7 @@ fn links_named( // Record: hash(4) + heap ID. let hash = jenkins_lookup3(name.as_bytes()); - let records = find_btree_v2_records(file_data, &btree_hdr, offset_size, &mut |r| { + let records = find_btree_v2_records_in(file_data, &btree_hdr, offset_size, &mut |r| { match r.get(..4) { Some(h) => u32::from_le_bytes([h[0], h[1], h[2], h[3]]).cmp(&hash), // Too short to hold a hash (a corrupt record size): never a match. @@ -256,7 +277,7 @@ fn links_named( let Some(id_bytes) = record.data.get(4..4 + id_len) else { continue; }; - let link_data = fh.read_managed_object(file_data, id_bytes, offset_size)?; + let link_data = fh.read_managed_object_in(file_data, id_bytes, offset_size)?; if let Some(link) = parse_link(&link_data, offset_size)? && link.name == name { @@ -278,8 +299,8 @@ fn links_named( /// and may land on another of several exact duplicates. The listing /// ([`resolve_group_children`]), [`resolve_child`] and path resolution all /// apply this rule, so they agree. -fn first_link_named( - file_data: &[u8], +fn first_link_named( + file_data: &S, object_header: &ObjectHeader, name: &str, offset_size: u8, @@ -296,8 +317,8 @@ fn first_link_named( /// the group with header `object_header`: a hard link (as `Hard`), else a /// soft or external link of that name, else `None`. Fails with /// `PathNotFound` if the object is not a group. -fn lookup_link( - file_data: &[u8], +fn lookup_link( + file_data: &S, object_header: &ObjectHeader, name: &str, offset_size: u8, @@ -346,13 +367,23 @@ pub fn resolve_child( superblock: &Superblock, group_address: u64, name: &str, +) -> Result { + resolve_child_in(file_data, superblock, group_address, name) +} + +/// [`resolve_child`] over any [`Storage`]. +pub fn resolve_child_in( + file_data: &S, + superblock: &Superblock, + group_address: u64, + name: &str, ) -> Result { let os = superblock.offset_size; let ls = superblock.length_size; let not_found = || FormatError::PathNotFound(String::from(name)); - let header = ObjectHeader::parse(file_data, to_usize(group_address)?, os, ls)?; + let header = ObjectHeader::parse_in(file_data, checked_addr(group_address)?, os, ls)?; if !is_v2_group(&header) || is_v1_group(&header) { - return resolve_group_children(file_data, superblock, group_address)? + return resolve_group_children_in(file_data, superblock, group_address)? .into_iter() .find(|e| e.name == name) .map(|e| e.object_header_address) @@ -365,7 +396,7 @@ pub fn resolve_child( object_header_address, }) => Ok(object_header_address), Some(LinkTarget::Soft { target_path }) => { - match resolve_path_from(file_data, superblock, group_address, &target_path) { + match resolve_path_from_in(file_data, superblock, group_address, &target_path) { // Left out of the listing: dangling, cyclic, or in another file. Err( FormatError::PathNotFound(_) @@ -421,6 +452,15 @@ pub fn resolve_path_any( file_data: &[u8], superblock: &Superblock, path: &str, +) -> Result { + resolve_path_any_in(file_data, superblock, path) +} + +/// [`resolve_path_any`] over any [`Storage`]. +pub fn resolve_path_any_in( + file_data: &S, + superblock: &Superblock, + path: &str, ) -> Result { resolve_path_following_links( file_data, @@ -439,6 +479,16 @@ pub fn resolve_path_from( superblock: &Superblock, group_address: u64, path: &str, +) -> Result { + resolve_path_from_in(file_data, superblock, group_address, path) +} + +/// [`resolve_path_from`] over any [`Storage`]. +pub fn resolve_path_from_in( + file_data: &S, + superblock: &Superblock, + group_address: u64, + path: &str, ) -> Result { let start = if path.starts_with('/') { superblock.root_group_address @@ -462,10 +512,19 @@ pub fn resolve_group_children( file_data: &[u8], superblock: &Superblock, group_address: u64, +) -> Result, FormatError> { + resolve_group_children_in(file_data, superblock, group_address) +} + +/// [`resolve_group_children`] over any [`Storage`]. +pub fn resolve_group_children_in( + file_data: &S, + superblock: &Superblock, + group_address: u64, ) -> Result, FormatError> { let os = superblock.offset_size; let ls = superblock.length_size; - let header = ObjectHeader::parse(file_data, to_usize(group_address)?, os, ls)?; + let header = ObjectHeader::parse_in(file_data, checked_addr(group_address)?, os, ls)?; let mut entries = Vec::new(); let mut soft = Vec::new(); @@ -476,9 +535,9 @@ pub fn resolve_group_children( .find(|m| m.msg_type == MessageType::SymbolTable) .ok_or_else(|| FormatError::PathNotFound(String::from("no symbol table message")))?; let stm = SymbolTableMessage::parse(&sym_msg.data, os)?; - let all = group_v1::resolve_v1_group_entries(file_data, &stm, os, ls)?; + let all = group_v1::resolve_v1_group_entries_in(file_data, &stm, os, ls)?; if all.iter().any(group_v1::is_v1_soft_link) { - soft = group_v1::v1_soft_links(file_data, &stm, os, ls)?; + soft = group_v1::v1_soft_links_in(file_data, &stm, os, ls)?; } entries.extend(all.into_iter().filter(|e| !group_v1::is_v1_soft_link(e))); } else if is_v2_group(&header) { @@ -515,7 +574,7 @@ pub fn resolve_group_children( } for (name, target) in soft { - match resolve_path_from(file_data, superblock, group_address, &target) { + match resolve_path_from_in(file_data, superblock, group_address, &target) { Ok(object_header_address) => entries.push(GroupEntry { name, object_header_address, @@ -538,8 +597,8 @@ pub fn resolve_group_children( const MAX_SOFT_LINK_DEPTH: u8 = 16; /// Walk `path` from the group at `start`, following soft links. -fn resolve_path_following_links( - file_data: &[u8], +fn resolve_path_following_links( + file_data: &S, superblock: &Superblock, start: u64, path: &str, @@ -557,7 +616,7 @@ fn resolve_path_following_links( let ls = superblock.length_size; let mut current_addr = start; - let mut current_header = ObjectHeader::parse(file_data, to_usize(start)?, os, ls)?; + let mut current_header = ObjectHeader::parse_in(file_data, checked_addr(start)?, os, ls)?; for (i, component) in components.iter().enumerate() { match lookup_link(file_data, ¤t_header, component, os, ls)? { @@ -568,7 +627,8 @@ fn resolve_path_following_links( return Ok(object_header_address); } current_addr = object_header_address; - current_header = ObjectHeader::parse(file_data, to_usize(current_addr)?, os, ls)?; + current_header = + ObjectHeader::parse_in(file_data, checked_addr(current_addr)?, os, ls)?; } found => { return match found { @@ -607,8 +667,8 @@ fn resolve_path_following_links( } /// Resolve group entries from an object header, auto-detecting v1 vs v2. -fn resolve_group_entries( - file_data: &[u8], +fn resolve_group_entries( + file_data: &S, object_header: &ObjectHeader, offset_size: u8, length_size: u8, @@ -625,7 +685,7 @@ fn resolve_group_entries( // skipped by the name comparison, as in libhdf5. group_v1::v1_group_entries(file_data, &stm, offset_size, length_size) } else if is_v2_group(object_header) { - resolve_v2_group_entries(file_data, object_header, offset_size, length_size) + resolve_v2_group_entries_in(file_data, object_header, offset_size, length_size) } else { Err(FormatError::PathNotFound(String::from( "object header is not a group", diff --git a/crates/clawhdf5-format/src/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 05ee649..5f0fc2e 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -23,12 +23,12 @@ use alloc::vec::Vec; #[cfg(feature = "std")] use std::borrow::Cow; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; +use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records_in}; use crate::error::FormatError; use crate::fractal_heap::FractalHeapHeader; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; -use crate::storage::{Storage, Window, read_exact_at, require_contiguous}; +use crate::storage::{Storage, Window, read_exact_at}; /// Fractal heap ID length for SOHM entries (fixed at 8 bytes). const FHEAP_ID_LEN: usize = 8; @@ -423,19 +423,15 @@ pub fn parse_sohm_btree_entries( parse_sohm_btree_entries_in(file_data, btree_addr as u64, offset_size, length_size) } -/// [`parse_sohm_btree_entries`] over any [`Storage`]. The v2 B-tree is not -/// read over [`Storage`] yet, so this needs the whole file in memory -/// ([`FormatError::ContiguousStorageRequired`] otherwise). +/// [`parse_sohm_btree_entries`] over any [`Storage`]. pub fn parse_sohm_btree_entries_in( file: &S, btree_addr: u64, offset_size: u8, length_size: u8, ) -> Result, FormatError> { - let file_data = require_contiguous(file, "a shared-message B-tree index")?; - let btree_addr = usize::try_from(btree_addr).unwrap_or(usize::MAX); - let header = BTreeV2Header::parse(file_data, btree_addr, offset_size, length_size)?; - let records = collect_btree_v2_records(file_data, &header, offset_size, length_size)?; + let header = BTreeV2Header::parse_in(file, btree_addr, offset_size, length_size)?; + let records = collect_btree_v2_records_in(file, &header, offset_size, length_size)?; let mut entries = Vec::with_capacity(records.len()); for rec in &records { let entry = parse_sohm_entry(&rec.data, offset_size)?; @@ -1243,11 +1239,12 @@ mod tests { } } assert!(compared > 200); - // The B-tree index is not read over Storage yet: a clean error. - let st = CountingStorage::new(vec![0u8; 64]); + // The B-tree index reads through Storage too, errors included. + let junk = vec![0u8; 64]; + let st = CountingStorage::new(junk.clone()); assert_eq!( - parse_sohm_btree_entries_in(&st, 0, 8, 8).unwrap_err(), - FormatError::ContiguousStorageRequired("a shared-message B-tree index") + format!("{:?}", parse_sohm_btree_entries_in(&st, 0, 8, 8)), + format!("{:?}", parse_sohm_btree_entries(&junk, 0, 8, 8)) ); } } diff --git a/crates/clawhdf5-format/tests/storage_equivalence.rs b/crates/clawhdf5-format/tests/storage_equivalence.rs index 5cc71b9..02a4f17 100644 --- a/crates/clawhdf5-format/tests/storage_equivalence.rs +++ b/crates/clawhdf5-format/tests/storage_equivalence.rs @@ -1,24 +1,19 @@ //! Equivalence harness for the range-read migration -//! (`docs/design/range-reads.md`, milestone M1). +//! (`docs/design/range-reads.md`, milestones M1 and M2). //! -//! Every metadata parser converted to [`Storage`] must give exactly what its -//! `&[u8]` form gives. This walks real files — the fixtures, files h5py -//! writes to exercise the less common structures, and optionally the -//! conformance corpus — and, for every object, runs each converted parser -//! twice: over the file as a slice, and over a [`CountingStorage`] that -//! serves the same bytes through `read_at` only (`as_contiguous()` is -//! `None`, so no parser can fall back to the whole slice). The results must -//! be identical, value for value and error for error. +//! Every parser converted to [`Storage`] must give exactly what its `&[u8]` +//! form gives. This walks real files — the fixtures, files h5py writes to +//! exercise the less common structures, and optionally the conformance +//! corpus — and, for every object, runs each converted parser twice: over +//! the file as a slice, and over a [`CountingStorage`] that serves the same +//! bytes through `read_at` only (`as_contiguous()` is `None`, so no parser +//! can fall back to the whole slice). The results must be identical, value +//! for value and error for error. //! -//! The one allowed difference is [`FormatError::ContiguousStorageRequired`] -//! from the storage path, and only from the structures still indexed by a v2 -//! B-tree (dense attributes, a SOHM B-tree index, huge fractal-heap objects; -//! see `CONTIGUOUS_REQUIRED`), which fail cleanly instead of reading the -//! whole file. Those are counted; the error from any other site or check -//! fails the harness. -//! -//! Milestones M2/M3 extend `check_object` with the raw-data and group -//! parsers as they are converted. +//! Nothing may answer [`FormatError::ContiguousStorageRequired`] any more: +//! since milestone M2 every read path works over `read_at` alone, the v2 +//! B-tree structures (dense groups and attributes, a SOHM B-tree index, +//! huge fractal-heap objects) included. //! //! - `CLAWHDF5_STORAGE_CORPUS=dir[:dir...]` adds every `.h5`/`.hdf5`/`.he5`/ //! `.nc`/`.h5ad` file under those directories (the conformance corpus is @@ -38,7 +33,10 @@ use clawhdf5_format::attribute::{ }; use clawhdf5_format::attribute_info::AttributeInfoMessage; use clawhdf5_format::btree_v1::{collect_symbol_table_nodes, collect_symbol_table_nodes_in}; -use clawhdf5_format::btree_v2::{BTreeV2Header, collect_btree_v2_records}; +use clawhdf5_format::btree_v2::{ + BTreeV2Header, collect_btree_v2_records, collect_btree_v2_records_in, find_btree_v2_records, + find_btree_v2_records_in, +}; use clawhdf5_format::data_layout::DataLayout; use clawhdf5_format::dataspace::Dataspace; use clawhdf5_format::datatype::Datatype; @@ -51,6 +49,7 @@ use clawhdf5_format::fixed_array::{ FixedArrayHeader, read_fixed_array_chunks, read_fixed_array_chunks_in, }; use clawhdf5_format::fractal_heap::FractalHeapHeader; +use clawhdf5_format::group_v2; use clawhdf5_format::link_info::LinkInfoMessage; use clawhdf5_format::local_heap::LocalHeap; use clawhdf5_format::message_type::MessageType; @@ -73,44 +72,11 @@ use clawhdf5_format::symbol_table::{SymbolTableMessage, SymbolTableNode}; const MAX_OBJECTS: usize = 1500; const MAX_HEAP_IDS: usize = 200; -/// The structures that still need the whole file in memory, because they -/// are found through a version-2 B-tree (not converted yet), and the checks -/// that can reach each of them. Anything else answering -/// [`FormatError::ContiguousStorageRequired`] is a converted parser falling -/// back to the whole file, and fails the harness. -const CONTIGUOUS_REQUIRED: &[(&str, &[&str])] = &[ - ( - "dense attribute storage (a v2 B-tree)", - &["attributes", "attributes (tolerant)"], - ), - ( - "a shared-message B-tree index", - &[ - "SOHM B-tree", - "shared message", - "fill value", - "attributes", - "attributes (tolerant)", - ], - ), - ( - "a huge fractal-heap object's B-tree", - &["heap object", "attributes", "attributes (tolerant)"], - ), -]; - -fn may_require_contiguous(check: &str, site: &str) -> bool { - CONTIGUOUS_REQUIRED - .iter() - .any(|(s, checks)| *s == site && checks.contains(&check)) -} - #[derive(Default, Debug)] struct Tally { files: usize, objects: usize, checks: usize, - contiguous_required: usize, reads: u64, bytes: u64, /// Chunk indexes (fixed and extensible arrays) read, and the most bytes @@ -127,8 +93,8 @@ struct Walk<'a> { } impl Walk<'_> { - /// The storage result must equal the slice result, or be the clean - /// "needs the whole file" error. + /// The storage result must equal the slice result; no parser may ask + /// for the whole file. fn same( &mut self, what: &str, @@ -137,14 +103,11 @@ impl Walk<'_> { ) { self.tally.checks += 1; if let Err(FormatError::ContiguousStorageRequired(site)) = got { - assert!( - may_require_contiguous(what, site), - "{}: {what} fell back to the whole file ({site}), which only the \ - v2-B-tree-indexed structures may do", + panic!( + "{}: {what} fell back to the whole file ({site}); every read path \ + must work through read_at", self.name ); - self.tally.contiguous_required += 1; - return; } let (w, g) = (format!("{want:?}"), format!("{got:?}")); assert!( @@ -206,13 +169,34 @@ impl Walk<'_> { } self.tally.objects += 1; self.check_object(&sb, addr); - // Traversal only (group lookups are milestone M0/M3 work). - if let Ok(children) = - clawhdf5_format::group_v2::resolve_group_children(slice, &sb, addr) - { + let children = group_v2::resolve_group_children(slice, &sb, addr); + let got = group_v2::resolve_group_children_in(self.st(), &sb, addr); + self.same("group listing", &children, &got); + if let Ok(children) = children { + for c in children.iter().take(MAX_HEAP_IDS) { + let want = group_v2::resolve_child(slice, &sb, addr, &c.name); + let got = group_v2::resolve_child_in(self.st(), &sb, addr, &c.name); + self.same("child lookup", &want, &got); + } + // A name no group has: the lookup's not-found path. + let want = group_v2::resolve_child(slice, &sb, addr, "no such child"); + let got = group_v2::resolve_child_in(self.st(), &sb, addr, "no such child"); + self.same("child lookup (missing)", &want, &got); queue.extend(children.iter().map(|c| c.object_header_address)); } } + // Paths: every listed name from the root, and one that is missing. + if let Ok(children) = group_v2::resolve_group_children(slice, &sb, sb.root_group_address) { + for c in children.iter().take(MAX_HEAP_IDS) { + let path = format!("/{}", c.name); + let want = group_v2::resolve_path_any(slice, &sb, &path); + let got = group_v2::resolve_path_any_in(self.st(), &sb, &path); + self.same("path", &want, &got); + } + } + let want = group_v2::resolve_path_any(slice, &sb, "/no/such/path"); + let got = group_v2::resolve_path_any_in(self.st(), &sb, "/no/such/path"); + self.same("path (missing)", &want, &got); } fn check_object(&mut self, sb: &Superblock, addr: u64) { @@ -336,12 +320,24 @@ impl Walk<'_> { let (Ok(fh), Some(index)) = (fh, index) else { return; }; - let Ok(bt) = BTreeV2Header::parse(slice, index as usize, os, ls) else { - return; - }; - let Ok(records) = collect_btree_v2_records(slice, &bt, os, ls) else { - return; - }; + let bt = BTreeV2Header::parse(slice, index as usize, os, ls); + self.same( + "v2 B-tree header", + &bt, + &BTreeV2Header::parse_in(self.st(), index, os, ls), + ); + let Ok(bt) = bt else { return }; + let records = collect_btree_v2_records(slice, &bt, os, ls); + let got = collect_btree_v2_records_in(self.st(), &bt, os, ls); + self.same("v2 B-tree records", &records, &got); + let Ok(records) = records else { return }; + // Descents to single records (by their bytes), as name lookups do. + for rec in records.iter().take(8) { + let key = rec.data.clone(); + let want = find_btree_v2_records(slice, &bt, os, &mut |r| r.cmp(&key[..])); + let got = find_btree_v2_records_in(self.st(), &bt, os, &mut |r| r.cmp(&key[..])); + self.same("v2 B-tree descent", &want, &got); + } let id_len = fh.heap_id_length as usize; for rec in records.iter().take(MAX_HEAP_IDS) { let Some(id) = rec.data.get(id_at..id_at + id_len) else { @@ -696,6 +692,4 @@ fn h5py_files_parse_identically_through_storage() { } eprintln!("h5py files: {tally:?}"); assert!(tally.objects >= 700, "{tally:?}"); - // Dense attributes and the SOHM B-tree are the known clean errors. - assert!(tally.contiguous_required > 0, "{tally:?}"); }