crypto: replace goto SSL_CTX_use_certificate_chain#23113
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added c++ 
danbev commented Sep 27, 2018
Uh oh!
There was an error while loading. Please reload this page.
src/node_crypto.cc Outdated
There was a problem hiding this comment.
There is a potential footgun here. Someone someday might look at line 686 and see this:
ret = SSL_CTX_use_certificate_chain(...); return ret;And change that to:
return SSL_CTX_use_certificate_chain(...);Without realizing that the assignment to ret is necessary because of its side effects. Probably also easy to miss in code review.
There was a problem hiding this comment.
I think we could really do a variant here that uses a smart pointer and releases it if necessary; it’s just not trivial to translate the code to that…
There was a problem hiding this comment.
Sorry about the late response on this. I'll take another stab at this today. Thanks
danbevforce-pushed the crypto_cert_chain_goto branch from bbfee1c to 6540586Compare
This commit removes the goto statements in SSL_CTX_use_certificate_chain by using a unique_ptr.
danbevforce-pushed the crypto_cert_chain_goto branch from 6540586 to a46fae0CompareUpdated CI: https://ci.nodejs.org/job/node-test-pull-request/17623/ (:heavy_check_mark:) |
addaleax added the author ready 
danbev commented Oct 8, 2018
Landed in 8d218a9. |
This commit removes the goto statements in SSL_CTX_use_certificate_chain by using a unique_ptr. PR-URL: #23113 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit removes the goto statements in SSL_CTX_use_certificate_chain by using a unique_ptr. PR-URL: #23113 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit removes the goto statements in SSL_CTX_use_certificate_chain by using a unique_ptr. PR-URL: #23113 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
5 participants
This commit removes the goto statements in SSL_CTX_use_certificate_chain
by using a unique_ptr.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes