From 22c0962ab9897d2eadb7876902e7af7ca9d17852 Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Tue, 20 Dec 2016 06:47:36 -0800 Subject: [PATCH 1/2] Use clang-format to fix up properties.c BasedOnStyle: Google IndentWidth: 4 ColumnLimit: 100 Test: unit tests pass Change-Id: Ie1a9bf85c001ef1a2dcdafdc5bf696b9267116f7 --- libcutils/properties.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/libcutils/properties.c b/libcutils/properties.c index 5aa63711a..69c6c7ffa 100644 --- a/libcutils/properties.c +++ b/libcutils/properties.c @@ -36,7 +36,7 @@ int8_t property_get_bool(const char *key, int8_t default_value) { } int8_t result = default_value; - char buf[PROPERTY_VALUE_MAX] = {'\0',}; + char buf[PROPERTY_VALUE_MAX] = {'\0'}; int len = property_get(key, buf, ""); if (len == 1) { @@ -47,7 +47,7 @@ int8_t property_get_bool(const char *key, int8_t default_value) { result = true; } } else if (len > 1) { - if (!strcmp(buf, "no") || !strcmp(buf, "false") || !strcmp(buf, "off")) { + 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; @@ -59,13 +59,13 @@ int8_t property_get_bool(const char *key, int8_t default_value) { // 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) { + intmax_t default_value) { if (!key) { return default_value; } intmax_t result = default_value; - char buf[PROPERTY_VALUE_MAX] = {'\0',}; + char buf[PROPERTY_VALUE_MAX] = {'\0'}; char *end = NULL; int len = property_get(key, buf, ""); @@ -74,7 +74,7 @@ static intmax_t property_get_imax(const char *key, intmax_t lower_bound, intmax_ errno = 0; // Infer base automatically - result = strtoimax(buf, &end, /*base*/0); + result = strtoimax(buf, &end, /*base*/ 0); if ((result == INTMAX_MIN || result == INTMAX_MAX) && errno == ERANGE) { // Over or underflow result = default_value; @@ -86,8 +86,8 @@ static intmax_t property_get_imax(const char *key, intmax_t lower_bound, intmax_ } else if (end == buf) { // Numeric conversion failed result = default_value; - ALOGV("%s(%s,%" PRIdMAX ") - numeric conversion failed", - __FUNCTION__, key, default_value); + ALOGV("%s(%s,%" PRIdMAX ") - numeric conversion failed", __FUNCTION__, key, + default_value); } errno = tmp; @@ -107,20 +107,18 @@ int32_t property_get_int32(const char *key, int32_t 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; len = __system_property_get(key, value); - if(len > 0) { + if (len > 0) { return len; } - if(default_value) { + if (default_value) { len = strlen(default_value); if (len >= PROPERTY_VALUE_MAX) { len = PROPERTY_VALUE_MAX - 1; @@ -131,14 +129,12 @@ int property_get(const char *key, char *value, const char *default_value) return len; } -struct property_list_callback_data -{ +struct property_list_callback_data { void (*propfn)(const char *key, const char *value, void *cookie); void *cookie; }; -static void property_list_callback(const prop_info *pi, void *cookie) -{ +static void property_list_callback(const prop_info *pi, void *cookie) { char name[PROP_NAME_MAX]; char value[PROP_VALUE_MAX]; struct property_list_callback_data *data = cookie; @@ -147,10 +143,7 @@ static void property_list_callback(const prop_info *pi, void *cookie) data->propfn(name, value, data->cookie); } -int property_list( - void (*propfn)(const char *key, const char *value, void *cookie), - void *cookie) -{ - struct property_list_callback_data data = { propfn, cookie }; +int property_list(void (*propfn)(const char *key, const char *value, void *cookie), void *cookie) { + struct property_list_callback_data data = {propfn, cookie}; return __system_property_foreach(property_list_callback, &data); } From e67abec51428bf93c664d632af24fb28e74a6281 Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Thu, 22 Dec 2016 09:05:18 -0800 Subject: [PATCH 2/2] libcutils: Use strnlen for default property values Add unit tests to test the corner cases. Test: unit tests pass before and after the change. Change-Id: Idafeb8354cd6c7db2a68cd398dafe153453a3940 --- libcutils/properties.c | 5 +-- libcutils/tests/PropertiesTest.cpp | 55 ++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/libcutils/properties.c b/libcutils/properties.c index 69c6c7ffa..740c7a97a 100644 --- a/libcutils/properties.c +++ b/libcutils/properties.c @@ -119,10 +119,7 @@ int property_get(const char *key, char *value, const char *default_value) { return len; } if (default_value) { - len = strlen(default_value); - if (len >= PROPERTY_VALUE_MAX) { - len = PROPERTY_VALUE_MAX - 1; - } + len = strnlen(default_value, PROPERTY_VALUE_MAX - 1); memcpy(value, default_value, len); value[len] = '\0'; } diff --git a/libcutils/tests/PropertiesTest.cpp b/libcutils/tests/PropertiesTest.cpp index f0cdffdc6..79219720e 100644 --- a/libcutils/tests/PropertiesTest.cpp +++ b/libcutils/tests/PropertiesTest.cpp @@ -159,19 +159,68 @@ TEST_F(PropertiesTest, SetString) { TEST_F(PropertiesTest, GetString) { - // Try to use a default value that's too long => set fails + // Try to use a default value that's too long => get truncates the value { ASSERT_OK(property_set(PROPERTY_TEST_KEY, "")); - std::string maxLengthString = std::string(PROPERTY_VALUE_MAX-1, '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_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(); + } } TEST_F(PropertiesTest, GetBool) {