diff options
4 files changed, 209 insertions, 201 deletions
diff --git a/core/java/android/os/SystemProperties.java b/core/java/android/os/SystemProperties.java index 5a79a54d5825..560b4b31cdc6 100644 --- a/core/java/android/os/SystemProperties.java +++ b/core/java/android/os/SystemProperties.java @@ -16,6 +16,8 @@ package android.os; +import android.annotation.NonNull; +import android.annotation.Nullable; import android.util.Log; import android.util.MutableInt; @@ -43,17 +45,12 @@ public class SystemProperties { public static final int PROP_VALUE_MAX = 91; + @GuardedBy("sChangeCallbacks") private static final ArrayList<Runnable> sChangeCallbacks = new ArrayList<Runnable>(); @GuardedBy("sRoReads") - private static final HashMap<String, MutableInt> sRoReads; - static { - if (TRACK_KEY_ACCESS) { - sRoReads = new HashMap<>(); - } else { - sRoReads = null; - } - } + private static final HashMap<String, MutableInt> sRoReads = + TRACK_KEY_ACCESS ? new HashMap<>() : null; private static void onKeyAccess(String key) { if (!TRACK_KEY_ACCESS) return; @@ -90,9 +87,11 @@ public class SystemProperties { * <b>WARNING:</b> Do not use this method if the value may not be a valid UTF string! This * method will crash in native code. * + * @param key the key to lookup * @return an empty string if the {@code key} isn't found */ - public static String get(String key) { + @NonNull + public static String get(@NonNull String key) { if (TRACK_KEY_ACCESS) onKeyAccess(key); return native_get(key); } @@ -103,68 +102,82 @@ public class SystemProperties { * <b>WARNING:</b> Do not use this method if the value may not be a valid UTF string! This * method will crash in native code. * + * @param key the key to lookup + * @param def the default value in case the property is not set or empty * @return if the {@code key} isn't found, return {@code def} if it isn't null, or an empty * string otherwise */ - public static String get(String key, String def) { + @NonNull + public static String get(@NonNull String key, @Nullable String def) { if (TRACK_KEY_ACCESS) onKeyAccess(key); return native_get(key, def); } /** - * Get the value for the given key, and return as an integer. + * Get the value for the given {@code key}, and return as an integer. + * * @param key the key to lookup * @param def a default value to return * @return the key parsed as an integer, or def if the key isn't found or * cannot be parsed */ - public static int getInt(String key, int def) { + public static int getInt(@NonNull String key, int def) { if (TRACK_KEY_ACCESS) onKeyAccess(key); return native_get_int(key, def); } /** - * Get the value for the given key, and return as a long. + * Get the value for the given {@code key}, and return as a long. + * * @param key the key to lookup * @param def a default value to return * @return the key parsed as a long, or def if the key isn't found or * cannot be parsed */ - public static long getLong(String key, long def) { + public static long getLong(@NonNull String key, long def) { if (TRACK_KEY_ACCESS) onKeyAccess(key); return native_get_long(key, def); } /** - * Get the value for the given key, returned as a boolean. + * Get the value for the given {@code key}, returned as a boolean. * Values 'n', 'no', '0', 'false' or 'off' are considered false. * Values 'y', 'yes', '1', 'true' or 'on' are considered true. * (case sensitive). * If the key does not exist, or has any other value, then the default * result is returned. + * * @param key the key to lookup * @param def a default value to return * @return the key parsed as a boolean, or def if the key isn't found or is * not able to be parsed as a boolean. */ - public static boolean getBoolean(String key, boolean def) { + public static boolean getBoolean(@NonNull String key, boolean def) { if (TRACK_KEY_ACCESS) onKeyAccess(key); return native_get_boolean(key, def); } /** - * Set the value for the given key. - * @throws IllegalArgumentException if the value exceeds 92 characters + * Set the value for the given {@code key} to {@code val}. + * + * @throws IllegalArgumentException if the {@code val} exceeds 91 characters */ - public static void set(String key, String val) { + public static void set(@NonNull String key, @Nullable String val) { if (val != null && val.length() > PROP_VALUE_MAX) { - throw newValueTooLargeException(key, val); + throw new IllegalArgumentException("value of system property '" + key + + "' is longer than " + PROP_VALUE_MAX + " characters: " + val); } if (TRACK_KEY_ACCESS) onKeyAccess(key); native_set(key, val); } - public static void addChangeCallback(Runnable callback) { + /** + * Add a callback that will be run whenever any system property changes. + * + * @param callback The {@link Runnable} that should be executed when a system property + * changes. + */ + public static void addChangeCallback(@NonNull Runnable callback) { synchronized (sChangeCallbacks) { if (sChangeCallbacks.size() == 0) { native_add_change_callback(); @@ -173,7 +186,8 @@ public class SystemProperties { } } - static void callChangeCallbacks() { + @SuppressWarnings("unused") // Called from native code. + private static void callChangeCallbacks() { synchronized (sChangeCallbacks) { //Log.i("foo", "Calling " + sChangeCallbacks.size() + " change callbacks!"); if (sChangeCallbacks.size() == 0) { @@ -186,11 +200,6 @@ public class SystemProperties { } } - private static IllegalArgumentException newValueTooLargeException(String key, String value) { - return new IllegalArgumentException("value of system property '" + key + "' is longer than " - + PROP_VALUE_MAX + " characters: " + value); - } - /* * Notifies listeners that a system property has changed */ diff --git a/core/jni/android_os_SystemProperties.cpp b/core/jni/android_os_SystemProperties.cpp index 8844fb0a261f..a94cac0f18f5 100644 --- a/core/jni/android_os_SystemProperties.cpp +++ b/core/jni/android_os_SystemProperties.cpp @@ -17,188 +17,109 @@ #define LOG_TAG "SysPropJNI" +#include "android-base/logging.h" +#include "android-base/properties.h" #include "cutils/properties.h" #include "utils/misc.h" #include <utils/Log.h> #include "jni.h" #include "core_jni_helpers.h" #include <nativehelper/JNIHelp.h> +#include <nativehelper/ScopedPrimitiveArray.h> +#include <nativehelper/ScopedUtfChars.h> namespace android { -static jstring SystemProperties_getSS(JNIEnv *env, jobject clazz, - jstring keyJ, jstring defJ) -{ - int len; - const char* key; - char buf[PROPERTY_VALUE_MAX]; - jstring rvJ = NULL; - - if (keyJ == NULL) { - jniThrowNullPointerException(env, "key must not be null."); - goto error; - } - - key = env->GetStringUTFChars(keyJ, NULL); - - len = property_get(key, buf, ""); - if ((len <= 0) && (defJ != NULL)) { - rvJ = defJ; - } else if (len >= 0) { - rvJ = env->NewStringUTF(buf); - } else { - rvJ = env->NewStringUTF(""); +namespace { + +template <typename T, typename Handler> +T ConvertKeyAndForward(JNIEnv *env, jstring keyJ, T defJ, Handler handler) { + std::string key; + { + // Scope the String access. If the handler can throw an exception, + // releasing the string characters late would trigger an abort. + ScopedUtfChars key_utf(env, keyJ); + if (key_utf.c_str() == nullptr) { + return defJ; + } + key = key_utf.c_str(); // This will make a copy, but we can't avoid + // with the existing interface in + // android::base. } - - env->ReleaseStringUTFChars(keyJ, key); - -error: - return rvJ; + return handler(key, defJ); } -static jstring SystemProperties_getS(JNIEnv *env, jobject clazz, - jstring keyJ) +jstring SystemProperties_getSS(JNIEnv *env, jclass clazz, jstring keyJ, + jstring defJ) { - return SystemProperties_getSS(env, clazz, keyJ, NULL); + // Using ConvertKeyAndForward is sub-optimal for copying the key string, + // but improves reuse and reasoning over code. + auto handler = [&](const std::string& key, jstring defJ) { + std::string prop_val = android::base::GetProperty(key, ""); + if (!prop_val.empty()) { + return env->NewStringUTF(prop_val.c_str()); + }; + if (defJ != nullptr) { + return defJ; + } + // This function is specified to never return null (or have an + // exception pending). + return env->NewStringUTF(""); + }; + return ConvertKeyAndForward(env, keyJ, defJ, handler); } -static jint SystemProperties_get_int(JNIEnv *env, jobject clazz, - jstring keyJ, jint defJ) +jstring SystemProperties_getS(JNIEnv *env, jclass clazz, jstring keyJ) { - int len; - const char* key; - char buf[PROPERTY_VALUE_MAX]; - char* end; - jint result = defJ; - - if (keyJ == NULL) { - jniThrowNullPointerException(env, "key must not be null."); - goto error; - } - - key = env->GetStringUTFChars(keyJ, NULL); - - len = property_get(key, buf, ""); - if (len > 0) { - result = strtol(buf, &end, 0); - if (end == buf) { - result = defJ; - } - } - - env->ReleaseStringUTFChars(keyJ, key); - -error: - return result; + return SystemProperties_getSS(env, clazz, keyJ, nullptr); } -static jlong SystemProperties_get_long(JNIEnv *env, jobject clazz, - jstring keyJ, jlong defJ) +template <typename T> +T SystemProperties_get_integral(JNIEnv *env, jclass, jstring keyJ, + T defJ) { - int len; - const char* key; - char buf[PROPERTY_VALUE_MAX]; - char* end; - jlong result = defJ; - - if (keyJ == NULL) { - jniThrowNullPointerException(env, "key must not be null."); - goto error; - } - - key = env->GetStringUTFChars(keyJ, NULL); - - len = property_get(key, buf, ""); - if (len > 0) { - result = strtoll(buf, &end, 0); - if (end == buf) { - result = defJ; - } - } - - env->ReleaseStringUTFChars(keyJ, key); - -error: - return result; + auto handler = [](const std::string& key, T defV) { + return android::base::GetIntProperty<T>(key, defV); + }; + return ConvertKeyAndForward(env, keyJ, defJ, handler); } -static jboolean SystemProperties_get_boolean(JNIEnv *env, jobject clazz, - jstring keyJ, jboolean defJ) +jboolean SystemProperties_get_boolean(JNIEnv *env, jclass, jstring keyJ, + jboolean defJ) { - int len; - const char* key; - char buf[PROPERTY_VALUE_MAX]; - jboolean result = defJ; - - if (keyJ == NULL) { - jniThrowNullPointerException(env, "key must not be null."); - goto error; - } - - key = env->GetStringUTFChars(keyJ, NULL); - - len = property_get(key, buf, ""); - if (len == 1) { - char ch = buf[0]; - if (ch == '0' || ch == 'n') - result = false; - else if (ch == '1' || ch == 'y') - result = true; - } else if (len > 1) { - if (!strcmp(buf, "no") || !strcmp(buf, "false") || !strcmp(buf, "off")) { - result = false; - } else if (!strcmp(buf, "yes") || !strcmp(buf, "true") || !strcmp(buf, "on")) { - result = true; - } - } - - env->ReleaseStringUTFChars(keyJ, key); - -error: - return result; + auto handler = [](const std::string& key, jboolean defV) -> jboolean { + bool result = android::base::GetBoolProperty(key, defV); + return result ? JNI_TRUE : JNI_FALSE; + }; + return ConvertKeyAndForward(env, keyJ, defJ, handler); } -static void SystemProperties_set(JNIEnv *env, jobject clazz, - jstring keyJ, jstring valJ) +void SystemProperties_set(JNIEnv *env, jobject clazz, jstring keyJ, + jstring valJ) { - int err; - const char* key; - const char* val; - - if (keyJ == NULL) { - jniThrowNullPointerException(env, "key must not be null."); - return ; - } - key = env->GetStringUTFChars(keyJ, NULL); - - if (valJ == NULL) { - val = ""; /* NULL pointer not allowed here */ - } else { - val = env->GetStringUTFChars(valJ, NULL); - } - - err = property_set(key, val); - - env->ReleaseStringUTFChars(keyJ, key); - - if (valJ != NULL) { - env->ReleaseStringUTFChars(valJ, val); - } - - if (err < 0) { + auto handler = [&](const std::string& key, bool) { + std::string val; + if (valJ != nullptr) { + ScopedUtfChars key_utf(env, valJ); + val = key_utf.c_str(); + } + return android::base::SetProperty(key, val); + }; + if (!ConvertKeyAndForward(env, keyJ, true, handler)) { + // Must have been a failure in SetProperty. jniThrowException(env, "java/lang/RuntimeException", "failed to set system property"); } } -static JavaVM* sVM = NULL; -static jclass sClazz = NULL; -static jmethodID sCallChangeCallbacks; +JavaVM* sVM = nullptr; +jclass sClazz = nullptr; +jmethodID sCallChangeCallbacks; -static void do_report_sysprop_change() { +void do_report_sysprop_change() { //ALOGI("Java SystemProperties: VM=%p, Clazz=%p", sVM, sClazz); - if (sVM != NULL && sClazz != NULL) { + if (sVM != nullptr && sClazz != nullptr) { JNIEnv* env; if (sVM->GetEnv((void **)&env, JNI_VERSION_1_4) >= 0) { //ALOGI("Java SystemProperties: calling %p", sCallChangeCallbacks); @@ -207,47 +128,49 @@ static void do_report_sysprop_change() { } } -static void SystemProperties_add_change_callback(JNIEnv *env, jobject clazz) +void SystemProperties_add_change_callback(JNIEnv *env, jobject clazz) { // This is called with the Java lock held. - if (sVM == NULL) { + if (sVM == nullptr) { env->GetJavaVM(&sVM); } - if (sClazz == NULL) { + if (sClazz == nullptr) { sClazz = (jclass) env->NewGlobalRef(clazz); sCallChangeCallbacks = env->GetStaticMethodID(sClazz, "callChangeCallbacks", "()V"); add_sysprop_change_callback(do_report_sysprop_change, -10000); } } -static void SystemProperties_report_sysprop_change(JNIEnv /**env*/, jobject /*clazz*/) +void SystemProperties_report_sysprop_change(JNIEnv /**env*/, jobject /*clazz*/) { report_sysprop_change(); } -static const JNINativeMethod method_table[] = { - { "native_get", "(Ljava/lang/String;)Ljava/lang/String;", - (void*) SystemProperties_getS }, - { "native_get", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", - (void*) SystemProperties_getSS }, - { "native_get_int", "(Ljava/lang/String;I)I", - (void*) SystemProperties_get_int }, - { "native_get_long", "(Ljava/lang/String;J)J", - (void*) SystemProperties_get_long }, - { "native_get_boolean", "(Ljava/lang/String;Z)Z", - (void*) SystemProperties_get_boolean }, - { "native_set", "(Ljava/lang/String;Ljava/lang/String;)V", - (void*) SystemProperties_set }, - { "native_add_change_callback", "()V", - (void*) SystemProperties_add_change_callback }, - { "native_report_sysprop_change", "()V", - (void*) SystemProperties_report_sysprop_change }, -}; +} // namespace int register_android_os_SystemProperties(JNIEnv *env) { - return RegisterMethodsOrDie(env, "android/os/SystemProperties", method_table, - NELEM(method_table)); + const JNINativeMethod method_table[] = { + { "native_get", "(Ljava/lang/String;)Ljava/lang/String;", + (void*) SystemProperties_getS }, + { "native_get", + "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", + (void*) SystemProperties_getSS }, + { "native_get_int", "(Ljava/lang/String;I)I", + (void*) SystemProperties_get_integral<jint> }, + { "native_get_long", "(Ljava/lang/String;J)J", + (void*) SystemProperties_get_integral<jlong> }, + { "native_get_boolean", "(Ljava/lang/String;Z)Z", + (void*) SystemProperties_get_boolean }, + { "native_set", "(Ljava/lang/String;Ljava/lang/String;)V", + (void*) SystemProperties_set }, + { "native_add_change_callback", "()V", + (void*) SystemProperties_add_change_callback }, + { "native_report_sysprop_change", "()V", + (void*) SystemProperties_report_sysprop_change }, + }; + return RegisterMethodsOrDie(env, "android/os/SystemProperties", + method_table, NELEM(method_table)); } }; diff --git a/core/tests/systemproperties/run_core_systemproperties_test.sh b/core/tests/systemproperties/run_core_systemproperties_test.sh index d39adbbfa390..9b1fe4be666b 100755 --- a/core/tests/systemproperties/run_core_systemproperties_test.sh +++ b/core/tests/systemproperties/run_core_systemproperties_test.sh @@ -15,7 +15,7 @@ fi if [[ $rebuild == true ]]; then make -j4 FrameworksCoreSystemPropertiesTests - TESTAPP=${ANDROID_PRODUCT_OUT}/data/app/FrameworksCoreSystemPropertiesTests.apk + TESTAPP=${ANDROID_PRODUCT_OUT}/data/app/FrameworksCoreSystemPropertiesTests/FrameworksCoreSystemPropertiesTests.apk COMMAND="adb install -r $TESTAPP" echo $COMMAND $COMMAND diff --git a/core/tests/systemproperties/src/android/os/SystemPropertiesTest.java b/core/tests/systemproperties/src/android/os/SystemPropertiesTest.java index 544a96727217..282b0011eede 100644 --- a/core/tests/systemproperties/src/android/os/SystemPropertiesTest.java +++ b/core/tests/systemproperties/src/android/os/SystemPropertiesTest.java @@ -51,6 +51,11 @@ public class SystemPropertiesTest extends TestCase { value = SystemProperties.get(KEY, "default"); assertEquals("default", value); + // null default value is the same as "". + SystemProperties.set(KEY, null); + value = SystemProperties.get(KEY, "default"); + assertEquals("default", value); + SystemProperties.set(KEY, "SA"); value = SystemProperties.get(KEY, "default"); assertEquals("SA", value); @@ -62,7 +67,78 @@ public class SystemPropertiesTest extends TestCase { value = SystemProperties.get(KEY, "default"); assertEquals("default", value); + // null value is the same as "". + SystemProperties.set(KEY, "SA"); + SystemProperties.set(KEY, null); + value = SystemProperties.get(KEY, "default"); + assertEquals("default", value); + value = SystemProperties.get(KEY); assertEquals("", value); } + + private static void testInt(String setVal, int defValue, int expected) { + SystemProperties.set(KEY, setVal); + int value = SystemProperties.getInt(KEY, defValue); + assertEquals(expected, value); + } + + private static void testLong(String setVal, long defValue, long expected) { + SystemProperties.set(KEY, setVal); + long value = SystemProperties.getLong(KEY, defValue); + assertEquals(expected, value); + } + + @SmallTest + public void testIntegralProperties() throws Exception { + testInt("", 123, 123); + testInt("", 0, 0); + testInt("", -123, -123); + + testInt("123", 124, 123); + testInt("0", 124, 0); + testInt("-123", 124, -123); + + testLong("", 3147483647L, 3147483647L); + testLong("", 0, 0); + testLong("", -3147483647L, -3147483647L); + + testLong("3147483647", 124, 3147483647L); + testLong("0", 124, 0); + testLong("-3147483647", 124, -3147483647L); + } + + @SmallTest + @SuppressWarnings("null") + public void testNullKey() throws Exception { + try { + SystemProperties.get(null); + fail("Expected NullPointerException"); + } catch (NullPointerException npe) { + } + + try { + SystemProperties.get(null, "default"); + fail("Expected NullPointerException"); + } catch (NullPointerException npe) { + } + + try { + SystemProperties.set(null, "value"); + fail("Expected NullPointerException"); + } catch (NullPointerException npe) { + } + + try { + SystemProperties.getInt(null, 0); + fail("Expected NullPointerException"); + } catch (NullPointerException npe) { + } + + try { + SystemProperties.getLong(null, 0); + fail("Expected NullPointerException"); + } catch (NullPointerException npe) { + } + } } |