Allocation-free operations#349
Conversation
bal-e
force-pushed
the
alloc-free-ops
branch
2 times, most recently
from
3c1d3ff to
bfba84a
Compare
|
I see working with |
hug-dev
left a comment
hug-dev
left a comment
There was a problem hiding this comment.
Thank you!
Really like the added documentation, I think that's good! My only suggestion is to simplify and return the original error as is :)
For testing I think it's fine trying one or a few variants. You can for example use the same one which is done in the test currently for the sign operation!
wiktor-k
left a comment
wiktor-k
left a comment
There was a problem hiding this comment.
Looks great, a couple of nitpicks/clarifications. Thanks for your contribution! 🙇
I think it looks fine as it is 👌
No pressure to add more, this may grow organically as use-cases appear. If you feel like doing additional work I'm not stopping you though 😅
Yep, ideally the tests should exercise all paths, including error ones and check if returned errors match what we expect. Choosing one algo for tests is sufficient. (Ideally it should be implemented by both softhsm and kryoptic but I don't think you'll use anything exotic). Thanks! 👋 |
bal-e
force-pushed
the
alloc-free-ops
branch
from
c072317 to
cbf4015
Compare
bal-e
force-pushed
the
alloc-free-ops
branch
from
cbf4015 to
489e10d
Compare
|
Added tests, resolved review comments, and switched to |
|
It's been a long time @bal-e but if you have the time you could rebase the PR and then we should merge as is since we agreed on going forward with the assert! |
Jakuje
left a comment
Jakuje
left a comment
There was a problem hiding this comment.
We need rebase, but otherwise I think we are good to merge this!
|
Awesome, on it! |
These '_into()' variants can be used when the caller knows the expected buffer size, and has allocated a buffer for the signature themselves. It requires fewer calls into the underlying library and can save on heap allocations. Signed-off-by: arya dradjica <arya@nlnetlabs.nl>
Head branch was pushed to by a user without write access
4 participants
Starts addressing #346.
Question for the reviewers:
What do you think of the added documentation? Is it too much? Or should I add more to the related
sign()functions for consistency?I don't have a strict need to implement this for more operations, but I'm happy to do one or two sets more. Are there any in particular you'd like to see? I'm leaning towards key generation (which I need, but is not performance-sensitive) and encryption (which I guess more people would want to use).
Would you have any tips for adding these methods to the test suite? It'll have to be a bit algorithm-specific.