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.
