-
Notifications
You must be signed in to change notification settings - Fork 41
Make allocation of large structures/buffers configurable (without workspaces) #1389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
6dba3fb to
1bf050d
Compare
e7ce05a to
db0a93a
Compare
db0a93a to
5d4bc68
Compare
mkannwischer
requested changes
Dec 16, 2025
Contributor
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hanno-becker. I only found some minor issues. Rest looks good.
5d4bc68 to
d41474e
Compare
mkannwischer
approved these changes
Dec 16, 2025
Contributor
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hanno-becker. LGTM now.
This commit introduces the configuration option MLK_CONFIG_CUSTOM_ALLOC_FREE which allows users to provide custom macros MLK_CUSTOM_ALLOC(v, T, N) MLK_CUSTOM_FREE(v, T, N) that should be used in place of the default stack-allocation to allocate/free large internal structures. Those macros are then wrapped into MLK_ALLOC/MLK_FREE and used in the source. Importantly, MLK_FREE adds zeroization ahead of calling MLK_CUSTOM_FREE, so the latter need not take care of this. The macros are put to use in kem.c and indcpa.c, but not yet further down the call-stack. The option is documented as experimental and (hence) unstable so we have freedom to adjust this ahead of v2. A custom configuration is added which implements the macros based on C11's `aligned_alloc`. We enable the leak sanitizer in the tests for this configuration to catch any missing calls to MLK_FREE. Since allocation can fail, additional error paths are introduced to the functions using MLK_ALLOC. This uniformly follows the pattern of all pointers being initialized to NULL before allocation, and a cleanup section calling MLK_FREE on them before returning the result. To distinguish between out-of-memory failure and other kinds of functional failures, we introduce named error codes MLK_ERR_FAIL and MLK_ERR_OUT_OF_MEMORY. This will likely need further refinement, but this can only be done in v2. Signed-off-by: Hanno Becker <[email protected]>
d41474e to
6b87ab4
Compare
3 tasks
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 commit introduces the configuration option
MLK_CONFIG_CUSTOM_ALLOC_FREE
which allows users to provide custom macros
MLK_CUSTOM_ALLOC(v, T, N)
MLK_CUSTOM_FREE(v, T, N)
that should be used in place of the default stack-allocation
to allocate/free large internal structures. Those macros are
then wrapped into MLK_ALLOC/MLK_FREE and used in the source.
Importantly, MLK_FREE adds zeroization ahead of calling
MLK_CUSTOM_FREE, so the latter need not take care of this.
The macros are put to use in kem.c and indcpa.c, but not yet
further down the call-stack.
The option is documented as experimental and (hence) unstable
so we have freedom to adjust this ahead of v2.
A custom configuration is added which implements the macros
based on C11's
aligned_alloc. We enable the leak sanitizerin the tests for this configuration to catch any missing calls
to MLK_FREE.
Since allocation can fail, additional error paths are introduced
to the functions using MLK_ALLOC. This uniformly follows the pattern
of all pointers being initialized to NULL before allocation, and a
cleanup section calling MLK_FREE on them before returning the result.
To distinguish between out-of-memory failure and other kinds of
functional failures, we introduce named error codes MLK_ERR_FAIL
and MLK_ERR_OUT_OF_MEMORY. This will likely need further refinement,
but this can only be done in v2.
Signed-off-by: Hanno Becker [email protected]