Review request for 7034570 - java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited

David Holmes David.Holmes at oracle.com
Fri Apr 15 01:03:33 UTC 2011


Hi Michael,

Michael McMahon said the following on 04/15/11 00:06:
> An updated webrev is available for this fix. I'll probably go ahead
> with the CCC request for the spec. change anyway.
> 
> http://cr.openjdk.java.net/~michaelm/7034570/webrev.2/

Based on Alan's comments I checked for and found webrev.3 so that's what 
I reviewed :)

Minor nit:

+     private final static String systemroot = "systemroot";

constant names are usually all-caps.

So if I understand the logic here, if we find SystemRoot in the passed 
in environment then we simply use that for the child. Else we check if 
it is set in the parents environment, and if so we add that to the 
childs environment. Else neither parent nor child have it. So my only 
nit with this is that addEnv(sb, systemroot) gives the initial 
appearance of adding to the environment when in fact it only 
conditionally adds. Perhaps addToEnvIfSet(sb, systemroot) ?

Minor nit:

+           .append("=")

could be a char instead of String:

+           .append('=')


Why do we append the NUL in addEnv when we do that at the end of the 
method anyway? Does it require double termination?

David

> 
> Thanks,
> Michael.



More information about the core-libs-dev mailing list