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

@@ -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<jchar*>(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<int>(OSversion.dwMajorVersion),
+        static_cast<int>(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<jchar*>(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<jchar*>(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

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<String, String> 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?

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<uintptr_t>(array);
 }
 

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<object>(arguments[0]);
-  object found = reinterpret_cast<object>(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<int64_t>
-      (makeString(t, "%s", t->m->appFinder->path()));
-  } else if (::strcmp(RUNTIME_ARRAY_BODY(n), "avian.version") == 0) {
-    r = reinterpret_cast<int64_t>(makeString(t, AVIAN_VERSION));
-  } else if (::strcmp(RUNTIME_ARRAY_BODY(n), "file.encoding") == 0) {
-    r = reinterpret_cast<int64_t>(makeString(t, "ASCII"));
-  } else {
-    const char* v = findProperty(t, RUNTIME_ARRAY_BODY(n));
-    if (v) {
-      r = reinterpret_cast<int64_t>(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<int64_t>(array);
 }
 
 extern "C" AVIAN_EXPORT void JNICALL

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<const char**>
     (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);

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 { }