crates/clawhdf5-format/src/file_writer.rs

@@ -903,9 +903,6 @@ pub(crate) fn build_dense_attrs(
         .map(|a| crate::checksum::jenkins_lookup3(a.name.as_bytes()))
         .collect();
 
-    let os = OFFSET_SIZE as usize;
-    let ls = LENGTH_SIZE as usize;
-
     // Attribute heaps use max_heap_size 40 / heap ID length 8 (matching libhdf5).
     let heap = build_single_block_fractal_heap(&serialized, base_address, 40, 8)?;
     let frhp_addr = heap.frhp_addr;
@@ -926,48 +923,16 @@ pub(crate) fn build_dense_attrs(
     }
     records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1)));
 
-    let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4;
-    let num_records = attrs.len();
-    let btlf_size = 4 + 1 + 1 + (num_records * record_size as usize) + 4;
-    let node_size = btlf_size.next_power_of_two().max(512) as u32;
-
+    let records: Vec<Vec<u8>> = records.into_iter().map(|(_, _, rec)| rec).collect();
     let bthd_addr = btree_addr;
-    let btlf_addr = bthd_addr + bthd_size as u64;
-
-    let mut bthd = Vec::with_capacity(bthd_size);
-    bthd.extend_from_slice(b"BTHD");
-    bthd.push(0); // version
-    bthd.push(8); // type = attribute name index
-    bthd.extend_from_slice(&node_size.to_le_bytes());
-    bthd.extend_from_slice(&record_size.to_le_bytes());
-    bthd.extend_from_slice(&0u16.to_le_bytes()); // depth = 0
-    bthd.push(100); // split_percent
-    bthd.push(40); // merge_percent
-    write_offset(&mut bthd, btlf_addr, OFFSET_SIZE);
-    bthd.extend_from_slice(&(num_records as u16).to_le_bytes());
-    write_length(&mut bthd, num_records as u64, LENGTH_SIZE);
-    let bthd_checksum = crate::checksum::jenkins_lookup3(&bthd);
-    bthd.extend_from_slice(&bthd_checksum.to_le_bytes());
-    debug_assert_eq!(bthd.len(), bthd_size);
-
-    let mut btlf = Vec::with_capacity(node_size as usize);
-    btlf.extend_from_slice(b"BTLF");
-    btlf.push(0); // version
-    btlf.push(8); // type
-    for (_, _, rec) in &records {
-        btlf.extend_from_slice(rec);
-    }
-    // Checksum goes immediately after records (NOT at end of node).
-    // HDF5 C library computes checksum over sig+ver+type+records only.
-    let btlf_checksum = crate::checksum::jenkins_lookup3(&btlf);
-    btlf.extend_from_slice(&btlf_checksum.to_le_bytes());
-    // Pad to node_size
-    btlf.resize(node_size as usize, 0);
-
-    let mut blob = Vec::with_capacity(heap.blob.len() + bthd.len() + btlf.len());
-    blob.extend_from_slice(&heap.blob);
-    blob.extend_from_slice(&bthd);
-    blob.extend_from_slice(&btlf);
+    let mut blob = heap.blob;
+    blob.extend_from_slice(&single_leaf_v2_btree(
+        8,
+        record_size,
+        &records,
+        bthd_addr,
+        "attributes on one object",
+    )?);
 
     let attr_info = serialize_attribute_info(frhp_addr, bthd_addr);
 
@@ -989,12 +954,14 @@ pub(crate) struct DenseLinkBlob {
 }
 
 /// A v2 B-tree of `btree_type` holding `records` (already in key order) in a
