Skip to content

Comments

Introduce X509_add_cert[s] simplifying various additions to cert lists#12615

Merged
openssl-machine merged 1 commit intoopenssl:masterfrom
siemens:cleanup_sk_X509_add1_cert
Aug 12, 2020
Merged

Introduce X509_add_cert[s] simplifying various additions to cert lists#12615
openssl-machine merged 1 commit intoopenssl:masterfrom
siemens:cleanup_sk_X509_add1_cert

Conversation

@DDvO
Copy link
Contributor

@DDvO DDvO commented Aug 9, 2020

This adds two versatile cert addition functions and uses them for simplifying various code
that adds (lists of) certificates to certificate lists, including reference count and error handling.
This includes options for leaving out duplicate certs and/or self-signed certs and for prepending rather than appending.

Checklist
  • documentation is added or updated
  • tests are added or updated

@DDvO DDvO force-pushed the cleanup_sk_X509_add1_cert branch from adba946 to c098b22 Compare August 10, 2020 06:33
@DDvO
Copy link
Contributor Author

DDvO commented Aug 10, 2020

Rebased to take advantage of the (unrelated) mem leak fix #12613.
Now all CI runs will pass.

@DDvO DDvO requested a review from mattcaswell August 10, 2020 06:36
@DDvO DDvO force-pushed the cleanup_sk_X509_add1_cert branch from c098b22 to 1ff01d7 Compare August 10, 2020 20:07
@DDvO DDvO changed the title Introduce OSSL_sk_X509_add_cert[s] simpliying various additions to cert lists Introduce X509_add_cert[s] simpliying various additions to cert lists Aug 10, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Aug 10, 2020

Thanks @mattcaswell for your very swift and useful comments.
I've meanwhile done the suggested changes.

@DDvO DDvO force-pushed the cleanup_sk_X509_add1_cert branch from 1ff01d7 to b479b3f Compare August 11, 2020 09:53
@DDvO DDvO changed the title Introduce X509_add_cert[s] simpliying various additions to cert lists Introduce X509_add_cert[s] simplifying various additions to cert lists Aug 11, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Aug 11, 2020

I've done the respective fixups and squashed the commits.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

LGTM

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Aug 11, 2020
@DDvO DDvO added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Aug 12, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Aug 12, 2020

Setting "ready to merge" manually because the only Travis issue reported is unrelated (CI-internal issue "Automatic restarts limited").

@DDvO DDvO force-pushed the cleanup_sk_X509_add1_cert branch from b479b3f to eeccc23 Compare August 12, 2020 12:00
@openssl-machine openssl-machine merged commit eeccc23 into openssl:master Aug 12, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Aug 12, 2020

Merged - thanks @mattcaswell

swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
DDvO added a commit to siemens/openssl that referenced this pull request Mar 6, 2021
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 approval: ready to merge The 24 hour grace period has passed, ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants