On 12/17/18 5:26 PM, Xue-Lei Fan wrote:
On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
It looks like in TransportContext.java:68, you had a mistype that
added "fa" to the end of a comment.
Oops, I will update it.
Also in fatal():267, did you plan to return the exception and have the
calling method throw the exception? As is, the exception is never
return and fatal() continues to throw the exceptions.
I considered to return the exception. I'm not very confident with if I
searched out all use of the fatal() methods. For safe, I kept to use
thrown exception instead. If the method is updated later that there is a
case that now exception get thrown, there will be a compiling issue. Are
you OK with it?
Since the stacktrace is based on the exception's creation and not where
it's thrown, I guess there's no reason to return the value. As you have
it now should be fine.
Tony
Thanks,
Xuelei
Tony
On 12/15/18 7:51 AM, Xue-Lei Fan wrote:
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8215443/webrev.00/
The TransportContext.fatal() methods always throw exception. While
the compiler does not aware of it, and may not happy without
following a return statement. Currently, a lot never executable
return statements are inserted. As make the code hard to read
(thanks for Jamil and Tony's points). For example:
shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
return null; // fatal() always throws, make the compiler happy.
In this update, I changed the fatal() method with a return value:
- void fatal(Alert alert, ...
+ SSLException fatal(Alert alert, ...
Then we can change the use of method as:
- shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
- return null; // fatal() always throws, make the compiler happy.
+ throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
The changeset is mostly about removing the never executed return
statements and add the 'throw' keyword to lines that use the fatal()
methods.
Thanks,
Xuelei
