Fix write-path integer overflow in scale-offset filter - #6517
Open
mattjala wants to merge 8 commits into
Open
Conversation
mattjala
requested review from
fortnern,
glennsong09,
jhendersonHDF and
vchoi-hdfgroup
as code owners
July 10, 2026 15:12
mattjala
added
the
Component - Testing
There was a problem hiding this comment.
Pull request overview
Adds a regression test to ensure the scale-offset filter can round-trip integer chunk data that spans the full precision of the datatype (i.e., stored minbits equals the type’s full bit width), covering the failure mode described in #5861 and fixed in #5864.
Changes:
- Add a new dedicated test file name entry (
scaleoffset_fullprec) to the test file list. - Introduce
test_scaleoffset_int_fullprec()intest/dsets.cto write, close/reopen, and read back full-range integer data through the scale-offset filter. - Register the new test in
main()alongside existing scale-offset tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
removed request for
glennsong09,
jhendersonHDF and
vchoi-hdfgroup
July 10, 2026 15:31
Contributor
mattjala
changed the title
fortnern
previously approved these changes
Jul 10, 2026
hyoklee
requested changes
Jul 13, 2026
github-project-automation
Bot
moved this from On-Deck
to In progress
in HDF5 - TRIAGE & TRACK
hyoklee
previously approved these changes
Jul 14, 2026
mattjala
force-pushed
the
5861_regression_test
branch
from
July 17, 2026 16:36
de866bf to
df8c66c
Compare
hyoklee
previously approved these changes
Jul 20, 2026
github-actions
Bot
requested review from
hyoklee
and removed request for
jhendersonHDF
August 5, 2026 14:42
hyoklee
previously approved these changes
Aug 6, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hyoklee
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This started as a regression test for #5864,
test_scaleoffset_int_fullprecintest/dsets.c. It writes integer data spanning the entire type range, which forces the filter into its full-precision fallback and exercises the read path that #5864 fixed.The test ran into a different bug, a signed-integer overflow on the write path. The filter was computing
max - minin signed int which, for full-range data, would overflow a signed int.That subtraction is now performed in unsigned arithmetic instead, which will be safe for any range of input values.
I also added a pre-emptive test for a the analgous full-precision for float datatypes (
test_scaleoffset_float_fullprec), though there's no existing problems in that case.Resolves #5861