On Tue, 27 Apr 2021 23:48:23 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:

>> Xue-Lei Andrew Fan has updated the pull request with a new target base due 
>> to a merge or a rebase. The incremental webrev excludes the unrelated 
>> changes brought in by the merge/rebase. The pull request contains three 
>> additional commits since the last revision:
>> 
>>  - Merge
>>  - improved fix
>>  - 8263779: SSLEngine reports NEED_WRAP continuously without producing any 
>> further output
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1694:
> 
>> 1692:         if (cause instanceof SocketException) {
>> 1693:             try {
>> 1694:                 throw conContext.fatal(alert, cause);
> 
> Why did you change to a throw here?  Isn't everything from fatal() going to 
> be thrown anyway?

The fatal() will always throw an exception.  We could use the method without 
throw.  In some places, IDE or compiler could complain because it cannot detect 
that the fatal() method will throw and the program ends here.  In the past, we 
use fatal() method without throw, later we change the fatal() return value form 
"void" to an exception so that throw could be used to mitigate the IDE or 
compiler complaints.

It is not really necessary to make this code change here.  I just want to make 
the coding style consistent for the fatal() calling.

> src/java.base/share/classes/sun/security/ssl/TransportContext.java line 587:
> 
>> 585:     }
>> 586: 
>> 587:     // Note: HandshakeStatus.FINISHED status is retrieved in other 
>> places.
> 
> I now see that v1 of this change had quite a few changes here, and what's 
> left is now just compacting a line and making some old comment corrections.

Yes.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3292

Reply via email to