From 8f4c0e78ce24e14d0a152a7737395fe8c5716b5e Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Fri, 4 Apr 2014 11:10:38 -0600 Subject: [PATCH] clean up System.getProperties and related methods The behavior of Avian's versions of these methods was egregiously non-standard, and there were problems with the Android implementations as well. --- classpath/java-lang.cpp | 293 ++++++++++++++++++++------------ classpath/java/lang/System.java | 72 +++----- src/classpath-android.cpp | 16 +- src/classpath-avian.cpp | 35 +--- src/jnienv.cpp | 28 ++- test/Misc.java | 28 +++ 6 files changed, 281 insertions(+), 191 deletions(-) diff --git a/classpath/java-lang.cpp b/classpath/java-lang.cpp index 4d1a280e61..acdcfac7bc 100644 --- a/classpath/java-lang.cpp +++ b/classpath/java-lang.cpp @@ -80,8 +80,54 @@ #endif // WINAPI_FAMILY namespace { + +void add(JNIEnv* e, jobjectArray array, unsigned index, const char* format, ...) +{ + int size = 256; + while (true) { + va_list a; + va_start(a, format); + RUNTIME_ARRAY(char, buffer, size); + int r = vsnprintf(RUNTIME_ARRAY_BODY(buffer), size - 1, format, a); + va_end(a); + if (r >= 0 and r < size - 1) { + e->SetObjectArrayElement( + array, index++, e->NewStringUTF(RUNTIME_ARRAY_BODY(buffer))); + return; + } + + size *= 2; + } +} + #ifdef PLATFORM_WINDOWS +void add(JNIEnv* e, + jobjectArray array, + unsigned index, + const WCHAR* format, + ...) +{ + int size = 256; + while (true) { + va_list a; + va_start(a, format); + RUNTIME_ARRAY(WCHAR, buffer, size); + int r = _vsnwprintf(RUNTIME_ARRAY_BODY(buffer), size - 1, format, a); + va_end(a); + if (r >= 0 and r < size - 1) { + e->SetObjectArrayElement( + array, + index++, + e->NewString(reinterpret_cast(RUNTIME_ARRAY_BODY(buffer)), + r)); + return; + } + + size *= 2; + } +} + #if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) char* getErrorStr(DWORD err) { LPSTR errorStr = 0; @@ -641,143 +687,166 @@ Locale getLocale() { } #endif -extern "C" JNIEXPORT jstring JNICALL -Java_java_lang_System_getProperty(JNIEnv* e, jclass, jstring name, - jbooleanArray found) +extern "C" JNIEXPORT jobjectArray JNICALL + Java_java_lang_System_getNativeProperties(JNIEnv* e, jclass) { - jstring r = 0; - const char* chars = e->GetStringUTFChars(name, 0); - if (chars) { -#ifdef PLATFORM_WINDOWS - if (strcmp(chars, "line.separator") == 0) { - r = e->NewStringUTF("\r\n"); - } else if (strcmp(chars, "file.separator") == 0) { - r = e->NewStringUTF("\\"); - } else if (strcmp(chars, "path.separator") == 0) { - r = e->NewStringUTF(";"); - } else if (strcmp(chars, "os.name") == 0) { + jobjectArray array + = e->NewObjectArray(32, e->FindClass("java/lang/String"), 0); + + unsigned index = 0; + +#ifdef ARCH_x86_32 + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.arch=x86")); + +#elif defined ARCH_x86_64 + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.arch=x86_64")); + +#elif defined ARCH_powerpc + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.arch=ppc")); + +#elif defined ARCH_arm + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.arch=arm")); + +#endif + +#ifdef PLATFORM_WINDOWS + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("line.separator=\r\n")); + + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("file.separator=\\")); + + e->SetObjectArrayElement(array, index++, e->NewStringUTF("path.separator=;")); + # if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) - r = e->NewStringUTF("Windows"); + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.name=Windows")); + # elif WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE) - r = e->NewStringUTF("Windows Phone"); + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("os.name=Windows Phone")); + # else - r = e->NewStringUTF("Windows RT"); + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("os.name=Windows RT")); + # endif - } else if (strcmp(chars, "os.version") == 0) { + # if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) - unsigned size = 32; - RUNTIME_ARRAY(char, buffer, size); - OSVERSIONINFO OSversion; - OSversion.dwOSVersionInfoSize=sizeof(OSVERSIONINFO); - ::GetVersionEx(&OSversion); - snprintf(RUNTIME_ARRAY_BODY(buffer), size, "%i.%i", (int)OSversion.dwMajorVersion, (int)OSversion.dwMinorVersion); - r = e->NewStringUTF(RUNTIME_ARRAY_BODY(buffer)); + { + OSVERSIONINFO OSversion; + OSversion.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + ::GetVersionEx(&OSversion); + + add(e, + array, + index++, + "os.version=%i.%i", + static_cast(OSversion.dwMajorVersion), + static_cast(OSversion.dwMinorVersion)); + } + # else - // Currently there is no alternative on WinRT/WP8 - r = e->NewStringUTF("8.0"); + // Currently there is no alternative on WinRT/WP8 + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.version=8.0")); + # endif - } else if (strcmp(chars, "os.arch") == 0) { -#ifdef ARCH_x86_32 - r = e->NewStringUTF("x86"); -#elif defined ARCH_x86_64 - r = e->NewStringUTF("x86_64"); -#elif defined ARCH_powerpc - r = e->NewStringUTF("ppc"); -#elif defined ARCH_arm - r = e->NewStringUTF("arm"); -#endif - } else if (strcmp(chars, "java.io.tmpdir") == 0) { + # if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) - TCHAR buffer[MAX_PATH]; - GetTempPath(MAX_PATH, buffer); - r = e->NewStringUTF(buffer); + { + WCHAR buffer[MAX_PATH]; + GetTempPathW(MAX_PATH, buffer); + add(e, array, index++, L"java.io.tmpdir=%ls", buffer); + } + # else - std::wstring tmpDir = AvianInterop::GetTemporaryFolder(); - r = e->NewString((const jchar*)tmpDir.c_str(), tmpDir.length()); + add(e, + array, + index++, + L"java.io.tmpdir=%ls", + AvianInterop::GetTemporaryFolder().c_str()); + # endif - } else if (strcmp(chars, "user.dir") == 0) { + # if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) - TCHAR buffer[MAX_PATH]; - GetCurrentDirectory(MAX_PATH, buffer); - r = e->NewStringUTF(buffer); + { + WCHAR buffer[MAX_PATH]; + GetCurrentDirectoryW(MAX_PATH, buffer); + add(e, array, index++, L"user.dir=%ls", buffer); + } + # else - std::wstring userDir = AvianInterop::GetInstalledLocation(); - r = e->NewString((const jchar*)userDir.c_str(), userDir.length()); + add(e, + array, + index++, + L"user.dir=%ls", + AvianInterop::GetInstalledLocation().c_str()); + # endif - } else if (strcmp(chars, "user.home") == 0) { -# ifdef _MSC_VER + # if !defined(WINAPI_FAMILY) || WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) - WCHAR buffer[MAX_PATH]; - size_t needed; - if (_wgetenv_s(&needed, buffer, MAX_PATH, L"USERPROFILE") == 0) { - r = e->NewString(reinterpret_cast(buffer), lstrlenW(buffer)); - } else { - r = 0; - } +#ifdef _MSC_VER + { + WCHAR buffer[MAX_PATH]; + size_t needed; + if (_wgetenv_s(&needed, buffer, MAX_PATH, L"USERPROFILE") == 0) { + add(e, array, index++, L"user.home=%ls", buffer); + } + } + +#else + add(e, array, index++, L"user.home=%ls", _wgetenv(L"USERPROFILE")); + +#endif # else - std::wstring userHome = AvianInterop::GetDocumentsLibraryLocation(); - r = e->NewString((const jchar*)userHome.c_str(), userHome.length()); + add(e, + array, + index++, + L"user.home=%ls", + AvianInterop::GetDocumentsLibraryLocation().c_str()); + # endif -# else - LPWSTR home = _wgetenv(L"USERPROFILE"); - r = e->NewString(reinterpret_cast(home), lstrlenW(home)); -# endif - } -#else - if (strcmp(chars, "line.separator") == 0) { - r = e->NewStringUTF("\n"); - } else if (strcmp(chars, "file.separator") == 0) { - r = e->NewStringUTF("/"); - } else if (strcmp(chars, "path.separator") == 0) { - r = e->NewStringUTF(":"); - } else if (strcmp(chars, "os.name") == 0) { + +#else // not Windows + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("line.separator=\n")); + + e->SetObjectArrayElement(array, index++, e->NewStringUTF("file.separator=/")); + + e->SetObjectArrayElement(array, index++, e->NewStringUTF("path.separator=:")); + #ifdef __APPLE__ - r = e->NewStringUTF("Mac OS X"); + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.name=Mac OS X")); + #elif defined __FreeBSD__ - r = e->NewStringUTF("FreeBSD"); + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.name=FreeBSD")); + #else - r = e->NewStringUTF("Linux"); -#endif - } else if (strcmp(chars, "os.version") == 0) { - struct utsname system_id; - uname(&system_id); - r = e->NewStringUTF(system_id.release); - } else if (strcmp(chars, "os.arch") == 0) { -#ifdef ARCH_x86_32 - r = e->NewStringUTF("x86"); -#elif defined ARCH_x86_64 - r = e->NewStringUTF("x86_64"); -#elif defined ARCH_powerpc - r = e->NewStringUTF("ppc"); -#elif defined ARCH_arm - r = e->NewStringUTF("arm"); -#endif - } else if (strcmp(chars, "java.io.tmpdir") == 0) { - r = e->NewStringUTF("/tmp"); - } else if (strcmp(chars, "user.dir") == 0) { - char buffer[PATH_MAX]; - r = e->NewStringUTF(getcwd(buffer, PATH_MAX)); - } else if (strcmp(chars, "user.home") == 0) { - r = e->NewStringUTF(getenv("HOME")); - } -#endif - else if (strcmp(chars, "user.language") == 0) { - Locale locale = getLocale(); - if (strlen(locale.getLanguage())) r = e->NewStringUTF(locale.getLanguage()); - } else if (strcmp(chars, "user.region") == 0) { - Locale locale = getLocale(); - if (strlen(locale.getRegion())) r = e->NewStringUTF(locale.getRegion()); - } + e->SetObjectArrayElement(array, index++, e->NewStringUTF("os.name=Linux")); - e->ReleaseStringUTFChars(name, chars); +#endif + { + struct utsname system_id; + uname(&system_id); + add(e, array, index++, "os.version=%s", system_id.release); } - if (r) { - jboolean v = true; - e->SetBooleanArrayRegion(found, 0, 1, &v); + e->SetObjectArrayElement( + array, index++, e->NewStringUTF("java.io.tmpdir=/tmp")); + + { + char buffer[PATH_MAX]; + add(e, array, index++, "user.dir=%s", getcwd(buffer, PATH_MAX)); } - return r; +#endif // not Windows + + { + Locale locale = getLocale(); + add(e, array, index++, "user.language=%s", locale.getLanguage()); + add(e, array, index++, "user.region=%s", locale.getRegion()); + } + + return array; } // System.getEnvironment() implementation diff --git a/classpath/java/lang/System.java b/classpath/java/lang/System.java index 51a1839676..8ef45f5441 100644 --- a/classpath/java/lang/System.java +++ b/classpath/java/lang/System.java @@ -24,7 +24,10 @@ import java.util.Properties; public abstract class System { private static final long NanoTimeBaseInMillis = currentTimeMillis(); - private static Property properties; + private static class Static { + public static Properties properties = makeProperties(); + } + private static Map environment; private static SecurityManager securityManager; @@ -45,20 +48,7 @@ public abstract class System { int dstOffset, int length); public static String getProperty(String name) { - for (Property p = properties; p != null; p = p.next) { - if (p.name.equals(name)) { - return p.value; - } - } - - boolean[] found = new boolean[1]; - String value = getProperty(name, found); - if (found[0]) return value; - - value = getVMProperty(name, found); - if (found[0]) return value; - - return null; + return (String) Static.properties.get(name); } public static String getProperty(String name, String defaultValue) { @@ -69,31 +59,35 @@ public abstract class System { return result; } - public static String setProperty(String name, String value) { - for (Property p = properties; p != null; p = p.next) { - if (p.name.equals(name)) { - String oldValue = p.value; - p.value = value; - return oldValue; - } - } - - properties = new Property(name, value, properties); - return null; + return (String) Static.properties.put(name, value); } - public static Properties getProperties () { - Properties prop = new Properties(); - for (Property p = properties; p != null; p = p.next) { - prop.put(p.name, p.value); + public static Properties getProperties() { + return Static.properties; + } + + private static Properties makeProperties() { + Properties properties = new Properties(); + + for (String p: getNativeProperties()) { + if (p == null) break; + int index = p.indexOf('='); + properties.put(p.substring(0, index), p.substring(index + 1)); } - return prop; + + for (String p: getVMProperties()) { + if (p == null) break; + int index = p.indexOf('='); + properties.put(p.substring(0, index), p.substring(index + 1)); + } + + return properties; } - private static native String getProperty(String name, boolean[] found); + private static native String[] getNativeProperties(); - private static native String getVMProperty(String name, boolean[] found); + private static native String[] getVMProperties(); public static native long currentTimeMillis(); @@ -137,18 +131,6 @@ public abstract class System { System.securityManager = securityManager; } - private static class Property { - public final String name; - public String value; - public final Property next; - - public Property(String name, String value, Property next) { - this.name = name; - this.value = value; - this.next = next; - } - } - public static String getenv(String name) throws NullPointerException, SecurityException { if (getSecurityManager() != null) { // is this allowed? diff --git a/src/classpath-android.cpp b/src/classpath-android.cpp index 0966755984..e3ad8a58d1 100644 --- a/src/classpath-android.cpp +++ b/src/classpath-android.cpp @@ -1288,13 +1288,21 @@ extern "C" AVIAN_EXPORT int64_t JNICALL Avian_dalvik_system_VMRuntime_properties (Thread* t, object, uintptr_t*) { - object array = makeObjectArray(t, type(t, Machine::StringType), 1); + object array = makeObjectArray( + t, type(t, Machine::StringType), t->m->propertyCount + 1); PROTECT(t, array); - object property = makeString(t, "java.protocol.handler.pkgs=avian"); + unsigned i; + for (i = 0; i < t->m->propertyCount; ++i) { + object s = makeString(t, "%s", t->m->properties[i]); + set(t, array, ArrayBody + (i * BytesPerWord), s); + } + + { + object s = makeString(t, "%s", "java.protocol.handler.pkgs=avian"); + set(t, array, ArrayBody + (i++ * BytesPerWord), s); + } - set(t, array, ArrayBody, property); - return reinterpret_cast(array); } diff --git a/src/classpath-avian.cpp b/src/classpath-avian.cpp index 217333659b..64c996a611 100644 --- a/src/classpath-avian.cpp +++ b/src/classpath-avian.cpp @@ -507,37 +507,18 @@ Avian_java_lang_String_intern } extern "C" AVIAN_EXPORT int64_t JNICALL -Avian_java_lang_System_getVMProperty -(Thread* t, object, uintptr_t* arguments) + Avian_java_lang_System_getVMProperties(Thread* t, object, uintptr_t*) { - object name = reinterpret_cast(arguments[0]); - object found = reinterpret_cast(arguments[1]); - PROTECT(t, found); + object array + = makeObjectArray(t, type(t, Machine::StringType), t->m->propertyCount); + PROTECT(t, array); - unsigned length = stringLength(t, name); - THREAD_RUNTIME_ARRAY(t, char, n, length + 1); - stringChars(t, name, RUNTIME_ARRAY_BODY(n)); - - int64_t r = 0; - if (::strcmp(RUNTIME_ARRAY_BODY(n), "java.lang.classpath") == 0) { - r = reinterpret_cast - (makeString(t, "%s", t->m->appFinder->path())); - } else if (::strcmp(RUNTIME_ARRAY_BODY(n), "avian.version") == 0) { - r = reinterpret_cast(makeString(t, AVIAN_VERSION)); - } else if (::strcmp(RUNTIME_ARRAY_BODY(n), "file.encoding") == 0) { - r = reinterpret_cast(makeString(t, "ASCII")); - } else { - const char* v = findProperty(t, RUNTIME_ARRAY_BODY(n)); - if (v) { - r = reinterpret_cast(makeString(t, v)); - } - } - - if (r) { - booleanArrayBody(t, found, 0) = true; + for (unsigned i = 0; i < t->m->propertyCount; ++i) { + object s = makeString(t, "%s", t->m->properties[i]); + set(t, array, ArrayBody + (i * BytesPerWord), s); } - return r; + return reinterpret_cast(array); } extern "C" AVIAN_EXPORT void JNICALL diff --git a/src/jnienv.cpp b/src/jnienv.cpp index 1dcd9e6769..6872d775f1 100644 --- a/src/jnienv.cpp +++ b/src/jnienv.cpp @@ -3875,9 +3875,13 @@ JNI_CreateJavaVM(Machine** m, Thread** t, void* args) if (heapLimit == 0) heapLimit = 128 * 1024 * 1024; if (stackLimit == 0) stackLimit = 128 * 1024; - - if (classpath == 0) classpath = "."; - + + bool addClasspathProperty = classpath == 0; + if (addClasspathProperty) { + classpath = "."; + ++propertyCount; + } + System* s = makeSystem(); Heap* h = makeHeap(s, heapLimit); Classpath* c = makeClasspath(s, h, javaHome, embedPrefix); @@ -3915,6 +3919,9 @@ JNI_CreateJavaVM(Machine** m, Thread** t, void* args) free(bootLibrary); Processor* p = makeProcessor(s, h, crashDumpDirectory, true); + // reserve space for avian.version and file.encoding: + propertyCount += 2; + const char** properties = static_cast (h->allocate(sizeof(const char*) * propertyCount)); @@ -3932,6 +3939,21 @@ JNI_CreateJavaVM(Machine** m, Thread** t, void* args) *(argumentPointer++) = a->options[i].optionString; } + unsigned cpl = strlen(classpath); + RUNTIME_ARRAY(char, classpathProperty, cpl + sizeof(CLASSPATH_PROPERTY) + 1); + if (addClasspathProperty) { + char* p = RUNTIME_ARRAY_BODY(classpathProperty); + local::append(&p, CLASSPATH_PROPERTY, sizeof(CLASSPATH_PROPERTY), '='); + local::append(&p, classpath, cpl, 0); + *(propertyPointer++) = RUNTIME_ARRAY_BODY(classpathProperty); + } + + *(propertyPointer++) = "avian.version=" AVIAN_VERSION; + + // todo: should this be derived from the OS locale? Should it be + // overrideable via JavaVMInitArgs? + *(propertyPointer++) = "file.encoding=UTF-8"; + *m = new (h->allocate(sizeof(Machine))) Machine (s, h, bf, af, p, c, properties, propertyCount, arguments, a->nOptions, stackLimit); diff --git a/test/Misc.java b/test/Misc.java index 22f7806b9e..274a7a0335 100644 --- a/test/Misc.java +++ b/test/Misc.java @@ -313,6 +313,34 @@ public class Misc { expect(! staticRan); Static.run(); expect(staticRan); + + expect(System.getProperty("java.class.path").equals + (System.getProperties().get("java.class.path"))); + + expect(System.getProperty("path.separator").equals + (System.getProperties().get("path.separator"))); + + expect(System.getProperty("user.dir").equals + (System.getProperties().get("user.dir"))); + + expect(System.getProperty("java.io.tmpdir").equals + (System.getProperties().get("java.io.tmpdir"))); + + System.setProperty("buzzy.buzzy.bim.bam", "dippy dopey flim flam"); + + expect(System.getProperty("buzzy.buzzy.bim.bam").equals + (System.getProperties().get("buzzy.buzzy.bim.bam"))); + + expect(System.getProperty("buzzy.buzzy.bim.bam").equals + ("dippy dopey flim flam")); + + System.getProperties().put("buzzy.buzzy.bim.bam", "yippy yappy yin yang"); + + expect(System.getProperty("buzzy.buzzy.bim.bam").equals + (System.getProperties().get("buzzy.buzzy.bim.bam"))); + + expect(System.getProperty("buzzy.buzzy.bim.bam").equals + ("yippy yappy yin yang")); } protected class Protected { }