Skip to content
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

Use EVP_PKEY_up_ref if available #238

Merged
merged 6 commits into from
Sep 13, 2022
Merged

Use EVP_PKEY_up_ref if available #238

merged 6 commits into from
Sep 13, 2022

Conversation

Thalhammer
Copy link
Owner

OpenSSL 1.1.0 introduced the function EVP_PKEY_up_ref which exposes the internal reference counting to user applications. The new type evp_pkey_handle takes advantage of this by providing RAII semantics for EVP_PKEY pointers similar to shared_ptr, but without having to allocate an additional control block.

One thing I am not sure about is the constructors/assignment of evp_pkey_handle. Technically EVP_PKEY_up_ref can return an error, however the only case where this might actually happen is on a plattform without atomics (pretty rare) and the mutex lock operation failed (you're probably screwed anyway if that happens), so I am tempted to replace the throw with std::terminate (or ignore the return value alltogether) and mark them as noexcept. What makes this even more of an option is the fact that EVP_PKEY_free technically has the same issue and ignores the return as well. What do you think about that @prince-chrismc ?

Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an API change so next up is 0.7.0

But this was something similar to what I was thinking. Having a helper type for evp key.

LGTM

@Thalhammer Thalhammer merged commit 3ed4ff9 into master Sep 13, 2022
@Thalhammer Thalhammer deleted the rework-pkey branch September 13, 2022 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants