Merge changes I49057737,I68e3096d

* changes:
  Frameworks: Clean up SystemProperties
  Frameworks: Add warning to SystemProperties.get
This commit is contained in:
Treehugger Robot
2017-08-31 15:01:37 +00:00
committed by Gerrit Code Review
4 changed files with 224 additions and 207 deletions

View File

@@ -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;
@@ -85,77 +82,102 @@ public class SystemProperties {
private static native void native_report_sysprop_change();
/**
* Get the value for the given key.
* @return an empty string if the key isn't found
* Get the String value for the given {@code key}.
*
* <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);
}
/**
* Get the value for the given key.
* @return if the key isn't found, return def if it isn't null, or an empty string otherwise
* Get the String value for the given {@code key}.
*
* <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();
@@ -164,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) {
@@ -177,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
*/

View File

@@ -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;
namespace {
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("");
}
env->ReleaseStringUTFChars(keyJ, key);
error:
return rvJ;
}
static jstring SystemProperties_getS(JNIEnv *env, jobject clazz,
jstring keyJ)
{
return SystemProperties_getSS(env, clazz, keyJ, NULL);
}
static jint SystemProperties_get_int(JNIEnv *env, jobject clazz,
jstring keyJ, jint defJ)
{
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;
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 result;
return handler(key, defJ);
}
static jlong SystemProperties_get_long(JNIEnv *env, jobject clazz,
jstring keyJ, jlong defJ)
jstring SystemProperties_getSS(JNIEnv *env, jclass clazz, jstring keyJ,
jstring 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;
// 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;
}
}
env->ReleaseStringUTFChars(keyJ, key);
error:
return result;
// This function is specified to never return null (or have an
// exception pending).
return env->NewStringUTF("");
};
return ConvertKeyAndForward(env, keyJ, defJ, handler);
}
static jboolean SystemProperties_get_boolean(JNIEnv *env, jobject clazz,
jstring keyJ, jboolean defJ)
jstring SystemProperties_getS(JNIEnv *env, jclass clazz, jstring keyJ)
{
int len;
const char* key;
char buf[PROPERTY_VALUE_MAX];
jboolean result = defJ;
return SystemProperties_getSS(env, clazz, keyJ, nullptr);
}
if (keyJ == NULL) {
jniThrowNullPointerException(env, "key must not be null.");
goto error;
}
template <typename T>
T SystemProperties_get_integral(JNIEnv *env, jclass, jstring keyJ,
T defJ)
{
auto handler = [](const std::string& key, T defV) {
return android::base::GetIntProperty<T>(key, defV);
};
return ConvertKeyAndForward(env, keyJ, defJ, handler);
}
key = env->GetStringUTFChars(keyJ, NULL);
jboolean SystemProperties_get_boolean(JNIEnv *env, jclass, jstring keyJ,
jboolean defJ)
{
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);
}
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;
void SystemProperties_set(JNIEnv *env, jobject clazz, jstring keyJ,
jstring valJ)
{
auto handler = [&](const std::string& key, bool) {
std::string val;
if (valJ != nullptr) {
ScopedUtfChars key_utf(env, valJ);
val = key_utf.c_str();
}
}
env->ReleaseStringUTFChars(keyJ, key);
error:
return result;
}
static 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) {
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));
}
};

View File

@@ -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

View File

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