From 100b08a848d018eeb1caa5d5e7c7c2aaac65da15 Mon Sep 17 00:00:00 2001 From: S Vasudev Prasad Date: Fri, 8 May 2020 11:45:45 +0530 Subject: [PATCH] libcutils: reimplement system property functions with libbase. This makes the system property functions available on the host too, and ensures a consistent view of system properties between libcutils and libbase, if you have code running on the host that uses both. Bug: http://b/151789258 Test: treehugger Change-Id: Ie524a77d0c392d7b23e9d12becbb1bf53c81eac6 --- libcutils/Android.bp | 4 +- libcutils/include/cutils/properties.h | 29 +-- libcutils/properties.cpp | 102 ++++------ libcutils/properties_test.cpp | 283 ++++++++++++++------------ 4 files changed, 206 insertions(+), 212 deletions(-) diff --git a/libcutils/Android.bp b/libcutils/Android.bp index d7c83a201..60400c98c 100644 --- a/libcutils/Android.bp +++ b/libcutils/Android.bp @@ -152,6 +152,7 @@ cc_library { "iosched_policy.cpp", "load_file.cpp", "native_handle.cpp", + "properties.cpp", "record_stream.cpp", "strlcpy.c", "threads.cpp", @@ -187,7 +188,6 @@ cc_library { "fs_config.cpp", "klog.cpp", "partition_utils.cpp", - "properties.cpp", "qtaguid.cpp", "trace-dev.cpp", "uevent.cpp", @@ -268,6 +268,7 @@ cc_defaults { name: "libcutils_test_default", srcs: [ "native_handle_test.cpp", + "properties_test.cpp", "sockets_test.cpp", ], @@ -280,7 +281,6 @@ cc_defaults { "fs_config_test.cpp", "memset_test.cpp", "multiuser_test.cpp", - "properties_test.cpp", "sched_policy_test.cpp", "str_parms_test.cpp", "trace-dev_test.cpp", diff --git a/libcutils/include/cutils/properties.h b/libcutils/include/cutils/properties.h index d2e08712a..78d8bc654 100644 --- a/libcutils/include/cutils/properties.h +++ b/libcutils/include/cutils/properties.h @@ -14,27 +14,30 @@ * limitations under the License. */ -#ifndef __CUTILS_PROPERTIES_H -#define __CUTILS_PROPERTIES_H +#pragma once #include #include -#include #include +#if __has_include() +#include +#else +#define PROP_VALUE_MAX 92 +#endif + #ifdef __cplusplus extern "C" { #endif -/* System properties are *small* name value pairs managed by the -** property service. If your data doesn't fit in the provided -** space it is not appropriate for a system property. -** -** WARNING: system/bionic/include/sys/system_properties.h also defines -** these, but with different names. (TODO: fix that) -*/ -#define PROPERTY_KEY_MAX PROP_NAME_MAX -#define PROPERTY_VALUE_MAX PROP_VALUE_MAX +// +// Deprecated. +// +// See for better API. +// + +#define PROPERTY_KEY_MAX PROP_NAME_MAX +#define PROPERTY_VALUE_MAX PROP_VALUE_MAX /* property_get: returns the length of the value which will never be ** greater than PROPERTY_VALUE_MAX - 1 and will always be zero terminated. @@ -146,5 +149,3 @@ int property_get(const char *key, char *value, const char *default_value) { #ifdef __cplusplus } #endif - -#endif diff --git a/libcutils/properties.cpp b/libcutils/properties.cpp index 5dbbeba48..03f04961c 100644 --- a/libcutils/properties.cpp +++ b/libcutils/properties.cpp @@ -16,27 +16,19 @@ #include -#define LOG_TAG "properties" -// #define LOG_NDEBUG 0 - -#include -#include #include #include #include #include #include -#include -#include +#include -int8_t property_get_bool(const char *key, int8_t default_value) { - if (!key) { - return default_value; - } +int8_t property_get_bool(const char* key, int8_t default_value) { + if (!key) return default_value; int8_t result = default_value; - char buf[PROPERTY_VALUE_MAX] = {'\0'}; + char buf[PROPERTY_VALUE_MAX] = {}; int len = property_get(key, buf, ""); if (len == 1) { @@ -57,73 +49,53 @@ int8_t property_get_bool(const char *key, int8_t default_value) { return result; } -// Convert string property to int (default if fails); return default value if out of bounds -static intmax_t property_get_imax(const char *key, intmax_t lower_bound, intmax_t upper_bound, - intmax_t default_value) { - if (!key) { - return default_value; +template +static T property_get_int(const char* key, T default_value) { + if (!key) return default_value; + + char value[PROPERTY_VALUE_MAX] = {}; + if (property_get(key, value, "") < 1) return default_value; + + // libcutils unwisely allows octal, which libbase doesn't. + T result = default_value; + int saved_errno = errno; + errno = 0; + char* end = nullptr; + intmax_t v = strtoimax(value, &end, 0); + if (errno != ERANGE && end != value && v >= std::numeric_limits::min() && + v <= std::numeric_limits::max()) { + result = v; } - - intmax_t result = default_value; - char buf[PROPERTY_VALUE_MAX] = {'\0'}; - char *end = NULL; - - int len = property_get(key, buf, ""); - if (len > 0) { - int tmp = errno; - errno = 0; - - // Infer base automatically - result = strtoimax(buf, &end, /*base*/ 0); - if ((result == INTMAX_MIN || result == INTMAX_MAX) && errno == ERANGE) { - // Over or underflow - result = default_value; - ALOGV("%s(%s,%" PRIdMAX ") - overflow", __FUNCTION__, key, default_value); - } else if (result < lower_bound || result > upper_bound) { - // Out of range of requested bounds - result = default_value; - ALOGV("%s(%s,%" PRIdMAX ") - out of range", __FUNCTION__, key, default_value); - } else if (end == buf) { - // Numeric conversion failed - result = default_value; - ALOGV("%s(%s,%" PRIdMAX ") - numeric conversion failed", __FUNCTION__, key, - default_value); - } - - errno = tmp; - } - + errno = saved_errno; return result; } -int64_t property_get_int64(const char *key, int64_t default_value) { - return (int64_t)property_get_imax(key, INT64_MIN, INT64_MAX, default_value); +int64_t property_get_int64(const char* key, int64_t default_value) { + return property_get_int(key, default_value); } -int32_t property_get_int32(const char *key, int32_t default_value) { - return (int32_t)property_get_imax(key, INT32_MIN, INT32_MAX, default_value); +int32_t property_get_int32(const char* key, int32_t default_value) { + return property_get_int(key, default_value); } -#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ -#include - -int property_set(const char *key, const char *value) { +int property_set(const char* key, const char* value) { return __system_property_set(key, value); } -int property_get(const char *key, char *value, const char *default_value) { +int property_get(const char* key, char* value, const char* default_value) { int len = __system_property_get(key, value); - if (len > 0) { - return len; - } - if (default_value) { - len = strnlen(default_value, PROPERTY_VALUE_MAX - 1); - memcpy(value, default_value, len); - value[len] = '\0'; + if (len < 1 && default_value) { + snprintf(value, PROPERTY_VALUE_MAX, "%s", default_value); + return strlen(value); } return len; } +#if __has_include() + +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include + struct callback_data { void (*callback)(const char* name, const char* value, void* cookie); void* cookie; @@ -139,6 +111,8 @@ static void property_list_callback(const prop_info* pi, void* data) { } int property_list(void (*fn)(const char* name, const char* value, void* cookie), void* cookie) { - callback_data data = { fn, cookie }; + callback_data data = {fn, cookie}; return __system_property_foreach(property_list_callback, &data); } + +#endif diff --git a/libcutils/properties_test.cpp b/libcutils/properties_test.cpp index 79219720e..efc01833a 100644 --- a/libcutils/properties_test.cpp +++ b/libcutils/properties_test.cpp @@ -93,160 +93,179 @@ protected: } }; -TEST_F(PropertiesTest, SetString) { - +TEST_F(PropertiesTest, property_set_null_key) { // Null key -> unsuccessful set - { - // Null key -> fails - EXPECT_GT(0, property_set(/*key*/NULL, PROPERTY_TEST_VALUE_DEFAULT)); - } + EXPECT_GT(0, property_set(/*key*/ NULL, PROPERTY_TEST_VALUE_DEFAULT)); +} - // Null value -> returns default value - { - // Null value -> OK , and it clears the value - EXPECT_OK(property_set(PROPERTY_TEST_KEY, /*value*/NULL)); - ResetValue(); +TEST_F(PropertiesTest, property_set_null_value) { + // Null value -> OK, and it clears the value + EXPECT_OK(property_set(PROPERTY_TEST_KEY, /*value*/ NULL)); + ResetValue(); - // Since the value is null, default value will be returned - size_t len = property_get(PROPERTY_TEST_KEY, mValue, PROPERTY_TEST_VALUE_DEFAULT); - EXPECT_EQ(strlen(PROPERTY_TEST_VALUE_DEFAULT), len); - EXPECT_STREQ(PROPERTY_TEST_VALUE_DEFAULT, mValue); - } + // Since the value is null, default value will be returned + size_t len = property_get(PROPERTY_TEST_KEY, mValue, PROPERTY_TEST_VALUE_DEFAULT); + EXPECT_EQ(strlen(PROPERTY_TEST_VALUE_DEFAULT), len); + EXPECT_STREQ(PROPERTY_TEST_VALUE_DEFAULT, mValue); +} +TEST_F(PropertiesTest, property_set) { // Trivial case => get returns what was set - { - size_t len = SetAndGetProperty("hello_world"); - EXPECT_EQ(strlen("hello_world"), len) << "hello_world key"; - EXPECT_STREQ("hello_world", mValue); - ResetValue(); - } + size_t len = SetAndGetProperty("hello_world"); + EXPECT_EQ(strlen("hello_world"), len) << "hello_world key"; + EXPECT_STREQ("hello_world", mValue); + ResetValue(); +} +TEST_F(PropertiesTest, property_set_empty) { // Set to empty string => get returns default always - { - const char* EMPTY_STRING_DEFAULT = "EMPTY_STRING"; - size_t len = SetAndGetProperty("", EMPTY_STRING_DEFAULT); - EXPECT_EQ(strlen(EMPTY_STRING_DEFAULT), len) << "empty key"; - EXPECT_STREQ(EMPTY_STRING_DEFAULT, mValue); - ResetValue(); - } + const char* EMPTY_STRING_DEFAULT = "EMPTY_STRING"; + size_t len = SetAndGetProperty("", EMPTY_STRING_DEFAULT); + EXPECT_EQ(strlen(EMPTY_STRING_DEFAULT), len) << "empty key"; + EXPECT_STREQ(EMPTY_STRING_DEFAULT, mValue); + ResetValue(); +} +TEST_F(PropertiesTest, property_set_max_length) { // Set to max length => get returns what was set - { - std::string maxLengthString = std::string(PROPERTY_VALUE_MAX-1, 'a'); + std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'a'); - int len = SetAndGetProperty(maxLengthString.c_str()); - EXPECT_EQ(PROPERTY_VALUE_MAX-1, len) << "max length key"; - EXPECT_STREQ(maxLengthString.c_str(), mValue); - ResetValue(); - } + int len = SetAndGetProperty(maxLengthString.c_str()); + EXPECT_EQ(PROPERTY_VALUE_MAX - 1, len) << "max length key"; + EXPECT_STREQ(maxLengthString.c_str(), mValue); + ResetValue(); +} +TEST_F(PropertiesTest, property_set_too_long) { // Set to max length + 1 => set fails - { - const char* VALID_TEST_VALUE = "VALID_VALUE"; - ASSERT_OK(property_set(PROPERTY_TEST_KEY, VALID_TEST_VALUE)); + const char* VALID_TEST_VALUE = "VALID_VALUE"; + ASSERT_OK(property_set(PROPERTY_TEST_KEY, VALID_TEST_VALUE)); - std::string oneLongerString = std::string(PROPERTY_VALUE_MAX, 'a'); + std::string oneLongerString = std::string(PROPERTY_VALUE_MAX, 'a'); - // Expect that the value set fails since it's too long - EXPECT_GT(0, property_set(PROPERTY_TEST_KEY, oneLongerString.c_str())); - size_t len = property_get(PROPERTY_TEST_KEY, mValue, PROPERTY_TEST_VALUE_DEFAULT); + // Expect that the value set fails since it's too long + EXPECT_GT(0, property_set(PROPERTY_TEST_KEY, oneLongerString.c_str())); + size_t len = property_get(PROPERTY_TEST_KEY, mValue, PROPERTY_TEST_VALUE_DEFAULT); - EXPECT_EQ(strlen(VALID_TEST_VALUE), len) << "set should've failed"; - EXPECT_STREQ(VALID_TEST_VALUE, mValue); - ResetValue(); - } + EXPECT_EQ(strlen(VALID_TEST_VALUE), len) << "set should've failed"; + EXPECT_STREQ(VALID_TEST_VALUE, mValue); + ResetValue(); } -TEST_F(PropertiesTest, GetString) { - +TEST_F(PropertiesTest, property_get_too_long) { // Try to use a default value that's too long => get truncates the value - { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'a'); - std::string oneLongerString = std::string(PROPERTY_VALUE_MAX, 'a'); + std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'a'); + std::string oneLongerString = std::string(PROPERTY_VALUE_MAX, 'a'); - // Expect that the value is truncated since it's too long (by 1) - int len = property_get(PROPERTY_TEST_KEY, mValue, oneLongerString.c_str()); - EXPECT_EQ(PROPERTY_VALUE_MAX - 1, len); - EXPECT_STREQ(maxLengthString.c_str(), mValue); - ResetValue(); - } - - // Try to use a default value that's the max length => get succeeds - { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - - std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'b'); - - // Expect that the value matches maxLengthString - int len = property_get(PROPERTY_TEST_KEY, mValue, maxLengthString.c_str()); - EXPECT_EQ(PROPERTY_VALUE_MAX - 1, len); - EXPECT_STREQ(maxLengthString.c_str(), mValue); - ResetValue(); - } - - // Try to use a default value of length one => get succeeds - { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - - std::string oneCharString = std::string(1, 'c'); - - // Expect that the value matches oneCharString - int len = property_get(PROPERTY_TEST_KEY, mValue, oneCharString.c_str()); - EXPECT_EQ(1, len); - EXPECT_STREQ(oneCharString.c_str(), mValue); - ResetValue(); - } - - // Try to use a default value of length zero => get succeeds - { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - - std::string zeroCharString = std::string(0, 'd'); - - // Expect that the value matches oneCharString - int len = property_get(PROPERTY_TEST_KEY, mValue, zeroCharString.c_str()); - EXPECT_EQ(0, len); - EXPECT_STREQ(zeroCharString.c_str(), mValue); - ResetValue(); - } - - // Try to use a NULL default value => get returns 0 - { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - - // Expect a return value of 0 - int len = property_get(PROPERTY_TEST_KEY, mValue, NULL); - EXPECT_EQ(0, len); - ResetValue(); - } + // Expect that the value is truncated since it's too long (by 1) + int len = property_get(PROPERTY_TEST_KEY, mValue, oneLongerString.c_str()); + EXPECT_EQ(PROPERTY_VALUE_MAX - 1, len); + EXPECT_STREQ(maxLengthString.c_str(), mValue); + ResetValue(); } -TEST_F(PropertiesTest, GetBool) { - /** - * TRUE - */ - const char *valuesTrue[] = { "1", "true", "y", "yes", "on", }; - for (size_t i = 0; i < arraysize(valuesTrue); ++i) { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, valuesTrue[i])); - bool val = property_get_bool(PROPERTY_TEST_KEY, /*default_value*/false); - EXPECT_TRUE(val) << "Property should've been TRUE for value: '" << valuesTrue[i] << "'"; - } +TEST_F(PropertiesTest, property_get_default_too_long) { + // Try to use a default value that's the max length => get succeeds + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - /** - * FALSE - */ - const char *valuesFalse[] = { "0", "false", "n", "no", "off", }; - for (size_t i = 0; i < arraysize(valuesFalse); ++i) { - ASSERT_OK(property_set(PROPERTY_TEST_KEY, valuesFalse[i])); - bool val = property_get_bool(PROPERTY_TEST_KEY, /*default_value*/true); - EXPECT_FALSE(val) << "Property shoud've been FALSE For string value: '" << valuesFalse[i] << "'"; - } + std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'b'); - /** - * NEITHER - */ + // Expect that the value matches maxLengthString + int len = property_get(PROPERTY_TEST_KEY, mValue, maxLengthString.c_str()); + EXPECT_EQ(PROPERTY_VALUE_MAX - 1, len); + EXPECT_STREQ(maxLengthString.c_str(), mValue); + ResetValue(); +} + +TEST_F(PropertiesTest, property_get_default_okay) { + // Try to use a default value of length one => get succeeds + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); + + std::string oneCharString = std::string(1, 'c'); + + // Expect that the value matches oneCharString + int len = property_get(PROPERTY_TEST_KEY, mValue, oneCharString.c_str()); + EXPECT_EQ(1, len); + EXPECT_STREQ(oneCharString.c_str(), mValue); + ResetValue(); +} + +TEST_F(PropertiesTest, property_get_default_empty) { + // Try to use a default value of length zero => get succeeds + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); + + std::string zeroCharString = std::string(0, 'd'); + + // Expect that the value matches oneCharString + int len = property_get(PROPERTY_TEST_KEY, mValue, zeroCharString.c_str()); + EXPECT_EQ(0, len); + EXPECT_STREQ(zeroCharString.c_str(), mValue); + ResetValue(); +} + +TEST_F(PropertiesTest, property_get_default_NULL) { + // Try to use a NULL default value => get returns 0 + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); + + // Expect a return value of 0 + int len = property_get(PROPERTY_TEST_KEY, mValue, NULL); + EXPECT_EQ(0, len); + ResetValue(); +} + +TEST_F(PropertiesTest, property_get_bool_0) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "0")); + ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true)); +} + +TEST_F(PropertiesTest, property_get_bool_1) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "1")); + ASSERT_TRUE(property_get_bool(PROPERTY_TEST_KEY, false)); +} + +TEST_F(PropertiesTest, property_get_bool_false) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "false")); + ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true)); +} + +TEST_F(PropertiesTest, property_get_bool_n) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "n")); + ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true)); +} + +TEST_F(PropertiesTest, property_get_bool_no) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "no")); + ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true)); +} + +TEST_F(PropertiesTest, property_get_bool_off) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "off")); + ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true)); +} + +TEST_F(PropertiesTest, property_get_bool_on) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "on")); + ASSERT_TRUE(property_get_bool(PROPERTY_TEST_KEY, false)); +} + +TEST_F(PropertiesTest, property_get_bool_true) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "true")); + ASSERT_TRUE(property_get_bool(PROPERTY_TEST_KEY, false)); +} + +TEST_F(PropertiesTest, property_get_bool_y) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "y")); + ASSERT_TRUE(property_get_bool(PROPERTY_TEST_KEY, false)); +} + +TEST_F(PropertiesTest, property_get_bool_yes) { + ASSERT_OK(property_set(PROPERTY_TEST_KEY, "yes")); + ASSERT_TRUE(property_get_bool(PROPERTY_TEST_KEY, false)); +} + +TEST_F(PropertiesTest, property_get_bool_neither) { const char *valuesNeither[] = { "x0", "x1", "2", "-2", "True", "False", "garbage", "", " ", "+1", " 1 ", " true", " true ", " y ", " yes", "yes ", "+0", "-0", "00", " 00 ", " false", "false ", @@ -263,7 +282,7 @@ TEST_F(PropertiesTest, GetBool) { } } -TEST_F(PropertiesTest, GetInt64) { +TEST_F(PropertiesTest, property_get_int64) { const int64_t DEFAULT_VALUE = INT64_C(0xDEADBEEFBEEFDEAD); const std::string longMaxString = ToString(INT64_MAX); @@ -310,7 +329,7 @@ TEST_F(PropertiesTest, GetInt64) { } } -TEST_F(PropertiesTest, GetInt32) { +TEST_F(PropertiesTest, property_get_int32) { const int32_t DEFAULT_VALUE = INT32_C(0xDEADBEEF); const std::string intMaxString = ToString(INT32_MAX);