Merge "libcutils: reimplement system property functions with libbase."

4 files changed, 191 insertions, 197 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 <sys/cdefs.h>
 #include <stddef.h>
-#include <sys/system_properties.h>
 #include <stdint.h>
 
+#if __has_include(<sys/system_properties.h>)
+#include <sys/system_properties.h>
+#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 <android-base/properties.h> 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 <cutils/properties.h>
 
-#define LOG_TAG "properties"
-// #define LOG_NDEBUG 0
-
-#include <assert.h>
-#include <ctype.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-#include <cutils/sockets.h>
-#include <log/log.h>
+#include <android-base/properties.h>
 
-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 <typename T>
+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<T>::min() &&
+        v <= std::numeric_limits<T>::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<int64_t>(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<int32_t>(key, default_value);
 }
 
-#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
-#include <sys/_system_properties.h>
-
-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(<sys/system_properties.h>)
+
+#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
+#include <sys/_system_properties.h>
+
 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();
-    }
+    // 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, 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, ""));
+    ASSERT_OK(property_set(PROPERTY_TEST_KEY, ""));
 
-        std::string maxLengthString = std::string(PROPERTY_VALUE_MAX - 1, 'b');
+    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();
-    }
+    // 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, ""));
+    ASSERT_OK(property_set(PROPERTY_TEST_KEY, ""));
 
-        std::string oneCharString = std::string(1, 'c');
+    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();
-    }
+    // 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, ""));
+    ASSERT_OK(property_set(PROPERTY_TEST_KEY, ""));
 
-        std::string zeroCharString = std::string(0, 'd');
+    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();
-    }
+    // 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, ""));
+    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 a return value of 0
+    int len = property_get(PROPERTY_TEST_KEY, mValue, NULL);
+    EXPECT_EQ(0, len);
+    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_bool_0) {
+    ASSERT_OK(property_set(PROPERTY_TEST_KEY, "0"));
+    ASSERT_FALSE(property_get_bool(PROPERTY_TEST_KEY, true));
+}
 
-    /**
-     * 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] << "'";
-    }
+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));
+}
 
-    /**
-     * NEITHER
-     */
+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);