RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

Mikael Vidstedt mikael.vidstedt at oracle.com
Thu May 7 04:38:16 UTC 2020


Alan, thank you for the review! New webrev coming. Meanwhile comments inline..

> On May 4, 2020, at 1:49 AM, Alan Bateman <alan.bateman at oracle.com> wrote:
> 
> On 04/05/2020 06:12, Mikael Vidstedt wrote:
>> Please review this change which implements part of JEP 381:
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>> webrev: http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/corelibs/open/webrev/
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>> 
>> 
>> Note: When reviewing this, please be aware that this exercise was *extremely* mind-numbing, so I appreciate your help reviewing all the individual changes carefully. You may want to get that coffee cup filled up (or whatever keeps you awake)!
>> 
> I took a pass over the changes. I agree its a bit tedious. I'm sure there will be a few follow up issues as there are opportunities for cleanup in several areas. Just a few comments/questions from a first pass.
> 
> ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option that was terminally deprecated in 14. The patch removes the implementation but leave the API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a someone to take a follow-on issue to remove the API?

I would very much appreciate if somebody could take that on!

> ResolverConfigurationImpl.localDomain0 can be removed.

Fixed.

> The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a residual reference to Solaris.

Fixed.

> JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP”.

I was going back and forth on this one - in a way preserving the method was nicely symmetric with getSJISName(), but in the end I removed the method in favor of initializing the variable directly.

> Socket.setTrafficClass(int) swallows exceptions to workaround strange behaviour on Solaris. Tracked as JDK-8221487 so okay to leave it to that issue if you want. There is also cruft in the old plain SocketImpl that to work around eagerness to report "connection reset errors - I think we should just leave that because the old socket impl is not used by default and will be removed at some point.

Anything I can do to keep the complexity of this patch down is appreciated. I have *not* changed anything, let me know if you want me to do something with this.

Cheers,
Mikael



More information about the core-libs-dev mailing list