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

Improve wrapper around OpenSSL RAND #4174

Closed
wants to merge 1 commit into from
Closed

Improve wrapper around OpenSSL RAND #4174

wants to merge 1 commit into from

Conversation

nbougalis
Copy link
Contributor

@nbougalis nbougalis commented May 23, 2022

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

This was referenced Aug 4, 2022
@intelliot
Copy link
Collaborator

Reopening because it looks like this was merged with zero reviews. @ximinez can you confirm that you reviewed this change as part of #4270?

If so, you can put an approval on this PR.

I know it has already been merged, but we'd like to confirm approval since this change (if not reverted) will be included in the 1.10 release.

@intelliot intelliot reopened this Feb 23, 2023
@intelliot intelliot added this to the 1.10.0 milestone Feb 23, 2023
@drlongle
Copy link
Contributor

LGTM!

Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

I'm not an expert with openSSL RAND, but these changes look reasonable to me.

@intelliot intelliot closed this Feb 27, 2023
@intelliot
Copy link
Collaborator

  • There used to be a lock around OpenSSL code. OpenSSL internally implements a lock for this, so the other lock is no longer needed.
  • Super minor performance improvement when OpenSSL randomness is used, which is not often.

@nbougalis nbougalis deleted the csprng branch October 16, 2023 06:03
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.

4 participants