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

Ulf Zibis Ulf.Zibis at gmx.de
Mon Apr 18 13:10:39 UTC 2011


Am 16.04.2011 12:48, schrieb Alan Bateman:
> Michael McMahon wrote:
>> I have incorporated much of Ulf's approach into this version.
>>
>> I agree with Alan about the spec. We could find other variables in the future that need
>> to be set, but can have alternative values other than the default. I think this approach
>> is the most flexible.
>>
>> http://cr.openjdk.java.net/~michaelm/7034570/webrev.4/
> This looks much better - thanks Ulf for the improved loop and changing it to use nameComparator. 
> It will be good to get this one fixed.
>

Thanks for the partial acceptance.

I think, the comment in lines 295..297 more belongs to the code (lines 312..315 + 318..320 ) rather 
than to the constant SYSTEMROOT. Why defining it as class member, as it is only locally referred?

I still don't see the advantage, having the SYSTEMROOT insertion code at 2 call sites. One would be 
shorter and much clearer as shown in my code. Then we too could save addToEnvIfSet(..) and the call 
to it.

Additionally: Why instantiating, initializing and processing a fat j.u.List object, if a simple 
array would fulfil all needs?

You have removed the 'Ensure double NUL termination' code. I guess, you have approved, that 
String->CString already handles this, right?

-Ulf



More information about the core-libs-dev mailing list