Am 15.04.2011 11:45, schrieb Michael McMahon:
Ulf,

On "SystemRoot" vs "systemroot", I had thought that Windows required the sort
to be case-sensitive. But, I double-checked and it doesn't. So we can use 
"SystemRoot".

If that would be true, we wouldn't need to sort by CASE_INSENSITIVE_ORDER or 
rather NameComparator. ;-)

Again my question: What you think of my below code?
I think we only need 1 call site to insert the potentially missing "SystemRoot" variable, a simple array is faster than initializing a needless j.u.List, and boolean foundSysRoot is redundant to int cmp. Additionally I think having the addEnv method is too disproportionate. We can just inline 2 different String-catenations, and it should compile to 3/4 sb.append() under the hood.

So here my shortest version:

    // Only for use by ProcessImpl.start()
    String toEnvironmentBlock() {
        // Sort Unicode-case-insensitively by name
        Map.Entry<String,String>[] list = entrySet().toArray(new 
Map.Entry<>[size()]);
        Arrays.sort(list, entryComparator);

        StringBuilder sb = new StringBuilder(list.length*30);
        for (int i = 0, cmp = -1; ; i++) {
            Map.Entry<String,String> e = i < list.length ? list[i] : null;
            // Some versions of MSVCRT.DLL require SystemRoot to be set:
            final String ROOT = "SystemRoot";
            if (cmp < 0 && (e == null || (cmp = nameComparator.compare(e.getKey(), 
ROOT)) > 0)) {
                String value = getenv(ROOT);
                if (value != null)
                    sb.append(ROOT+'='+value+'\u0000');
                // Ensure double NUL termination, even if environment is empty.
                else if (list.length == 0)
                    sb.append('\u0000');
            }
            if (e == null)  break;
            sb.append(e.getKey()+'='+e.getValue()+'\u0000');
        }
        // Ensure double NUL termination
        return sb.append('\u0000').toString();
    }


-Ulf



Ulf Zibis wrote:
You can have the code much shorter (and faster):

    // Only for use by ProcessImpl.start()
    String toEnvironmentBlock() {
        // Sort Unicode-case-insensitively by name
        Map.Entry<String,String>[] list = entrySet().toArray(new 
Map.Entry<>[size()]);
        Arrays.sort(list, entryComparator);

        StringBuilder sb = new StringBuilder(list.length*30);
        for (int i = 0, cmp = -1; ; i++) {
            Map.Entry<String,String> e = i < list.length ? list[i] : null;
            final String ROOT = "SystemRoot";
            if (cmp < 0 && (e == null || (cmp = nameComparator.compare(e.getKey(), 
ROOT)) > 0)) {
                String value = getenv(ROOT);
                if (value != null)
                    addEnv(sb, ROOT, value);
                // Ensure double NUL termination, even if environment is empty.
                else if (list.length == 0)
                    sb.append('\u0000');
            }
            if (e == null)  break;
            addEnv(sb, e.getKey(), e.getValue());
        }
        // Ensure double NUL termination
        return sb.append('\u0000').toString();
    }

    // add the environment variable to the child
    private static void addEnv(StringBuilder sb, String key, String value) {
        sb.append(key+'='+value+'\u0000'); // under the hood, it compiles to 4 
sb.append()
    }

Do you like it?

Note: I think, if we use NameComparator, we can use "SystemRoot".

-Ulf

Reply via email to