Merge changes Ib074192d,I6df3afed,I69070455,Icbe31908,Id3fa4526,I038d451f

* changes:
  bionic: store property names as variable-length strings
  bionic: prevent root processes from calling __system_property_add
  bionic: revert to a single (larger) property area
  bionic: reimplement property area as hybrid trie/binary tree
  bionic: add missing memory barriers to system properties
  bionic: make property area expandable

libc/bionic/system_properties.c

@@ -34,6 +34,7 @@
 #include <poll.h>
 #include <fcntl.h>
 #include <stdbool.h>
+#include <string.h>
 
 #include <sys/mman.h>
 
@@ -49,31 +50,69 @@
 #include <sys/_system_properties.h>
 
 #include <sys/atomics.h>
+#include <bionic_atomic_inline.h>
+
+#define ALIGN(x, a) (((x) + (a - 1)) & ~(a - 1))
 
 struct prop_area {
-    unsigned volatile count;
+    unsigned bytes_used;
     unsigned volatile serial;
     unsigned magic;
     unsigned version;
-    unsigned reserved[4];
+    unsigned reserved[28];
-    unsigned toc[1];
+    char data[0];
 };
 
 typedef struct prop_area prop_area;
 
 struct prop_info {
-    char name[PROP_NAME_MAX];
     unsigned volatile serial;
     char value[PROP_VALUE_MAX];
+    char name[0];
 };
 
 typedef struct prop_info prop_info;
 
+/*
+ * Properties are stored in a hybrid trie/binary tree structure.
+ * Each property's name is delimited at '.' characters, and the tokens are put
+ * into a trie structure.  Siblings at each level of the trie are stored in a
+ * binary tree.  For instance, "ro.secure"="1" could be stored as follows:
+ *
+ * +-----+   children    +----+   children    +--------+
+ * |     |-------------->| ro |-------------->| secure |
+ * +-----+               +----+               +--------+
+ *                       /    \                /   |
+ *                 left /      \ right   left /    |  prop   +===========+
+ *                     v        v            v     +-------->| ro.secure |
+ *                  +-----+   +-----+     +-----+            +-----------+
+ *                  | net |   | sys |     | com |            |     1     |
+ *                  +-----+   +-----+     +-----+            +===========+
+ */
+
+typedef volatile uint32_t prop_off_t;
+struct prop_bt {
+    uint8_t namelen;
+    uint8_t reserved[3];
+
+    prop_off_t prop;
+
+    prop_off_t left;
+    prop_off_t right;
+
+    prop_off_t children;
+
+    char name[0];
+};
+
+typedef struct prop_bt prop_bt;
+
 static const char property_service_socket[] = "/dev/socket/" PROP_SERVICE_NAME;
+static char property_filename[PATH_MAX] = PROP_FILENAME;
 
-static unsigned dummy_props = 0;
+prop_area *__system_property_area__ = NULL;
 
-prop_area *__system_property_area__ = (void*) &dummy_props;
+const size_t PA_DATA_SIZE = PA_SIZE - sizeof(prop_area);
 
 static int get_fd_from_env(void)
 {
@@ -86,28 +125,85 @@ static int get_fd_from_env(void)
     return atoi(env);
 }
 
