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

Rework and simplify resource flow in drbg_add #7504

Conversation

bernd-edlinger
Copy link
Member

Not a bug fix, but making things a lot easier to understand (I hope).

@bernd-edlinger bernd-edlinger added branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch labels Oct 26, 2018
@mspncp
Copy link
Contributor

mspncp commented Oct 26, 2018

Thanks, I'll have a look. But let me tie up some other loose ends first. Currently my resources are limited due to family affairs...

Copy link
Contributor

@mspncp mspncp left a comment

Choose a reason for hiding this comment

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

You are right, things make much more sense if the lifetime of the drbg->pool is managed by a single function, in this case rand_drbg_restart(). Also, my idea to use drbg->pool != NULL as an indicator for what happened with the random data was a mistake and an unnecessary complication. With your approach, the undefinedness problem has naturally disappeared.

@mspncp mspncp added this to the 1.1.1a milestone Oct 28, 2018
@mspncp mspncp added the approval: review pending This pull request needs review by a committer label Oct 28, 2018
@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 29, 2018
levitte pushed a commit that referenced this pull request Oct 29, 2018
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #7504)
levitte pushed a commit that referenced this pull request Oct 29, 2018
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #7504)

(cherry picked from commit f9e4392)
@bernd-edlinger
Copy link
Member Author

Merged to master as f9e4392 and 1.1.1 as 6101850, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants