Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style
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
Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style
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? 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
Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style
It looks like in TransportContext.java:68, you had a mistype that added "fa" to the end of a comment. 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. 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
Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style
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
