Alan, thank you for the review! New webrev coming. Meanwhile comments inline..
> On May 4, 2020, at 1:49 AM, Alan Bateman <alan.bate...@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
