gh-131316: handle NULL values returned by HACL* functions #131324
Merged
Conversation
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
- Handle NULL returned by allocation functions. - Handle NULL returned by copy functions. - Suppress unused impossible return codes.
Most probably you've already found out: Py_ssize_t is only signed 32bit in 32bit platforms (same for Windows and Linux). |
Yeah... that's what I observed afterwards... so I just removed the assertion (can't fail if the assertion isn't here 😈) |
|
FWIW it is quite reasonable for these update loops to use a size lower than UINT32_MAX, by the time data gets that large the computation already takes a long time so just looping over 1-2gb chunks up to INT32_MAX to fit within our smallest signed size constraint is fine. |
gpshead
approved these changes
Mar 16, 2025
plashchynski
pushed a commit
to plashchynski/cpython
that referenced
this pull request
Mar 17, 2025
…on#131324) - Handle NULL returned by allocation functions. - Handle NULL returned by copy functions. - Suppress unused impossible return codes.
seehwan
pushed a commit
to seehwan/cpython
that referenced
this pull request
Apr 16, 2025
…on#131324) - Handle NULL returned by allocation functions. - Handle NULL returned by copy functions. - Suppress unused impossible return codes.
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.
In a follow-up PR I'd like to move all
UPDATEfunctions in a single helper. The construction is always the same and it would help the HMAC PR as well (I've done it out there).More generally, I'd like to have the same construction for MD5/SHA1/SHA2/SHA3 as they really share common code. I don't know how much I can change the code so that it looks better, which is why I didn't change anything here.
We had a discussion on the blake2 PR concerning the cosmetics of blake2module and I think we can definitely improve how it's currently structured, because the functions become quite ugly. But that'd be another issue.
cc @msprotz @chris-eibl