Thanks the advice, I'll raise JIRA and consolidate the patch.

Nathan Beyer wrote:
The only thing I'd consider would be doing a case-insensitive compare,
perhaps by doing the following.

osName = (osName == null ? null : osName.toLowerCase(Locale.ENGLISH));
// only comparing ASCII, so assume english localge

Also, need to check for null.

if (osName != null && osName.startsWith("windows"))

-Nathan

On Wed, Dec 31, 2008 at 12:21 AM, Regis <[email protected]> wrote:
Thanks Nathan! So the code could be changed to:

            String osName = AccessController.doPrivileged(new
PrivilegedAction<String>() {
                public String run() {
                    return System.getProperty("os.name"); //$NON-NLS-1$
                }
            });

            if (osName.startsWith("Windows")) {
                factoryClassName =
"java.util.prefs.RegistryPreferencesFactoryImpl";
            } else {
                factoryClassName =
"java.util.prefs.FilePreferencesFactoryImpl";
            }



Nathan Beyer wrote:
On Tue, Dec 30, 2008 at 11:54 PM, Regis <[email protected]> wrote:
Yes, but the value of "os.name" is human readable string, it must be
parsed
by hand, I'm not sure the format of the string is consistent on different
vm
or different versions of Windows, so I used File.pathSeparatorChar, as I
know only windows use "\\" as patch separator.
According to what source is "os.name" to be used for human
readability? It's not guaranteed to be anything, but I and many, many,
many others have written code that utilizes it, as well as the
correlated property, "os.arch". Additionally, "os.name" and "os.arch"
are used as part of JNLP to define platform-specific resources [1].

Here's a few sites that have attempted to catalog the various values,
so as to allow for utilizing the values.
http://mindprod.com/jgloss/properties.html#OSNAME
http://lopica.sourceforge.net/os.html

-Nathan

[1]
http://java.sun.com/j2se/1.5.0/docs/guide/javaws/developersguide/syntax.html#resources


Nathan Beyer wrote:
The System Property 'os.name' would be more appropriate for determining
the OS.

-Nathan

On Tue, Dec 30, 2008 at 11:32 PM, Regis <[email protected]> wrote:
Nathan Beyer wrote:
I doubt there is any significant reason for it other than the default
is based on the OS and the native might have been an easy decision
point.
Maybe.
Moving it to Java code is probably fine - the code just needs to use a
default based on the OS.
I try the following patch, all tests are passed, so I think we could
move
it
to java code safely. I used File.pathSeparatorChar to test the
platform,
is
there any better way to do this?


Index: modules/luni/src/main/native/luni/shared/luniglob.c
=====================================================================
--- modules/luni/src/main/native/luni/shared/luniglob.c
+++ modules/luni/src/main/native/luni/shared/luniglob.c
@@ -162,21 +162,6 @@ JNI_OnLoad (JavaVM * vm, void *reserved)
         }
     }

-       /* Set default PreferencesFactory implementation */
-       (*vmInterface)->GetSystemProperty (vmInterface,
"java.util.prefs.PreferencesFactory", &propVal);
-       if (propVal == NULL) {
-           propRes = (*vmInterface)->SetSystemProperty (vmInterface,
-               "java.util.prefs.PreferencesFactory",
-#ifdef _WIN32
-               "java.util.prefs.RegistryPreferencesFactoryImpl");
-#else
-               "java.util.prefs.FilePreferencesFactoryImpl");
-#endif
-           if (VMI_ERROR_NONE != propRes) {
-               /* goto fail2; */
-           }
-       }
-
     /* Prefer Xalan compiler for better performance, see HARMONY-3209.
*/
     (*vmInterface)->GetSystemProperty (vmInterface,
"javax.xml.transform.TransformerFactory", &propVal);
     if (propVal == NULL) {
Index: modules/prefs/src/main/java/java/util/prefs/Preferences.java
=====================================================================
--- modules/prefs/src/main/java/java/util/prefs/Preferences.java
+++ modules/prefs/src/main/java/java/util/prefs/Preferences.java
@@ -16,6 +16,7 @@

 package java.util.prefs;

+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -127,6 +128,13 @@ public abstract class Preferences {
              return
System.getProperty("java.util.prefs.PreferencesFactory"); //$NON-NLS-1$
          }
      });
+        if (factoryClassName == null) {
+            if (File.pathSeparatorChar == '\\') {
+                factoryClassName =
"java.util.prefs.RegistryPreferencesFactoryImpl";
+            } else {
+                factoryClassName =
"java.util.prefs.FilePreferencesFactoryImpl";
+            }
+        }
         try {
             ClassLoader loader =
Thread.currentThread().getContextClassLoader();
             if(loader == null){


-Nathan

On Tue, Dec 30, 2008 at 4:25 AM, Regis <[email protected]> wrote:
Hi,

The provider of prefs is controlled by the property
"java.util.prefs.PreferencesFactory" in Harmony, and we also have
different
default values for it on Linux and Windows, but the default values
are
set
in luni module at
modules/luni/src/main/native/luni/shared/luniglob.c,
are there any special concerns that we must do it in luni native
code?
Or
is
it possible set it in prefs module with java code, like this:

     if (factoryClassName == null) {
         if (isWindows) {
             factoryClassName =
"java.util.prefs.RegistryPreferencesFactoryImpl";
         } else {
             factoryClassName =
"java.util.prefs.FilePreferencesFactoryImpl";
         }
     }

I think there must be a way to get current platform at runtime in
java.

--
Best Regards,
Regis.

--
Best Regards,
Regis.

--
Best Regards,
Regis.

--
Best Regards,
Regis.



--
Best Regards,
Regis.

Reply via email to