From 74fdf0582b55ebd2daeed053c1c7a74541070bfb Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:09:31 -0500 Subject: [PATCH] fix(format): write paged files libhdf5 can open FileWriter::with_page_size wrote a "version 4" superblock with an extra page-size field. HDF5 has no superblock version 4, so libhdf5 refused every such file ("bad superblock version number"). A paged file is now what libhdf5 itself writes for fs_strategy="page": a v3 superblock whose extension object header holds a File Space Info message (strategy PAGE, the page size, free space not persisted; same bytes and flags as HDF5 2.0), with the file padded to a whole page. h5py opens it, reports the strategy and page size, and can modify it in r+ mode. Page sizes outside libhdf5's 512 B..1 GiB are an error. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 115 +++++++++++++++--- crates/clawhdf5-format/src/superblock.rs | 13 +- .../tests/writer_meta_tests.rs | 56 +++++++++ 3 files changed, 163 insertions(+), 21 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index b8d0edb..045ad9b 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -4,7 +4,7 @@ //! link messages, contiguous datasets, inline and dense attributes. #[cfg(not(feature = "std"))] -use alloc::{string::String, string::ToString, vec, vec::Vec}; +use alloc::{format, string::String, string::ToString, vec, vec::Vec}; use crate::attribute::AttributeMessage; use crate::chunked_write::{ @@ -38,6 +38,31 @@ const SUPERBLOCK_SIZE: usize = 48; /// field is 2 bytes. Bigger "compact" requests fall back to contiguous storage. const MAX_COMPACT_DATA_SIZE: usize = crate::object_header_writer::MAX_MESSAGE_SIZE - 4; +/// libhdf5's bounds on a file space page size (`H5F_FILE_SPACE_PAGE_SIZE_MIN` +/// and `_MAX`). +const MIN_FILE_SPACE_PAGE_SIZE: u32 = 512; +const MAX_FILE_SPACE_PAGE_SIZE: u32 = 1024 * 1024 * 1024; + +/// Superblock extension object header for a file using the paged file-space +/// strategy: a single File Space Info message (0x0017), as libhdf5 writes it +/// for `fs_strategy="page"` without persisted free space. +fn build_paged_superblock_extension(page_size: u32) -> Result, FormatError> { + let mut fsinfo = Vec::new(); + fsinfo.push(1); // version + fsinfo.push(1); // strategy: H5F_FSPACE_STRATEGY_PAGE + fsinfo.push(0); // persisting free space: no + write_length(&mut fsinfo, 1, LENGTH_SIZE); // free-space section threshold + write_length(&mut fsinfo, u64::from(page_size), LENGTH_SIZE); + fsinfo.extend_from_slice(&0u16.to_le_bytes()); // page end metadata threshold + write_undef_offset(&mut fsinfo, OFFSET_SIZE); // EOA before free-space info + let mut w = ObjectHeaderWriter::new(); + // Flags as libhdf5 sets them: bit 2 (never share) and bit 4 (mark if + // unknown). Not constant: libhdf5 rewrites the message when it closes a + // file it opened for writing. + w.add_message_with_flags(MessageType::Unknown(0x0017), fsinfo, 0x14); + w.serialize() +} + /// Threshold for switching from compact (inline) to dense attribute storage. const DENSE_ATTR_THRESHOLD: usize = 8; @@ -961,7 +986,9 @@ pub struct FileWriter { alignment_threshold: usize, /// Global alignment boundary in bytes (0 = disabled). alignment_bytes: usize, - /// Page size for page-buffer mode. When set, a v4 superblock is written. + /// File space page size. When set, the file uses libhdf5's paged + /// file-space strategy (a File Space Info message in the superblock + /// extension). page_size: Option, } @@ -993,9 +1020,16 @@ impl FileWriter { self } - /// Enable page-buffer mode with the given page size. Writing this causes - /// the file to be written with a v4 superblock (page_size field) instead - /// of the default v3. + /// Write the file with libhdf5's *paged* file-space strategy and the given + /// page size, as `H5Pset_file_space_strategy(H5F_FSPACE_STRATEGY_PAGE)` + + /// `H5Pset_file_space_page_size` (h5py: `fs_strategy="page"`, + /// `fs_page_size=...`) do: a v3 superblock with an extension holding a + /// File Space Info message, and the file padded to a whole number of + /// pages. Readers with a page buffer can then fetch metadata page by page. + /// + /// `page_size` must be between 512 bytes and 1 GiB (libhdf5's limits); + /// [`Self::finish`] fails otherwise. This used to write a "version 4" + /// superblock, which does not exist and no HDF5 library can open. pub fn with_page_size(&mut self, page_size: u32) -> &mut Self { self.page_size = Some(page_size); self @@ -1020,6 +1054,14 @@ impl FileWriter { pub fn finish(self) -> Result, FormatError> { let page_size = self.page_size; + if let Some(ps) = page_size + && !(MIN_FILE_SPACE_PAGE_SIZE..=MAX_FILE_SPACE_PAGE_SIZE).contains(&ps) + { + return Err(FormatError::SerializationError(format!( + "file space page size {ps} is outside libhdf5's \ + {MIN_FILE_SPACE_PAGE_SIZE}..={MAX_FILE_SPACE_PAGE_SIZE} bytes" + ))); + } struct DsFlat { name: String, dt: Datatype, @@ -1328,12 +1370,12 @@ impl FileWriter { let actual_ds_oh_sizes: Vec = dummy_blobs.iter().map(|b| b.oh_bytes.len()).collect(); // Pass 2: compute real addresses - // v4 superblocks add a 4-byte page_size field before the checksum. - let superblock_size = if page_size.is_some() { - SUPERBLOCK_SIZE + 4 - } else { - SUPERBLOCK_SIZE - }; + // A paged file carries its File Space Info in a superblock extension + // object header, placed right after the superblock. + let sb_ext = page_size + .map(build_paged_superblock_extension) + .transpose()?; + let superblock_size = SUPERBLOCK_SIZE + sb_ext.as_ref().map_or(0, Vec::len); let root_group_addr = superblock_size as u64; let mut cursor2 = superblock_size + root_oh_size; @@ -1507,11 +1549,16 @@ impl FileWriter { let actual_ds_oh_sizes2: Vec = ds_blobs2.iter().map(|b| b.oh_bytes.len()).collect(); debug_assert_eq!(actual_ds_oh_sizes, actual_ds_oh_sizes2); + // libhdf5 ends a paged file on a page boundary. + let data_end = cursor2; + if let Some(ps) = page_size { + cursor2 = cursor2.next_multiple_of(ps as usize); + } let eof_addr2 = cursor2 as u64; let mut buf = Vec::with_capacity(cursor2); let sb = Superblock { - version: if page_size.is_some() { 4 } else { 3 }, + version: 3, offset_size: OFFSET_SIZE, length_size: LENGTH_SIZE, base_address: 0, @@ -1523,11 +1570,18 @@ impl FileWriter { free_space_address: None, driver_info_address: None, consistency_flags: 0, - superblock_extension_address: Some(u64::MAX), + superblock_extension_address: Some(if sb_ext.is_some() { + SUPERBLOCK_SIZE as u64 + } else { + u64::MAX + }), checksum: None, - page_size, + page_size: None, }; buf.extend_from_slice(&sb.serialize()); + if let Some(ref ext) = sb_ext { + buf.extend_from_slice(ext); + } // Root group OH let mut root_links: Vec = Vec::new(); @@ -1595,7 +1649,8 @@ impl FileWriter { buf.extend_from_slice(&blob.data); } - debug_assert_eq!(buf.len(), cursor2); + debug_assert_eq!(buf.len(), data_end); + buf.resize(cursor2, 0); Ok(buf) } } @@ -2169,7 +2224,8 @@ mod tests { } #[test] - fn file_writer_v4_superblock() { + fn file_writer_paged_file_uses_v3_superblock_and_fsinfo_extension() { + // This used to write superblock "version 4", which does not exist. let mut fw = FileWriter::new(); fw.with_page_size(4096); fw.create_dataset("data").with_f64_data(&[1.0, 2.0]); @@ -2177,8 +2233,31 @@ mod tests { let sig = signature::find_signature(&bytes).unwrap(); let sb = Superblock::parse(&bytes, sig).unwrap(); - assert_eq!(sb.version, 4, "expected superblock v4"); - assert_eq!(sb.page_size, Some(4096)); + assert_eq!(sb.version, 3); + assert_eq!(sb.superblock_extension_address, Some(48)); + assert_eq!(bytes.len() % 4096, 0); + assert_eq!(sb.eof_address, bytes.len() as u64); + let ext = ObjectHeader::parse(&bytes, 48, 8, 8).unwrap(); + let fsinfo = &ext.messages[0]; + assert_eq!(fsinfo.msg_type, MessageType::Unknown(0x0017)); + // Byte-for-byte what HDF5 2.0 writes for fs_strategy="page", + // fs_page_size=4096. + let mut expected = vec![1u8, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0]; + expected.extend_from_slice(&4096u64.to_le_bytes()); + expected.extend_from_slice(&[0, 0]); + expected.extend_from_slice(&[0xff; 8]); + assert_eq!(fsinfo.data, expected); + assert_eq!(fsinfo.flags, 0x14); + assert_eq!(read_dataset_f64(&bytes, "data"), vec![1.0, 2.0]); + } + + #[test] + fn file_writer_rejects_page_sizes_libhdf5_would() { + for ps in [0u32, 511, MAX_FILE_SPACE_PAGE_SIZE + 1] { + let mut fw = FileWriter::new(); + fw.with_page_size(ps); + assert!(fw.finish().is_err(), "page size {ps}"); + } } #[test] diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index aedcc55..e971495 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -39,7 +39,13 @@ pub struct Superblock { pub superblock_extension_address: Option, /// CRC32C checksum (v2/v3 only). pub checksum: Option, - /// Page size for page-buffer mode (v4 only). `None` for v0–v3. + /// Page size of the non-standard "version 4" superblock layout (v4 only). + /// `None` for v0–v3. + /// + /// HDF5 has no superblock version 4 — libhdf5 refuses it. A real paged + /// file is a v2/v3 superblock whose extension holds a File Space Info + /// message (what `FileWriter::with_page_size` writes). This field is kept + /// only so such files written by older clawhdf5 versions still parse. pub page_size: Option, } @@ -127,8 +133,9 @@ impl Superblock { /// Serialize this superblock to bytes. /// - /// Writes v2/v3 format, or v4 (with `page_size`) when `self.version == 4`. - /// Computes and appends Jenkins lookup3 checksum. + /// Writes v2/v3 format, or the non-standard v4 (with `page_size`) when + /// `self.version == 4` — which no HDF5 library opens; see + /// [`Self::page_size`]. Computes and appends Jenkins lookup3 checksum. pub fn serialize(&self) -> Vec { let mut buf = Vec::with_capacity(48); buf.extend_from_slice(&HDF5_SIGNATURE); diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index 22db1c5..48f5c12 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -317,3 +317,59 @@ fn raw_attributes_copied_from_h5py_survive_a_rewrite() { assert_eq!(out, r#"[["/", "/"], "abcdefgh", 1, [258, 772]]"#); h5dump_ok(&path); } + +// ---- 3. paged file-space strategy ---- + +fn paged_file(page_size: u32) -> Vec { + let mut fw = FileWriter::new(); + fw.with_page_size(page_size); + fw.create_dataset("d").with_f64_data(&[1.0, 2.0, 3.0]); + fw.create_dataset("c") + .with_i32_data(&(0..100).collect::>()) + .with_chunks(&[10]); + fw.set_root_attr("a", AttrValue::I64(7)); + let mut g = fw.create_group("g"); + g.create_dataset("e").with_u8_data(&[9; 5000]); + fw.add_group(g.finish()); + fw.finish().unwrap() +} + +#[test] +fn paged_file_has_a_real_superblock() { + // Measured: `with_page_size` wrote superblock version 4, which does not + // exist ("bad superblock version number" in libhdf5). + for ps in [512u32, 4096, 65536] { + let bytes = paged_file(ps); + let (sb, _) = header_at(&bytes, "/"); + assert_eq!(sb.version, 3); + assert_eq!(bytes.len() % ps as usize, 0); + let (_, e) = header_at(&bytes, "g/e"); + assert!( + e.messages + .iter() + .any(|m| m.msg_type == MessageType::Dataspace) + ); + } +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_opens_paged_files() { + for ps in [512u32, 4096, 65536] { + let path = write_tmp(&format!("paged_{ps}"), &paged_file(ps)); + let out = h5py( + &path, + "f = h5py.File(path, 'r')\n\ + p = f.id.get_create_plist()\n\ + print(json.dumps([p.get_file_space_strategy()[0], p.get_file_space_page_size(),\n\ + \x20 f['d'][()].tolist(), int(f['c'][()].sum()), int(f.attrs['a']),\n\ + \x20 int(f['g/e'][()].sum())]))", + ); + assert_eq!( + out, + format!("[1, {ps}, [1.0, 2.0, 3.0], 4950, 7, 45000]"), + "page size {ps}" + ); + h5dump_ok(&path); + } +}