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

rand: Coverity fixes #12309

Closed
wants to merge 5 commits into from
Closed

Conversation

paulidale
Copy link
Contributor

Mostly fixing problems with the provider friendly rand work.

Also includes a fix for a null pointer dereference in properties and the removal of an unnecessary NULL check in apps/cmp.c

  • tests are added or updated

Instead appease coverity by marking 1464986 as a false positive.
Coverity is confused by the engine reference counting.
@paulidale paulidale added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Jun 28, 2020
@paulidale paulidale added this to the 3.0.0 milestone Jun 28, 2020
@paulidale paulidale added this to In progress in 3.0 New Core + FIPS via automation Jun 28, 2020
@paulidale paulidale changed the title rand: coverity fixes rand: Coverity fixes Jun 28, 2020
@paulidale
Copy link
Contributor Author

Ping for review please.

apps/cmp.c Show resolved Hide resolved
@slontis
Copy link
Member

slontis commented Jul 2, 2020

Looks ok to me.. Would approve if I could :)

@DDvO
Copy link
Contributor

DDvO commented Jul 2, 2020

Looks ok to me.. Would approve if I could :)

As @mattcaswell taught me:
Even where a normal committer's approval does not formally count, it's a helpful indication to others.
So please feel encouraged to give approvals from your perspective whenever you convinced yourself that a PR is fine.

@paulidale
Copy link
Contributor Author

@slontis and I work for the same employer, we don't approve each other's PRs. Doing so would risk merging happening.

3.0 New Core + FIPS automation moved this from In progress to Reviewer approved Jul 3, 2020
@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 Jul 3, 2020
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Jul 4, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Jul 4, 2020
@paulidale
Copy link
Contributor Author

Merged to master, thanks for the feedback.

@paulidale paulidale closed this Jul 5, 2020
3.0 New Core + FIPS automation moved this from Reviewer approved to Done Jul 5, 2020
@paulidale paulidale deleted the rand-coverity branch July 5, 2020 03:22
openssl-machine pushed a commit that referenced this pull request Jul 5, 2020
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #12309)
openssl-machine pushed a commit that referenced this pull request Jul 5, 2020
Instead appease coverity by marking 1464986 as a false positive.
Coverity is confused by the engine reference counting.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #12309)
openssl-machine pushed a commit that referenced this pull request Jul 5, 2020
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #12309)
openssl-machine pushed a commit that referenced this pull request Jul 5, 2020
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #12309)
openssl-machine pushed a commit that referenced this pull request Jul 5, 2020
…s it.

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #12309)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants