Alexey Serbin has posted comments on this change. Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu ......................................................................
Patch Set 2: (5 comments) Overall looks good to me, just some nits in the commit message. Maybe, Todd also want to take a look? http://gerrit.cloudera.org:8080/#/c/7662/2//COMMIT_MSG Commit Message: PS2, Line 15: 9) nit: please make the string <= 72 characters long PS2, Line 18: throught nit: typo PS2, Line 25: es, nit: please make the string <= 72 characters long http://gerrit.cloudera.org:8080/#/c/7662/2/src/kudu/security/openssl_util.cc File src/kudu/security/openssl_util.cc: PS2, Line 150: auto cleanup = MakeScopedCleanup([&]() { : sk_X509_INFO_pop_free(info, X509_INFO_free); : }); > sk_X509_INFO_pop_free() takes 2 arguments but kFreeFunc is always passed on OK, I think it's good enough. PS2, Line 165: // We don't want the free() call below to free the x509 certificates as well since we will : // use it as a part of the STACK_OF(X509) object to be returned, so we set it to nullptr. : // We will take the responsibility of freeing it when we are done with the STACK_OF(X509). : stack_item->x509 = nullptr; > It would if we didn't set the pointer to nullptr, but since we set it to nu Ah, it seems that was my misunderstanding: I somehow missed the 'X509_INFO *stack_item = sk_X509_INFO_value(info, i)' assignment. Sorry. I think the way it's implemented now is fine. -- To view, visit http://gerrit.cloudera.org:8080/7662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
