Skip to content

Make SSL_CTX_set_cert_store increase the reference counter#1795

Closed
kroeckx wants to merge 1 commit intoopenssl:masterfrom
kroeckx:set_cert_store
Closed

Make SSL_CTX_set_cert_store increase the reference counter#1795
kroeckx wants to merge 1 commit intoopenssl:masterfrom
kroeckx:set_cert_store

Conversation

@kroeckx
Copy link
Copy Markdown
Member

@kroeckx kroeckx commented Oct 27, 2016

I'm not sure if we covered something like this in our discussion about the style guide. But I think since it's freeing the old one, it should increase reference counter for the new one.

This came up in nodejs/node#8491 (review)

@kroeckx kroeckx added branch: master Applies to master branch 1.1.0 labels Oct 27, 2016
@davidben
Copy link
Copy Markdown
Contributor

This has been the behavior since SSLeay 0.8.1b, so it's probably not reasonable to change at this point. Calling free while not taking a reference is consistent with all the other set0 functions in the library. If you want one that takes a reference, probably call it SSL_CTX_set1_cert_store.

Though given that this function already was split into SSL_CTX_set1_verify_cert_store and SSL_CTX_set1_chain_cert_store which two have both set0 and set1 variants, perhaps leaving it alone and deprecating the combined one in favor of the split ones makes more sense?

@kroeckx
Copy link
Copy Markdown
Member Author

kroeckx commented Oct 28, 2016

I assume this was a new function, but you're right it's not. And the new functions already do the right thing it seems, so maybe I should just use those instead.

@kroeckx kroeckx closed this Oct 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants