Skip to content

Refactor crypto_openssl3 to use return types internally#76

Merged
bifurcation merged 3 commits intocisco:mainfrom
k-wasniowski:refactor-internal-openssl3
Mar 9, 2026
Merged

Refactor crypto_openssl3 to use return types internally#76
bifurcation merged 3 commits intocisco:mainfrom
k-wasniowski:refactor-internal-openssl3

Conversation

@k-wasniowski
Copy link
Contributor

No description provided.

{
auto mode = EVP_KDF_HKDF_MODE_EXTRACT_ONLY;
auto digest_name = openssl_digest_name(suite);
auto digest_name = SFRAME_VALUE_OR_THROW(openssl_digest_name(suite));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll allow this for now, in order to avoid changing the crypto.h method signatures. But we should ultimately not have any _OR_THROW except in the public API.

Comment on lines +41 to +42
return Result<const EVP_CIPHER*>::err(
SFrameErrorType::unsupported_ciphersuite_error);
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these Result::err calls get kind of verbose. I wonder if it would be worth making an abbreviation macro, something like:

Suggested change
return Result<const EVP_CIPHER*>::err(
SFrameErrorType::unsupported_ciphersuite_error);
return SFRAME_ERROR(const EVP_Cipher*, unsupported_ciphersuite_error);

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, now that I think of it, the Error constructor pretty much makes err() and ok() unnecessary. You can just return SFrameErrorType::unsupported_ciphersuite_error and those handy C++ implicit conversion rules will take care of things for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bifurcation updated

@bifurcation bifurcation merged commit acc16c1 into cisco:main Mar 9, 2026
13 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants