Alexey Serbin has posted comments on this change. Change subject: Move SSLFactory and SSLSocket to the security module ......................................................................
Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/5703/5//COMMIT_MSG Commit Message: PS5, Line 9: Soon these classes will rely on the security module Is that about RPC-related ones? If so, consider re-phrasing a bit. http://gerrit.cloudera.org:8080/#/c/5703/5/src/kudu/security/ssl_socket.h File src/kudu/security/ssl_socket.h: PS5, Line 17: #ifndef KUDU_UTIL_NET_SSL_SOCKET_H : #define KUDU_UTIL_NET_SSL_SOCKET_H Since you are replacing include guards with pragma once, consider doing this here as well. Otherwise, consider updating the name of the guard to reflect the new location. http://gerrit.cloudera.org:8080/#/c/5703/4/src/kudu/security/x509_check_host.cc File src/kudu/security/x509_check_host.cc: Line 14: #include <stdio.h> > I think this was purposefully included after the openssl library, and used This file is not going to be compiled in case of the newer version of the OpenSSL library, so it's not relevant. http://gerrit.cloudera.org:8080/#/c/5703/5/src/kudu/security/x509_check_host.h File src/kudu/security/x509_check_host.h: PS5, Line 12: #ifndef X509_CHECK_HOST_H : #define X509_CHECK_HOST_H Is this intentionally set to X509_CHECK_HOST_H to be the same as the guard in some other OpenSSL file? If not, consider replacing with pragma once. -- To view, visit http://gerrit.cloudera.org:8080/5703 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I100966cfd51435e2954459fce79baa7cf6da4dcb Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