-/// single leaf, laid out at `addr`: the header, then the leaf.
+/// single leaf, laid out at `addr`: the header, then the leaf. `what` names
+/// the records in the error for too many ("links in one group").
 fn single_leaf_v2_btree(
     btree_type: u8,
     record_size: u16,
     records: &[Vec<u8>],
     addr: u64,
+    what: &str,
 ) -> Result<Vec<u8>, FormatError> {
     let os = OFFSET_SIZE as usize;
     let ls = LENGTH_SIZE as usize;
@@ -1002,15 +969,21 @@ fn single_leaf_v2_btree(
     // internal nodes, which the writer does not build.
     let num_records = u16::try_from(records.len()).map_err(|_| {
         FormatError::SerializationError(format!(
-            "{} links in one group: a group holds at most {} links \
-             (a deeper link index is not implemented)",
+            "{} {what}: at most {} can be written \
+             (a deeper B-tree index is not implemented)",
             records.len(),
             u16::MAX
         ))
     })?;
     let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4;
     let btlf_size = 4 + 1 + 1 + (records.len() * record_size as usize) + 4;
-    let node_size = btlf_size.next_power_of_two().max(512) as u32;
+    // libhdf5 sizes a leaf's capacity from the node size, and a leaf's
+    // record count is a 2-byte field: a node with room for more than
+    // 65 535 records makes it overflow that count when it adds one (the
+    // group can then no longer be listed). Cap the node at a full leaf.
+    let max_node = btlf_size - records.len() * record_size as usize
+        + usize::from(u16::MAX) * record_size as usize;
+    let node_size = btlf_size.next_power_of_two().max(512).min(max_node) as u32;
     let btlf_addr = addr + bthd_size as u64;
 
     let mut out = Vec::with_capacity(bthd_size + node_size as usize);
@@ -1089,6 +1062,7 @@ pub(crate) fn build_dense_links(
         4 + heap_id_length,
         &name_records,
         name_bt_addr,
+        "links in one group",
     )?);
 
     let link_info_message = if track_order {
@@ -1113,6 +1087,7 @@ pub(crate) fn build_dense_links(
             8 + heap_id_length,
             &order_records,
             order_bt_addr,
+            "links in one group",
         )?);
         let next_order = by_order.last().map_or(0, |&(o, _)| o + 1);
         serialize_link_info(

crates/clawhdf5/tests/writer_groups_interop.rs

@@ -538,7 +538,23 @@ fn more_links_than_one_index_leaf_holds_is_an_error() {
         b.add_soft_link(&format!("s{i}"), "/x");
     }
     let err = b.finish().unwrap_err().to_string();
-    assert!(err.contains("at most 65535 links"), "{err}");
+    assert!(
+        err.contains("70000 links in one group: at most 65535"),
+        "{err}"
+    );
+    // Dense attributes have the same one-leaf index. Their count used to
+    // be written modulo 65 536.
+    let mut b = FileBuilder::new();
+    let x = b.create_dataset("x");
+    x.with_i32_data(&[1]);
+    for i in 0..70_000 {
+        x.set_attr(&format!("a{i}"), AttrValue::I64(i));
+    }
+    let err = b.finish().unwrap_err().to_string();
+    assert!(
+        err.contains("70000 attributes on one object: at most 65535"),
+        "{err}"
+    );
 }
 
 #[test]
@@ -712,17 +728,21 @@ fn dense_links_past_the_direct_blocks_of_the_root() {
 
     // libhdf5 can add to and delete from the heap. It could not when the
     // header's block allocation offset was 0: its next block overwrote the
-    // first ("bad version number for message").
+    // first ("bad version number for message"). Adding to `deep` also
+    // needs its index's leaf node to have room for at most 65 535 records:
+    // a bigger node made libhdf5 overflow the leaf's 2-byte record count
+    // (a crash, or "unknown link class" when listing).
     let out = h5py(
         &path,
         "with h5py.File(path, 'r+') as f:\n\
          \x20   f['g']['zz_new'] = np.arange(3)\n\
+         \x20   f['deep']['zz_new'] = np.arange(4)\n\
          \x20   del f['g/dataset_number_000005']\n\
          with h5py.File(path, 'r') as f:\n\
-         \x20   print(json.dumps([len(f['g']), int(f['g/zz_new'][2]),\n\
-         \x20       int(f['g/dataset_number_019998'][0])]))",
+         \x20   print(json.dumps([len(f['g']), int(f['g/zz_new'][2]), len(f['deep']),\n\
+         \x20       list(f['deep'])[-1], int(f['g/dataset_number_019998'][0])]))",
     );
-    assert_eq!(out, r#"[20000, 2, 19998]"#);
+    assert_eq!(out, r#"[20000, 2, 65536, "zz_new", 19998]"#);
     h5dump_ok(&path);
 }