From 5cf32de7a0fea0b10959b598300babc6e4f54d95 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 23 Jan 2013 23:07:06 -0800 Subject: [PATCH 1/2] bionic: move system property writing from init to bionic Move the implementation of writing to the system property area from init to bionic, next to the reader implementation. This will allow full property testing to be added to bionic tests. Add new accessor and waiting functions to hide the implementation from watchprops and various bionic users. Also hide some of the implementation details of the property area from init by moving them into _system_properties.h, and other details from everybody by moving them into system_properties.h. (cherry picked from commit dc1038b7900acb664e99643d2974e1a0f4703781) Change-Id: I192d3825ee276c5047bc751039fe6cfe226a7cca --- libc/bionic/system_properties.c | 101 ++++++++++++++++++++++++++ libc/include/sys/_system_properties.h | 66 ++++++++++++----- libc/netbsd/resolv/res_state.c | 6 +- 3 files changed, 153 insertions(+), 20 deletions(-) diff --git a/libc/bionic/system_properties.c b/libc/bionic/system_properties.c index 0587430a5..d96950709 100644 --- a/libc/bionic/system_properties.c +++ b/libc/bionic/system_properties.c @@ -26,6 +26,7 @@ * SUCH DAMAGE. */ #include +#include #include #include #include @@ -49,6 +50,25 @@ #include +struct prop_area { + unsigned volatile count; + unsigned volatile serial; + unsigned magic; + unsigned version; + unsigned reserved[4]; + unsigned toc[1]; +}; + +typedef struct prop_area prop_area; + +struct prop_info { + char name[PROP_NAME_MAX]; + unsigned volatile serial; + char value[PROP_VALUE_MAX]; +}; + +typedef struct prop_info prop_info; + static const char property_service_socket[] = "/dev/socket/" PROP_SERVICE_NAME; static unsigned dummy_props = 0; @@ -66,6 +86,17 @@ static int get_fd_from_env(void) return atoi(env); } +void __system_property_area_init(void *data) +{ + prop_area *pa = data; + memset(pa, 0, PA_SIZE); + pa->magic = PROP_AREA_MAGIC; + pa->version = PROP_AREA_VERSION; + + /* plug into the lib property services */ + __system_property_area__ = pa; +} + int __system_properties_init(void) { bool fromFile = true; @@ -147,6 +178,11 @@ const prop_info *__system_property_find(const char *name) 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; @@ -294,3 +330,68 @@ int __system_property_wait(const prop_info *pi) } return 0; } + +int __system_property_update(prop_info *pi, const char *value, unsigned int len) +{ + prop_area *pa = __system_property_area__; + + if (len >= PROP_VALUE_MAX) + return -1; + + pi->serial = pi->serial | 1; + memcpy(pi->value, value, len + 1); + pi->serial = (len << 24) | ((pi->serial + 1) & 0xffffff); + __futex_wake(&pi->serial, INT32_MAX); + + pa->serial++; + __futex_wake(&pa->serial, INT32_MAX); + + return 0; +} + +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); + prop_info *pi; + + if (pa->count == PA_COUNT_MAX) + return -1; + if (namelen >= PROP_NAME_MAX) + return -1; + if (valuelen >= PROP_VALUE_MAX) + return -1; + if (namelen < 1) + return -1; + + pi = pa_info_array + pa->count; + pi->serial = (valuelen << 24); + memcpy(pi->name, name, namelen + 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; +} + +unsigned int __system_property_serial(const prop_info *pi) +{ + return pi->serial; +} + +unsigned int __system_property_wait_any(unsigned int serial) +{ + prop_area *pa = __system_property_area__; + + do { + __futex_wait(&pa->serial, serial, 0); + } while(pa->serial == serial); + + return pa->serial; +} diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h index 5d2043d24..c5bc2235b 100644 --- a/libc/include/sys/_system_properties.h +++ b/libc/include/sys/_system_properties.h @@ -34,7 +34,6 @@ #else #include -typedef struct prop_area prop_area; typedef struct prop_msg prop_msg; #define PROP_AREA_MAGIC 0x504f5250 @@ -43,29 +42,20 @@ typedef struct prop_msg prop_msg; #define PROP_SERVICE_NAME "property_service" #define PROP_FILENAME "/dev/__properties__" -/* #define PROP_MAX_ENTRIES 247 */ -/* 247 -> 32620 bytes (<32768) */ +/* (8 header words + 247 toc words) = 1020 bytes */ +/* 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))) -struct prop_area { - unsigned volatile count; - unsigned volatile serial; - unsigned magic; - unsigned version; - unsigned reserved[4]; - unsigned toc[1]; -}; - #define SERIAL_VALUE_LEN(serial) ((serial) >> 24) #define SERIAL_DIRTY(serial) ((serial) & 1) -struct prop_info { - char name[PROP_NAME_MAX]; - unsigned volatile serial; - char value[PROP_VALUE_MAX]; -}; +__BEGIN_DECLS struct prop_msg { @@ -106,5 +96,47 @@ struct prop_msg #define PROP_PATH_LOCAL_OVERRIDE "/data/local.prop" #define PROP_PATH_FACTORY "/factory/factory.prop" +/* +** 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); + +/* Add a new system property. Can only be done by a single +** process that has write access to the property area, and +** that process must handle sequencing to ensure the property +** does not already exist and that only one property is added +** or updated at a time. +** +** Returns 0 on success, -1 if the property area is full. +*/ +int __system_property_add(const char *name, unsigned int namelen, + const char *value, unsigned int valuelen); + +/* Update the value of a system property returned by +** __system_property_find. Can only be done by a single process +** that has write access to the property area, and that process +** must handle sequencing to ensure that only one property is +** updated at a time. +** +** Returns 0 on success, -1 if the parameters are incorrect. +*/ +int __system_property_update(prop_info *pi, const char *value, unsigned int len); + +/* Read the serial number of a system property returned by +** __system_property_find. +** +** Returns the serial number on success, -1 on error. +*/ +unsigned int __system_property_serial(const prop_info *pi); + +/* Wait for any system property to be updated. Caller must pass +** in 0 the first time, and the previous return value on each +** successive call. */ +unsigned int __system_property_wait_any(unsigned int serial); + +__END_DECLS + #endif #endif diff --git a/libc/netbsd/resolv/res_state.c b/libc/netbsd/resolv/res_state.c index 3e1f67b82..2b34867f7 100644 --- a/libc/netbsd/resolv/res_state.c +++ b/libc/netbsd/resolv/res_state.c @@ -71,7 +71,7 @@ _res_thread_alloc(void) rt->_serial = 0; rt->_pi = (struct prop_info*) __system_property_find("net.change"); if (rt->_pi) { - rt->_serial = rt->_pi->serial; + rt->_serial = __system_property_serial(rt->_pi); } memset(rt->_rstatic, 0, sizeof rt->_rstatic); } @@ -135,14 +135,14 @@ _res_thread_get(void) return rt; } } - if (rt->_serial == rt->_pi->serial) { + if (rt->_serial == __system_property_serial(rt->_pi)) { /* Nothing changed, so return the current state */ D("%s: tid=%d rt=%p nothing changed, returning", __FUNCTION__, gettid(), rt); return rt; } /* Update the recorded serial number, and go reset the state */ - rt->_serial = rt->_pi->serial; + rt->_serial = __system_property_serial(rt->_pi); goto RESET_STATE; } From b27e200ad6170ba3163f5ae6ba581bdaabb2e696 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 28 Jan 2013 17:19:43 -0800 Subject: [PATCH 2/2] bionic: add tests for properties (cherry picked from commit 37d9f75dde881a0ba1c1b3253b1be19d4096963d) Change-Id: Iac00ce10a4272032a1cbdbc4204277d6876e3365 --- tests/Android.mk | 2 + tests/property_benchmark.cpp | 116 ++++++++++++++++ tests/system_properties_test.cpp | 229 +++++++++++++++++++++++++++++++ 3 files changed, 347 insertions(+) create mode 100644 tests/property_benchmark.cpp create mode 100644 tests/system_properties_test.cpp diff --git a/tests/Android.mk b/tests/Android.mk index 875746dcd..c65d20c34 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -31,6 +31,7 @@ benchmark_c_flags = \ benchmark_src_files = \ benchmark_main.cpp \ math_benchmark.cpp \ + property_benchmark.cpp \ string_benchmark.cpp \ time_benchmark.cpp \ @@ -78,6 +79,7 @@ test_src_files = \ string_test.cpp \ strings_test.cpp \ stubs_test.cpp \ + system_properties_test.cpp \ time_test.cpp \ unistd_test.cpp \ diff --git a/tests/property_benchmark.cpp b/tests/property_benchmark.cpp new file mode 100644 index 000000000..dbd11adcd --- /dev/null +++ b/tests/property_benchmark.cpp @@ -0,0 +1,116 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "benchmark.h" + +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include + +#include + +extern void *__system_property_area__; + +#define TEST_NUM_PROPS \ + Arg(1)->Arg(4)->Arg(16)->Arg(64)->Arg(128)->Arg(247) + +struct LocalPropertyTestState { + LocalPropertyTestState(int nprops) : nprops(nprops) { + static const char prop_name_chars[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.-_"; + old_pa = __system_property_area__; + pa = malloc(PA_SIZE); + __system_property_area_init(pa); + + names = new char* [nprops]; + name_lens = new int[nprops]; + values = new char* [nprops]; + value_lens = new int[nprops]; + + srandom(nprops); + + for (int i = 0; i < nprops; i++) { + name_lens[i] = random() % PROP_NAME_MAX; + names[i] = new char[PROP_NAME_MAX + 1]; + for (int j = 0; j < name_lens[i]; j++) { + names[i][j] = prop_name_chars[random() % (sizeof(prop_name_chars) - 1)]; + } + names[i][name_lens[i]] = 0; + value_lens[i] = random() % PROP_VALUE_MAX; + values[i] = new char[PROP_VALUE_MAX]; + for (int j = 0; j < value_lens[i]; j++) { + values[i][j] = prop_name_chars[random() % (sizeof(prop_name_chars) - 1)]; + } + __system_property_add(names[i], name_lens[i], values[i], value_lens[i]); + } + } + + ~LocalPropertyTestState() { + __system_property_area__ = old_pa; + for (int i = 0; i < nprops; i++) { + delete names[i]; + delete values[i]; + } + delete names; + delete name_lens; + delete values; + delete value_lens; + free(pa); + } +public: + const int nprops; + char **names; + int *name_lens; + char **values; + int *value_lens; + +private: + void *pa; + void *old_pa; +}; + +static void BM_property_get(int iters, int nprops) +{ + StopBenchmarkTiming(); + + LocalPropertyTestState pa(nprops); + char value[PROP_VALUE_MAX]; + + StartBenchmarkTiming(); + + for (int i = 0; i < iters; i++) { + for (int j = 0; j < nprops; j++) { + __system_property_get(pa.names[j], value); + } + } + StopBenchmarkTiming(); +} +BENCHMARK(BM_property_get)->TEST_NUM_PROPS; + +static void BM_property_find(int iters, int nprops) +{ + StopBenchmarkTiming(); + + LocalPropertyTestState pa(nprops); + + StartBenchmarkTiming(); + + for (int i = 0; i < iters; i++) { + for (int j = 0; j < nprops; j++) { + __system_property_find(pa.names[j]); + } + } + StopBenchmarkTiming(); +} +BENCHMARK(BM_property_find)->TEST_NUM_PROPS; diff --git a/tests/system_properties_test.cpp b/tests/system_properties_test.cpp new file mode 100644 index 000000000..60188f41d --- /dev/null +++ b/tests/system_properties_test.cpp @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#if __BIONIC__ + +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include + +extern void *__system_property_area__; + +struct LocalPropertyTestState { + LocalPropertyTestState() { + old_pa = __system_property_area__; + pa = malloc(PA_SIZE); + __system_property_area_init(pa); + } + + ~LocalPropertyTestState() { + __system_property_area__ = old_pa; + free(pa); + } +private: + void *pa; + void *old_pa; +}; + +TEST(properties, add) { + LocalPropertyTestState pa; + + char propvalue[PROP_VALUE_MAX]; + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); + ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); + + ASSERT_EQ(6, __system_property_get("property", propvalue)); + ASSERT_STREQ(propvalue, "value1"); + + ASSERT_EQ(6, __system_property_get("other_property", propvalue)); + ASSERT_STREQ(propvalue, "value2"); + + ASSERT_EQ(6, __system_property_get("property_other", propvalue)); + ASSERT_STREQ(propvalue, "value3"); +} + +TEST(properties, update) { + LocalPropertyTestState pa; + + char propvalue[PROP_VALUE_MAX]; + prop_info *pi; + + ASSERT_EQ(0, __system_property_add("property", 8, "oldvalue1", 9)); + ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); + ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); + + pi = (prop_info *)__system_property_find("property"); + ASSERT_NE((prop_info *)NULL, pi); + __system_property_update(pi, "value4", 6); + + pi = (prop_info *)__system_property_find("other_property"); + ASSERT_NE((prop_info *)NULL, pi); + __system_property_update(pi, "newvalue5", 9); + + pi = (prop_info *)__system_property_find("property_other"); + ASSERT_NE((prop_info *)NULL, pi); + __system_property_update(pi, "value6", 6); + + ASSERT_EQ(6, __system_property_get("property", propvalue)); + ASSERT_STREQ(propvalue, "value4"); + + ASSERT_EQ(9, __system_property_get("other_property", propvalue)); + ASSERT_STREQ(propvalue, "newvalue5"); + + ASSERT_EQ(6, __system_property_get("property_other", propvalue)); + ASSERT_STREQ(propvalue, "value6"); +} + +// 247 = max # of properties supported by current implementation +// (this should never go down) +TEST(properties, fill_247) { + LocalPropertyTestState pa; + char prop_name[PROP_NAME_MAX]; + char prop_value[PROP_VALUE_MAX]; + char prop_value_ret[PROP_VALUE_MAX]; + int ret; + + for (int i = 0; i < 247; 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); + 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)); + } + + for (int i = 0; i < 247; 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); + memset(prop_value + ret, 'b', PROP_VALUE_MAX - 1 - ret); + prop_name[PROP_NAME_MAX - 1] = 0; + prop_value[PROP_VALUE_MAX - 1] = 0; + memset(prop_value_ret, '\0', PROP_VALUE_MAX); + + ASSERT_EQ(PROP_VALUE_MAX - 1, __system_property_get(prop_name, prop_value_ret)); + ASSERT_EQ(0, memcmp(prop_value, prop_value_ret, PROP_VALUE_MAX)); + } +} + +TEST(properties, find_nth) { + LocalPropertyTestState pa; + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); + ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); + + ASSERT_NE((const prop_info *)NULL, __system_property_find_nth(0)); + ASSERT_NE((const prop_info *)NULL, __system_property_find_nth(1)); + ASSERT_NE((const prop_info *)NULL, __system_property_find_nth(2)); + + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(3)); + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(4)); + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(5)); + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(100)); + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(200)); + ASSERT_EQ((const prop_info *)NULL, __system_property_find_nth(247)); +} + +TEST(properties, errors) { + LocalPropertyTestState pa; + char prop_value[PROP_NAME_MAX]; + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); + ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); + + ASSERT_EQ(0, __system_property_find("property1")); + ASSERT_EQ(0, __system_property_get("property1", prop_value)); + + ASSERT_EQ(-1, __system_property_add("name", PROP_NAME_MAX, "value", 5)); + ASSERT_EQ(-1, __system_property_add("name", 4, "value", PROP_VALUE_MAX)); + ASSERT_EQ(-1, __system_property_update(NULL, "value", PROP_VALUE_MAX)); +} + +TEST(properties, serial) { + LocalPropertyTestState pa; + const prop_info *pi; + unsigned int serial; + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + ASSERT_NE((const prop_info *)NULL, pi = __system_property_find("property")); + serial = __system_property_serial(pi); + ASSERT_EQ(0, __system_property_update((prop_info *)pi, "value2", 6)); + ASSERT_NE(serial, __system_property_serial(pi)); +} + +static void *PropertyWaitHelperFn(void *arg) +{ + int *flag = (int *)arg; + prop_info *pi; + pi = (prop_info *)__system_property_find("property"); + usleep(100000); + + *flag = 1; + __system_property_update(pi, "value3", 6); + + return NULL; +} + +TEST(properties, wait) { + LocalPropertyTestState pa; + unsigned int serial; + prop_info *pi; + pthread_t t; + int flag = 0; + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + serial = __system_property_wait_any(0); + pi = (prop_info *)__system_property_find("property"); + ASSERT_NE((prop_info *)NULL, pi); + __system_property_update(pi, "value2", 6); + serial = __system_property_wait_any(serial); + + ASSERT_EQ(0, pthread_create(&t, NULL, PropertyWaitHelperFn, &flag)); + ASSERT_EQ(flag, 0); + serial = __system_property_wait_any(serial); + ASSERT_EQ(flag, 1); + + void* result; + ASSERT_EQ(0, pthread_join(t, &result)); +} + +class KilledByFault { + public: + explicit KilledByFault() {}; + bool operator()(int exit_status) const; +}; + +bool KilledByFault::operator()(int exit_status) const { + return WIFSIGNALED(exit_status) && + (WTERMSIG(exit_status) == SIGSEGV || + WTERMSIG(exit_status) == SIGBUS || + WTERMSIG(exit_status) == SIGABRT); +} + +TEST(properties_DeathTest, read_only) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + ASSERT_EXIT(__system_property_add("property", 8, "value", 5), + KilledByFault(), ""); +} +#endif