Ulf Zibis wrote:
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?
It makes no difference to the generated code. And imo, there is more
space for the comment there.
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.
Shorter doesn't always mean clearer. I think this way is clearer.
Additionally: Why instantiating, initializing and processing a fat
j.u.List object, if a simple array would fulfil all needs?
I don't see a compelling reason to change it from a List (which is what
it was). I think any additional overhead
is inconsequential in the context of creating an OS process. Or if it
is, then we should be looking at
our List implementations.
You have removed the 'Ensure double NUL termination' code. I guess,
you have approved, that String->CString already handles this, right?
Actually no. That shouldn't have been omitted. Thanks for finding that!
- Michael.
