Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12760 )
Change subject: Check result status of Socket::GetPeerAddress in TlsSocket::Recv ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12760/1/src/kudu/security/tls_socket.cc File src/kudu/security/tls_socket.cc: http://gerrit.cloudera.org:8080/#/c/12760/1/src/kudu/security/tls_socket.cc@120 PS1, Line 120: Status s = GetPeerAddress(&remote); : const string remote_str = s.ok() ? remote.ToString() : "unknown"; : string kErrString = Substitute("failed to read from TLS socket (remote: $0)", : remote_str); While you are at it, could you re-factor this piece a bit, so the code would call GetPeerAddress() and construct the error string only in case of errors? I.e., separate that code into some static function that returns string and then directly call that instead of passing pre-computed kErrString into every Status::XxxError() ? -- To view, visit http://gerrit.cloudera.org:8080/12760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd43f30ad11f192463d697f570a997b7e41c7088 Gerrit-Change-Number: 12760 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 15 Mar 2019 00:31:26 +0000 Gerrit-HasComments: Yes
