My nit is I’d remove the “make compiler happy” comment. If this was something for Xcomp, I’d understand, but this is standard return value requirement.
Otherwise I’m fine with this Tony > On Dec 14, 2018, at 11:49 AM, Jamil Nimeh <jamil.j.ni...@oracle.com> wrote: > > Looks pretty good. I did have one question about a few of the methods in > KeyShareExtension and PreSharedKeyExtension, specifically where you return > nulls on error branches with the make-compiler-happy comments. In those > cases would it be a bit cleaner to set a byte[] variable to null at the > beginning of the method and then in the successful code path set the value to > whatever byte array comes back from hmacs or hkdf operations? At the end of > the method you only need one return statement, rather than a return null on > every error branch, many of which won't be executed due to method calls which > always throw exceptions. Not a big deal if you wish to leave things as they > are since I know the current approach is used in many places in the code that > this review doesn't touch. > > --Jamil > >> On 12/14/2018 8:14 AM, Xue-Lei Fan wrote: >> Hi, >> >> Please review the update: >> http://cr.openjdk.java.net/~xuelei/8214339/webrev.00/ >> >> In some cases, the SSLProtocolException or SSLHandshakeException may be >> thrown if the underlying socket run into problems. An application may >> depends on the exception class for further action, for example retry the >> connection with different parameters. >> >> This update is trying to separate the socket problem from the TLS protocol >> or handshake problem, by using different exception classes. >> >> Thanks, >> Xuelei >
