Hi Mikael,

I may have created solinux when the macosx port was merged and in an effort to 
reduce the CPP conditionals.
solinux = solaris + linux  ie. Vanilla unix code vs Darwin code IIRC has 
Objective-C, MacOSX specific thread initialization etc.

I looked over the launcher related code looks ok to me.

I did notice the \n endings for the messages that Brent pointed out.

Best

Kumar Srinivasan






On May 6, 2020, at 9:43 PM, Mikael Vidstedt 
<mikael.vidst...@oracle.com<mailto:mikael.vidst...@oracle.com>> wrote:


I have always wondered what “solinux” is supposed to mean - though not enough 
to actually ask anybody :)

I’ll file a follow-up enhancement to cover renaming the files.

Thank you for the review!

Cheers,
Mikael

On May 4, 2020, at 7:59 AM, Roger Riggs 
<roger.ri...@oracle.com<mailto:roger.ri...@oracle.com>> wrote:

Hi Michael,

Looks good.

Maybe just a future cleanup to rename files, since the "...so..." is refering 
to solaris.

src/java.base/unix/native/libjli/java_md_solinux.h
src/java.base/unix/native/libjli/java_md_solinux.h

Regards, Roger


On 5/4/20 4:49 AM, Alan Bateman wrote:
On 04/05/2020 06:12, Mikael Vidstedt wrote:
Please review this change which implements part of JEP 381:

JBS: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8244224&amp;data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&amp;sdata=W5VvD1owIGoUcbcRkcwixXGPkFLjHUFof2v6cxMqchk%3D&amp;reserved=0
webrev: 
https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~mikael%2Fwebrevs%2F8244224%2Fwebrev.00%2Fcorelibs%2Fopen%2Fwebrev%2F&amp;data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&amp;sdata=tQZIjuF5cIPs%2FNqTRtY2WtmwAgwa0iv205wQ9vk0vOQ%3D&amp;reserved=0
JEP: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8241787&amp;data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434&amp;sdata=rDvBE%2BJ2F1IIFC9fbA92rs0KZNgYPg0JZM2ynqp7mcs%3D&amp;reserved=0


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?

ResolverConfigurationImpl.localDomain0 can be removed.

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

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

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.

-Alan.

Reply via email to