RFR: 8000975: (process) Merge UNIXProcess.java.bsd & UNIXProcess.java.linux (& .solaris & .aix) (original) (raw)

Martin Buchholz martinrb at google.com
Wed Apr 30 16:25:06 UTC 2014

Belated review.

Peter, thank you very much for taking on this difficult unification task. This is great work! Comments:

It's very sad that ... there's no portable way of getting the name of libarchdir. I thought os.arch system property would suffice, but apparently not on Solaris?

To avoid repetition, I would iniitalize with a vararg list of launch mechanisms, of which the first one is always the default.

LaunchMechanism.VFORK)),

LINUX(LaunchMechanism.VFORK, LaunchMechanism.FORK),

Never use toUpperCase without a Locale, to avoid the "Turkish i problem"

toUpperCase(Locale.ENGLISH)

On Wed, Apr 30, 2014 at 8:17 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

On 25/04/2014 17:47, roger riggs wrote:

Hi Peter,

Including the test update with the updated changeset is fine. (I think Alan had some comments on the refactoring and has not yet had a chance to comment). Thanks, Roger I reviewed previous rounds and you've addressed my points so I think I'm mostly happy with this. As some point I think we should look at Platform again as there may be an opportunity later in JDK 9 to move it out of UNIXProcess. One thing that I wasn't sure about is the additions to the @author tags as we've mostly been trying not to grow these (contentious topic, I don't have a strong opinion as I never use @author). A minor comment looking at the latest webrev.07 is that some of the lines are very long. It's not a problem now but I could image future side-by-side reviews needing a scroll bar.DeferredCloseProcessPipeInputStream is very long too. -Alan.

More information about the core-libs-dev mailing list