Todd Lipcon has posted comments on this change. Change subject: tls_socket: fix handling of syscall errors ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5954/1/src/kudu/security/tls_socket.cc File src/kudu/security/tls_socket.cc: Line 127: if (error_code == SSL_ERROR_SYSCALL && ERR_peek_error() == 0) { > Doesn't this have some overlap with GetSSLErrorDescription() in security/op I don't think it can really be consolidated, because we also are relying on the 'bytes_read' value to stringify it here. The docs for this particular error code seem to be specific to SSL_read() rather than a generic way to handle the code. Line 137: return Status::NetworkError("connection reset by peer", "", ECONNRESET); > May want to make this match the others, or have the others match this. e.g k, I'll change them to try to be more consistent and just try to distinguish SSL-specific errors from more generic socket errors. -- To view, visit http://gerrit.cloudera.org:8080/5954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9a0a63f861d71bd3186567ff98148476795530ab Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
