Ulf,
Thanks for the comments. I think I will remove the @SuppressWarning for now,
and we can look at generifying CheckedEntrySet another time.
The environment block is required to be sorted when you call CreateProcess()
on Windows.
Yes, as per the earlier message to David Holmes, I'm looking at
iterating once
over the list (rather than the Set).
Lastly, even though environment variables are case-insensitive on
Windows, it
does "remember" the case that was used (rather than normalising the
names to uppercase
or whatever). Java has always treated them as case-sensitive. So, I
don't think
we can convert the names to uppercase.
- Michael.
On 13/04/2011 15:20, Ulf Zibis wrote:
First: please add some more description to the subject line.
ProcessEnvironment :
Line 146: There is a superfluous space between 'checkedEntry' and
'(Object o)'.
Instead of suppressing the warnings, we could code:
public boolean contains(Map.Entry<String,String> o) {return
s.contains(checkedEntry(o));}
public boolean remove(Map.Entry<String,String> o) {return
s.remove(checkedEntry(o));}
Maybe we could generify the whole inner class against CheckedEntry:
private static class CheckedEntrySet extends
AbstractSet<CheckedEntry> {
I don't know why we need the environment block sorted, but I think, we
could use a SortedSet from the beginning instead of sorting a normal
Set later.
Iterating over the List list should be faster than iterating over the
Set entries to find the "SystemRoot".
Additionally I guess, TreeSet should be faster than HashSet for such
few entries.
Anyway, what about:
305 EnsureSystemRoot: {
306 final String SR = "SystemRoot";
307 for (Map.Entry<String,String> entry : list) {
308 if (entry.getKey().equalsIgnoreCase(SR))
309 break EnsureSystemRoot;
310 }
311 list.add(new
AbstractMap.SimpleEntry<String,String>(SR, getenv(SR)));
312 }
318
We do not have to iterate twice, to find the "SystemRoot". We could
insert the missing value while filling the StringBuilder
If we would generally uppercase the entries by put(), we wouldn't have
to scan each entry by equalsIgnoreCase() to find the "SystemRoot". We
simply could use entries.contains("SYSTEMROOT")
-Ulf
Am 13.04.2011 13:33, schrieb Michael McMahon:
This issue occurs on some versions of Windows since we switched compiler
to VC 2010. The new C runtime library requires the "SystemRoot"
environment
variable to be set, when launching sub-processes via Runtime.exec()
or ProcessBuilder.
The problem occurs in instances where the environment is specified by
the caller,
rather than simply inheriting (or inheriting and adding to) the
environment of the parent
process.
The change is simple enough. We just check for the presence of
"SystemRoot" in the
child process's environment, and set it if it's not there. No change
will be visible
to the parent process. But, to avoid ambiguity, we'd like to make the
change explicit
with a small spec. change.
http://cr.openjdk.java.net/~michaelm/7034570/webrev.1/
Thanks,
Michael.
