Skip to content

Fix reading global heap - #260

Open
davidhassell wants to merge 1 commit into
NCAS-CMS:mainfrom
davidhassell:heap-access
Open

Fix reading global heap#260
davidhassell wants to merge 1 commit into
NCAS-CMS:mainfrom
davidhassell:heap-access

Conversation

@davidhassell

Copy link
Copy Markdown
Collaborator

Fix reading global heap

Closes #259

No unit test has been provided, because I found it hard to recreate the conditions in a reliable way! However, the two use cases from the issue now work, and all existing tests pass, so I was going to leave it at that. Can try harder to make a units test, if you like ...

Before you get started

Checklist

  • This pull request has a descriptive title and labels
  • This pull request has a minimal description (most was discussed in the issue, but a two-liner description is still desirable)
  • Unit tests have been added (if codecov test fails)
  • Any changed dependencies have been added or removed correctly (if need be)
  • If you are working on the documentation, please ensure the current build passes
  • All tests pass

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.48%. Comparing base (667c468) to head (8daca6b).

Files with missing lines Patch % Lines
pyfive/misc_low_level.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #260   +/-   ##
=======================================
  Coverage   78.48%   78.48%           
=======================================
  Files          15       15           
  Lines        3416     3416           
  Branches      546      546           
=======================================
  Hits         2681     2681           
  Misses        593      593           
  Partials      142      142           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ajelenak

Copy link
Copy Markdown

Hi @davidhassell ! What a coincidence... I hit this same issue reading the vlen-string attributes of the
METADATA/DatasetIdentification group in NASA GEDI L2A files yesterday.

Since you have the right fix (mine was the same), below are the tests I prepared for my PR. You are welcome to use them, or I can make a PR.

""" Unit tests for pyfive's Global Heap collection reader. """
import io
import struct

import pytest
from pyfive.misc_low_level import GLOBAL_HEAP_HEADER_SIZE, GlobalHeap


def build_global_heap_collection(objects, trailing_pad=0):
    """ Return the bytes of a valid HDF5 Global Heap collection (GCOL).

    ``objects`` is a list of ``(object_index, object_data)`` pairs. Each object's
    data is padded to an 8-byte boundary, as libhdf5 writes it. ``trailing_pad`` is
    the number of unused free-space bytes appended after the last object. A pad
    smaller than one 16-byte object header cannot hold the index-0 free-space marker
    and is just padding.
    """
    body = b""
    for index, data in objects:
        header = struct.pack("<HHIQ", index, 1, 0, len(data))
        padded = data + b"\x00" * ((-len(data)) % 8)
        body += header + padded
    body += b"\x00" * trailing_pad

    collection_size = GLOBAL_HEAP_HEADER_SIZE + len(body)
    gcol_header = (
        b"GCOL" + struct.pack("<B", 1) + b"\x00\x00\x00" + struct.pack("<Q", collection_size)
    )
    return gcol_header + body


def read_objects(raw):
    return GlobalHeap(io.BytesIO(raw), 0).objects


def test_trailing_padding_smaller_than_header():
    """ Free space of < 16 bytes at the end must not be read as an object. """
    raw = build_global_heap_collection([(1, b"hello"), (2, b"abc")], trailing_pad=8)
    assert read_objects(raw) == {1: b"hello", 2: b"abc"}


def test_explicit_free_space_terminator():
    """ An index-0 free-space object still stops iteration cleanly. """
    body = (
        struct.pack("<HHIQ", 1, 1, 0, 5) + b"hello" + b"\x00" * 3
        + struct.pack("<HHIQ", 0, 0, 0, 0)
    )
    collection_size = GLOBAL_HEAP_HEADER_SIZE + len(body)
    raw = (
        b"GCOL" + struct.pack("<B", 1) + b"\x00\x00\x00"
        + struct.pack("<Q", collection_size) + body
    )
    assert read_objects(raw) == {1: b"hello"}


def test_collection_exactly_full():
    """ No trailing bytes: every object is parsed and the loop ends at the buffer end. """
    raw = build_global_heap_collection([(1, b"hello"), (2, b"abcdefgh")], trailing_pad=0)
    assert read_objects(raw) == {1: b"hello", 2: b"abcdefgh"}


@pytest.mark.parametrize("pad", [1, 4, 8, 15])
def test_sub_header_trailing_pad(pad):
    """ Any trailing pad shorter than one object header is tolerated. """
    raw = build_global_heap_collection([(7, b"payload")], trailing_pad=pad)
    assert read_objects(raw) == {7: b"payload"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error in consolidated_metadata when there are lots of variables

2 participants