-void __system_property_area_init(void *data)
+static int map_prop_area_rw()
 {
-    prop_area *pa = data;
+    prop_area *pa;
+    int fd;
+    int ret;
+
+    /* dev is a tmpfs that we can use to carve a shared workspace
+     * out of, so let's do that...
+     */
+    fd = open(property_filename, O_RDWR | O_CREAT | O_NOFOLLOW | O_CLOEXEC |
+            O_EXCL, 0444);
+    if (fd < 0) {
+        if (errno == EACCES) {
+            /* for consistency with the case where the process has already
+             * mapped the page in and segfaults when trying to write to it
+             */
+            abort();
+        }
+        return -1;
+    }
+
+    ret = fcntl(fd, F_SETFD, FD_CLOEXEC);
+    if (ret < 0)
+        goto out;
+
+    if (ftruncate(fd, PA_SIZE) < 0)
+        goto out;
+
+    pa = mmap(NULL, PA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+    if(pa == MAP_FAILED)
+        goto out;
+
     memset(pa, 0, PA_SIZE);
     pa->magic = PROP_AREA_MAGIC;
     pa->version = PROP_AREA_VERSION;
+    /* reserve root node */
+    pa->bytes_used = sizeof(prop_bt);
 
     /* plug into the lib property services */
     __system_property_area__ = pa;
+
+    close(fd);
+    return 0;
+
+out:
+    close(fd);
+    return -1;
 }
 
-int __system_properties_init(void)
+int __system_property_set_filename(const char *filename)
+{
+    size_t len = strlen(filename);
+    if (len >= sizeof(property_filename))
+        return -1;
+
+    strcpy(property_filename, filename);
+    return 0;
+}
+
+int __system_property_area_init()
+{
+    return map_prop_area_rw();
+}
+
+static int map_prop_area()
 {
     bool fromFile = true;
     int result = -1;
+    int fd;
+    int ret;
 
-    if(__system_property_area__ != ((void*) &dummy_props)) {
+    fd = open(property_filename, O_RDONLY | O_NOFOLLOW | O_CLOEXEC);
-        return 0;
+    if (fd >= 0) {
+        /* For old kernels that don't support O_CLOEXEC */
+        ret = fcntl(fd, F_SETFD, FD_CLOEXEC);
+        if (ret < 0)
+            goto cleanup;
     }
 
-    int fd = open(PROP_FILENAME, O_RDONLY | O_NOFOLLOW);
-
     if ((fd < 0) && (errno == ENOENT)) {
         /*
          * For backwards compatibility, if the file doesn't
@@ -133,24 +229,26 @@ int __system_properties_init(void)
 
     if ((fd_stat.st_uid != 0)
             || (fd_stat.st_gid != 0)
-            || ((fd_stat.st_mode & (S_IWGRP | S_IWOTH)) != 0)) {
+            || ((fd_stat.st_mode & (S_IWGRP | S_IWOTH)) != 0)
+            || (fd_stat.st_size < PA_SIZE) ) {
         goto cleanup;
     }
 
-    prop_area *pa = mmap(NULL, fd_stat.st_size, PROT_READ, MAP_SHARED, fd, 0);
+    prop_area *pa = mmap(NULL, PA_SIZE, PROT_READ, MAP_SHARED, fd, 0);
 
     if (pa == MAP_FAILED) {
         goto cleanup;
     }
 
     if((pa->magic != PROP_AREA_MAGIC) || (pa->version != PROP_AREA_VERSION)) {
-        munmap(pa, fd_stat.st_size);
+        munmap(pa, PA_SIZE);
         goto cleanup;
     }
 
-    __system_property_area__ = pa;
     result = 0;
 
+    __system_property_area__ = pa;
+
 cleanup:
     if (fromFile) {
         close(fd);
@@ -159,57 +257,169 @@ cleanup:
     return result;
 }
 
-int __system_property_foreach(
+int __system_properties_init()
-        void (*propfn)(const prop_info *pi, void *cookie),
-        void *cookie)
 {
-    prop_area *pa = __system_property_area__;
+    return map_prop_area();
-    unsigned i;
-
-    for (i = 0; i < pa->count; i++) {
-        unsigned entry = pa->toc[i];
-        prop_info *pi = TOC_TO_INFO(pa, entry);
-        propfn(pi, cookie);
-    }
-
-    return 0;
 }
 
-const prop_info *__system_property_find_nth(unsigned n)
+static void *new_prop_obj(size_t size, prop_off_t *off)
 {
     prop_area *pa = __system_property_area__;
+    size = ALIGN(size, sizeof(uint32_t));
 
-    if(n >= pa->count) {
+    if (pa->bytes_used + size > PA_DATA_SIZE)
-        return 0;
+        return NULL;
+
+    *off = pa->bytes_used;
+    __system_property_area__->bytes_used += size;
+    return __system_property_area__->data + *off;
+}
+
+static prop_bt *new_prop_bt(const char *name, uint8_t namelen, prop_off_t *off)
+{
+    prop_off_t off_tmp;
+    prop_bt *bt = new_prop_obj(sizeof(prop_bt) + namelen + 1, &off_tmp);
+    if (bt) {
+        memcpy(bt->name, name, namelen);
+        bt->name[namelen] = '\0';
+        bt->namelen = namelen;
+        ANDROID_MEMBAR_FULL();
+        *off = off_tmp;
+    }
+
+    return bt;
+}
+
+static prop_info *new_prop_info(const char *name, uint8_t namelen,
+        const char *value, uint8_t valuelen, prop_off_t *off)
+{
+    prop_off_t off_tmp;
+    prop_info *info = new_prop_obj(sizeof(prop_info) + namelen + 1, &off_tmp);
+    if (info) {
+        memcpy(info->name, name, namelen);
+        info->name[namelen] = '\0';
+        info->serial = (valuelen << 24);
+        memcpy(info->value, value, valuelen);
+        info->value[valuelen] = '\0';
+        ANDROID_MEMBAR_FULL();
+        *off = off_tmp;
+    }
+
+    return info;
+}
+
+static void *to_prop_obj(prop_off_t off)
+{
+    if (off > PA_DATA_SIZE)
+        return NULL;
+
+    return __system_property_area__->data + off;
+}
+
+static prop_bt *root_node()
+{
+    return to_prop_obj(0);
+}
+
+static int cmp_prop_name(const char *one, uint8_t one_len, const char *two,
+        uint8_t two_len)
+{
+    if (one_len < two_len)
+        return -1;
+    else if (one_len > two_len)
+        return 1;
+    else
+        return strncmp(one, two, one_len);
+}
+
+static prop_bt *find_prop_bt(prop_bt *bt, const char *name, uint8_t namelen,
+        bool alloc_if_needed)
+{
+    while (true) {
+        int ret;
+        if (!bt)
+            return bt;
+        ret = cmp_prop_name(name, namelen, bt->name, bt->namelen);
+
+        if (ret == 0) {
+            return bt;
+        } else if (ret < 0) {
+            if (bt->left) {
+                bt = to_prop_obj(bt->left);
+            } else {
+                if (!alloc_if_needed)
+                   return NULL;
+
+                bt = new_prop_bt(name, namelen, &bt->left);
+            }
+        } else {
+            if (bt->right) {
+                bt = to_prop_obj(bt->right);
+            } else {
+                if (!alloc_if_needed)
+                   return NULL;
+
+                bt = new_prop_bt(name, namelen, &bt->right);
+            }
+        }
+    }
+}
+
+static const prop_info *find_property(prop_bt *trie, const char *name,
+        uint8_t namelen, const char *value, uint8_t valuelen,
+        bool alloc_if_needed)
+{
+    const char *remaining_name = name;
+
+    while (true) {
+        char *sep = strchr(remaining_name, '.');
+        bool want_subtree = (sep != NULL);
+        uint8_t substr_size;
+
+        prop_bt *root;
+
+        if (want_subtree) {
+            substr_size = sep - remaining_name;
+        } else {
+            substr_size = strlen(remaining_name);
+        }
+
+        if (!substr_size)
+            return NULL;
+
+        if (trie->children) {
+            root = to_prop_obj(trie->children);
+        } else if (alloc_if_needed) {
+            root = new_prop_bt(remaining_name, substr_size, &trie->children);
+        } else {
+            root = NULL;
+        }
+
+        if (!root)
+            return NULL;
+
+        trie = find_prop_bt(root, remaining_name, substr_size, alloc_if_needed);
+        if (!trie)
+            return NULL;
+
+        if (!want_subtree)
+            break;
+
+        remaining_name = sep + 1;
+    }
+
+    if (trie->prop) {
+        return to_prop_obj(trie->prop);
+    } else if (alloc_if_needed) {
+        return new_prop_info(name, namelen, value, valuelen, &trie->prop);
     } else {
-        return TOC_TO_INFO(pa, pa->toc[n]);
+        return NULL;
     }
 }
 
 const prop_info *__system_property_find(const char *name)
 {
-    prop_area *pa = __system_property_area__;
+    return find_property(root_node(), name, strlen(name), NULL, 0, false);
-    unsigned count = pa->count;
-    unsigned *toc = pa->toc;
-    unsigned len = strlen(name);
-    prop_info *pi;
-
-    if (len >= PROP_NAME_MAX)
-        return 0;
-    if (len < 1)
-        return 0;
-
-    while(count--) {
-        unsigned entry = *toc++;
-        if(TOC_NAME_LEN(entry) != len) continue;
-
-        pi = TOC_TO_INFO(pa, entry);
-        if(memcmp(name, pi->name, len)) continue;
-
-        return pi;
-    }
-
-    return 0;
 }
 
 int __system_property_read(const prop_info *pi, char *name, char *value)
@@ -224,6 +434,7 @@ int __system_property_read(const prop_info *pi, char *name, char *value)
         }
         len = SERIAL_VALUE_LEN(serial);
         memcpy(value, pi->value, len + 1);
+        ANDROID_MEMBAR_FULL();
         if(serial == pi->serial) {
             if(name != 0) {
                 strcpy(name, pi->name);
@@ -355,7 +566,9 @@ int __system_property_update(prop_info *pi, const char *value, unsigned int len)
         return -1;
 
     pi->serial = pi->serial | 1;
+    ANDROID_MEMBAR_FULL();
     memcpy(pi->value, value, len + 1);
+    ANDROID_MEMBAR_FULL();
     pi->serial = (len << 24) | ((pi->serial + 1) & 0xffffff);
     __futex_wake(&pi->serial, INT32_MAX);
 
@@ -369,11 +582,8 @@ int __system_property_add(const char *name, unsigned int namelen,
             const char *value, unsigned int valuelen)
 {
     prop_area *pa = __system_property_area__;
-    prop_info *pa_info_array = (void*) (((char*) pa) + PA_INFO_START);
+    const prop_info *pi;
-    prop_info *pi;
 
-    if (pa->count == PA_COUNT_MAX)
-        return -1;
     if (namelen >= PROP_NAME_MAX)
         return -1;
     if (valuelen >= PROP_VALUE_MAX)
@@ -381,18 +591,12 @@ int __system_property_add(const char *name, unsigned int namelen,
     if (namelen < 1)
         return -1;
 
-    pi = pa_info_array + pa->count;
+    pi = find_property(root_node(), name, namelen, value, valuelen, true);
-    pi->serial = (valuelen << 24);
+    if (!pi)
-    memcpy(pi->name, name, namelen + 1);
+        return -1;
-    memcpy(pi->value, value, valuelen + 1);
 
-    pa->toc[pa->count] =
-        (namelen << 24) | (((unsigned) pi) - ((unsigned) pa));
-
-    pa->count++;
     pa->serial++;
     __futex_wake(&pa->serial, INT32_MAX);
-
     return 0;
 }
 
@@ -411,3 +615,72 @@ unsigned int __system_property_wait_any(unsigned int serial)
 
     return pa->serial;
 }
+
+struct find_nth_cookie {
+    unsigned count;
+    unsigned n;
+    const prop_info *pi;
+};
+
+static void find_nth_fn(const prop_info *pi, void *ptr)
+{
+    struct find_nth_cookie *cookie = ptr;
+
+    if (cookie->n == cookie->count)
+        cookie->pi = pi;
+
+    cookie->count++;
+}
+
+const prop_info *__system_property_find_nth(unsigned n)
+{
+    struct find_nth_cookie cookie;
+    int err;
+
+    memset(&cookie, 0, sizeof(cookie));
+    cookie.n = n;
+
+    err = __system_property_foreach(find_nth_fn, &cookie);
+    if (err < 0)
+        return NULL;
+
+    return cookie.pi;
+}
+
+static int foreach_property(prop_off_t off,
+        void (*propfn)(const prop_info *pi, void *cookie), void *cookie)
+{
+    prop_bt *trie = to_prop_obj(off);
+    if (!trie)
+        return -1;
+
+    if (trie->left) {
+        int err = foreach_property(trie->left, propfn, cookie);
+        if (err < 0)
+            return -1;
+    }
+    if (trie->prop) {
+        prop_info *info = to_prop_obj(trie->prop);
+        if (!info)
+            return -1;
+        propfn(info, cookie);
+    }
+    if (trie->children) {
+        int err = foreach_property(trie->children, propfn, cookie);
+        if (err < 0)
+            return -1;
+    }
+    if (trie->right) {
+        int err = foreach_property(trie->right, propfn, cookie);
+        if (err < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+int __system_property_foreach(void (*propfn)(const prop_info *pi, void *cookie),
+        void *cookie)
+{
+    return foreach_property(0, propfn, cookie);
+}

libc/include/sys/_system_properties.h

@@ -37,20 +37,12 @@
 typedef struct prop_msg prop_msg;
 
 #define PROP_AREA_MAGIC   0x504f5250
-#define PROP_AREA_VERSION 0x45434f76
+#define PROP_AREA_VERSION 0xfc6ed0ab
 
 #define PROP_SERVICE_NAME "property_service"
 #define PROP_FILENAME "/dev/__properties__"
 
-/* (8 header words + 247 toc words) = 1020 bytes */
+#define PA_SIZE         (128 * 1024)
-/* 1024 bytes header and toc + 247 prop_infos @ 128 bytes = 32640 bytes */
-
-#define PA_COUNT_MAX  247
-#define PA_INFO_START 1024
-#define PA_SIZE       32768
-
-#define TOC_NAME_LEN(toc)       ((toc) >> 24)
-#define TOC_TO_INFO(area, toc)  ((prop_info*) (((char*) area) + ((toc) & 0xFFFFFF)))
 
 #define SERIAL_VALUE_LEN(serial) ((serial) >> 24)
 #define SERIAL_DIRTY(serial) ((serial) & 1)
@@ -83,11 +75,6 @@ struct prop_msg
 **   1. pi->serial = pi->serial | 1
 **   2. memcpy(pi->value, local_value, value_len)
 **   3. pi->serial = (value_len << 24) | ((pi->serial + 1) & 0xffffff)
-**
-** Improvements:
-** - maintain the toc sorted by pi->name to allow lookup
-**   by binary search
-**
 */
 
 #define PROP_PATH_RAMDISK_DEFAULT  "/default.prop"
@@ -96,12 +83,18 @@ struct prop_msg
 #define PROP_PATH_LOCAL_OVERRIDE   "/data/local.prop"
 #define PROP_PATH_FACTORY          "/factory/factory.prop"
 
+/*
+** Map the property area from the specified filename.  This
+** method is for testing only.
+*/
+int __system_property_set_filename(const char *filename);
+
 /*
 ** Initialize the area to be used to store properties.  Can
 ** only be done by a single process that has write access to
 ** the property area.
 */
-void __system_property_area_init(void *data);
+int __system_property_area_init();
 
 /* Add a new system property.  Can only be done by a single
 ** process that has write access to the property area, and

tests/property_benchmark.cpp

@@ -15,23 +15,38 @@
  */
 
 #include "benchmark.h"
+#include <unistd.h>
 
 #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
 #include <sys/_system_properties.h>
 
 #include <vector>
+#include <string>
 
 extern void *__system_property_area__;
 
 #define TEST_NUM_PROPS \
-    Arg(1)->Arg(4)->Arg(16)->Arg(64)->Arg(128)->Arg(247)
+    Arg(1)->Arg(4)->Arg(16)->Arg(64)->Arg(128)->Arg(256)->Arg(512)->Arg(1024)
 
 struct LocalPropertyTestState {
-    LocalPropertyTestState(int nprops) : nprops(nprops) {
+    LocalPropertyTestState(int nprops) : nprops(nprops), valid(false) {
         static const char prop_name_chars[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.-_";
+
+        char dir_template[] = "/data/nativetest/prop-XXXXXX";
+        char *dirname = mkdtemp(dir_template);
+        if (!dirname) {
+            perror("making temp file for test state failed (is /data/nativetest writable?)");
+            return;
+        }
+
         old_pa = __system_property_area__;
-        pa = malloc(PA_SIZE);
+        __system_property_area__ = NULL;
-        __system_property_area_init(pa);
+
+        pa_dirname = dirname;
+        pa_filename = pa_dirname + "/__properties__";
+
+        __system_property_set_filename(pa_filename.c_str());
+        __system_property_area_init();
 
         names = new char* [nprops];
         name_lens = new int[nprops];
@@ -54,10 +69,20 @@ struct LocalPropertyTestState {
             }
             __system_property_add(names[i], name_lens[i], values[i], value_lens[i]);
         }
+
+        valid = true;
     }
 
     ~LocalPropertyTestState() {
+        if (!valid)
+            return;
+
         __system_property_area__ = old_pa;
+
+        __system_property_set_filename(PROP_FILENAME);
+        unlink(pa_filename.c_str());
+        rmdir(pa_dirname.c_str());
+
         for (int i = 0; i < nprops; i++) {
             delete names[i];
             delete values[i];
@@ -66,7 +91,6 @@ struct LocalPropertyTestState {
         delete[] name_lens;
         delete[] values;
         delete[] value_lens;
-        free(pa);
     }
 public:
     const int nprops;
@@ -74,9 +98,11 @@ public:
     int *name_lens;
     char **values;
     int *value_lens;
+    bool valid;
 
 private:
-    void *pa;
+    std::string pa_dirname;
+    std::string pa_filename;
     void *old_pa;
 };
 
@@ -87,6 +113,9 @@ static void BM_property_get(int iters, int nprops)
     LocalPropertyTestState pa(nprops);
     char value[PROP_VALUE_MAX];
 
+    if (!pa.valid)
+        return;
+
     srandom(iters * nprops);
 
     StartBenchmarkTiming();
@@ -104,6 +133,9 @@ static void BM_property_find(int iters, int nprops)
 
     LocalPropertyTestState pa(nprops);
 
+    if (!pa.valid)
+        return;
+
     srandom(iters * nprops);
 
     StartBenchmarkTiming();

tests/system_properties_test.cpp

@@ -16,6 +16,8 @@
 
 #include <gtest/gtest.h>
 #include <sys/wait.h>
+#include <unistd.h>
+#include <string>
 
 #if __BIONIC__
 
@@ -25,23 +27,46 @@
 extern void *__system_property_area__;
 
 struct LocalPropertyTestState {
-    LocalPropertyTestState() {
+    LocalPropertyTestState() : valid(false) {
+        char dir_template[] = "/data/nativetest/prop-XXXXXX";
+        char *dirname = mkdtemp(dir_template);
+        if (!dirname) {
+            perror("making temp file for test state failed (is /data/nativetest writable?)");
+            return;
+        }
+
         old_pa = __system_property_area__;
-        pa = malloc(PA_SIZE);
+        __system_property_area__ = NULL;
-        __system_property_area_init(pa);
+
+        pa_dirname = dirname;
+        pa_filename = pa_dirname + "/__properties__";
+
+        __system_property_set_filename(pa_filename.c_str());
+        __system_property_area_init();
+        valid = true;
     }
 
     ~LocalPropertyTestState() {
+        if (!valid)
+            return;
+
         __system_property_area__ = old_pa;
-        free(pa);
+
+        __system_property_set_filename(PROP_FILENAME);
+        unlink(pa_filename.c_str());
+        rmdir(pa_dirname.c_str());
     }
+public:
+    bool valid;
 private:
-    void *pa;
+    std::string pa_dirname;
+    std::string pa_filename;
     void *old_pa;
 };
 
 TEST(properties, add) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     char propvalue[PROP_VALUE_MAX];
 
@@ -61,6 +86,7 @@ TEST(properties, add) {
 
 TEST(properties, update) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     char propvalue[PROP_VALUE_MAX];
     prop_info *pi;
@@ -91,27 +117,34 @@ TEST(properties, update) {
     ASSERT_STREQ(propvalue, "value6");
 }
 
-// 247 = max # of properties supported by current implementation
+TEST(properties, fill) {
-// (this should never go down)
-TEST(properties, fill_247) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     char prop_name[PROP_NAME_MAX];
     char prop_value[PROP_VALUE_MAX];
     char prop_value_ret[PROP_VALUE_MAX];
+    int count = 0;
     int ret;
 
-    for (int i = 0; i < 247; i++) {
+    while (true) {
-        ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", i);
+        ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", count);
         memset(prop_name + ret, 'a', PROP_NAME_MAX - 1 - ret);
-        ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", i);
+        ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", count);
         memset(prop_value + ret, 'b', PROP_VALUE_MAX - 1 - ret);
         prop_name[PROP_NAME_MAX - 1] = 0;
         prop_value[PROP_VALUE_MAX - 1] = 0;
 
-        ASSERT_EQ(0, __system_property_add(prop_name, PROP_NAME_MAX - 1, prop_value, PROP_VALUE_MAX - 1));
+        ret = __system_property_add(prop_name, PROP_NAME_MAX - 1, prop_value, PROP_VALUE_MAX - 1);
+        if (ret < 0)
+            break;
+
+        count++;
     }
 
-    for (int i = 0; i < 247; i++) {
+    // For historical reasons at least 247 properties must be supported
+    ASSERT_GE(count, 247);
+
+    for (int i = 0; i < count; i++) {
         ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", i);
         memset(prop_name + ret, 'a', PROP_NAME_MAX - 1 - ret);
         ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", i);
@@ -134,6 +167,7 @@ static void foreach_test_callback(const prop_info *pi, void* cookie) {
 
 TEST(properties, foreach) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     size_t count = 0;
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
@@ -146,6 +180,7 @@ TEST(properties, foreach) {
 
 TEST(properties, find_nth) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
     ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6));
@@ -165,6 +200,7 @@ TEST(properties, find_nth) {
 
 TEST(properties, errors) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     char prop_value[PROP_NAME_MAX];
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
@@ -181,6 +217,7 @@ TEST(properties, errors) {
 
 TEST(properties, serial) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     const prop_info *pi;
     unsigned int serial;
 
@@ -206,6 +243,7 @@ static void *PropertyWaitHelperFn(void *arg)
 
 TEST(properties, wait) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     unsigned int serial;
     prop_info *pi;
     pthread_t t;