Todd Lipcon has posted comments on this change.

Change subject: KUDU-1738. Allow users of the client to disable OpenSSL 
initialization
......................................................................


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5992/6/src/kudu/client/client.h
File src/kudu/client/client.h:

PS6, Line 154: NOTE
> doxygen nit: please use @note instead to enable special formatting in the g
Done


http://gerrit.cloudera.org:8080/#/c/5992/6/src/kudu/security/openssl_util.cc
File src/kudu/security/openssl_util.cc:

Line 46: bool g_ssl_is_initialized = false;
> Could you add some notes on the thread safety of these booleans?  My first 
Done


Line 70:   SSL_CTX* ctx = SSL_CTX_new(SSLv23_method());
> This would be simpler as
Done


PS6, Line 80:     return;
> Is it worth adding LOG() statement to inform on this control path branch?  
LOG(INFO) is too noisy since that would show up in clients, and we want clients 
to be quiet in the normal case. (and this is the normal path for something like 
python which has external openssl init). I'll add a VLOG(2)


Line 88:   SSL_CTX* ctx = SSL_CTX_new(SSLv23_method());
> Same.
Done


Line 130:   g_disable_ssl_init = true;
> Consider adding LOG(INFO) about disabling OpenSSL initialization.
see above. Added the VLOG(2) in the "once" site


-- 
To view, visit http://gerrit.cloudera.org:8080/5992
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I43eab5c848b30362356422d0380a336f16587562
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to