Skip to content

Add an accumulating SHA256 API (SHA256State, UpdateSHA256, UpdateSHA256File) - #6509

Open
fingolfin wants to merge 2 commits into
gap-system:masterfrom
fingolfin:mh/sha256-accumulate
Open

Add an accumulating SHA256 API (SHA256State, UpdateSHA256, UpdateSHA256File)#6509
fingolfin wants to merge 2 commits into
gap-system:masterfrom
fingolfin:mh/sha256-accumulate

Conversation

@fingolfin

@fingolfin fingolfin commented Aug 14, 2026

Copy link
Copy Markdown
Member

GAP can hash a string, a stream, or a file, but not a concatenation of those without building it in memory first. Git object hashes are exactly that shape: "blob <size>\0" followed by a file's contents

This PR adds functionality to allow handling that within GAP:

s := SHA256State();
UpdateSHA256( s, "blob 15\000" );
UpdateSHA256File( s, name, false );
HexSHA256( s );      # = git hash-object, in a --object-format=sha256 repo

Two drive-by fixes:

  • the state bag is allocated sizeof(UInt4) + sizeof(sha256_state_t) but addressed from ADDR_OBJ(result)[1], i.e. 4 bytes short. Masked by GASMAN rounding bag sizes up to whole words.
  • building the result list allocates, which may move bags, so the state is now taken by value rather than as a pointer into one. On 64-bit a UInt4 is always an immediate integer, which is why this has never bitten.

Written with Claude Opus 5 via Claude Code; reviewed by me. The commit lists
the tool as co-author.

GAP can hash a string, a stream, or a file, but not a concatenation of
those without building it in memory first.  Git object hashes are exactly
that shape -- a header followed by a file's contents -- and packages
computing them have to fall back on the IO package, on piping through
sha256sum, or on reading the whole file into a string.

  s := SHA256State();
  UpdateSHA256( s, "blob 15\000" );
  UpdateSHA256File( s, name, false );
  HexSHA256( s );

The kernel already had the accumulator; it was undocumented, could not
consume a file, and its FINAL pads the state in place, so reading a digest
twice silently gave a wrong answer.  GAP_SHA256_DIGEST finalizes a copy
instead, which makes reading a digest an ordinary operation rather than
one the caller must know is destructive.  HexSHA256 gains a state as its
third case; HexSHA256File is unchanged.

Also:
- give the state a category and a print method, so it can be tested for
  and does not display as <object>;
- fix the state bag being allocated 4 bytes short of what it addresses,
  masked until now by GASMAN rounding bag sizes up;
- take the state by value when building the result list, since allocating
  it may move bags.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: kernel release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Aug 14, 2026
@fingolfin fingolfin changed the title Add an accumulating SHA256 API Add an accumulating SHA256 API (SHA256State, UpdateSHA256, UpdateSHA256File) Aug 14, 2026
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.43590% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.66%. Comparing base (e5b44f2) to head (c104798).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
lib/files.gi 91.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6509      +/-   ##
==========================================
- Coverage   78.87%   78.66%   -0.21%     
==========================================
  Files         685      685              
  Lines      294150   294164      +14     
  Branches     8646     8669      +23     
==========================================
- Hits       232001   231395     -606     
- Misses      60348    60941     +593     
- Partials     1801     1828      +27     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/sha256.c Outdated
Comment thread lib/files.gd Outdated
Comment thread lib/files.gd Outdated
Comment thread src/sha256.c Outdated
Addresses review: the kernel one-shot GAP_SHA256_FILE is gone, and
HexSHA256File is three lines of GAP over SHA256State, UpdateSHA256File
and HexSHA256.  Its argument checking goes too, since UpdateSHA256File
already raises the same errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant