btif: Don't persist remote devices to the config
We don't need to persist the unpaired devices to NVRAM
so skip saving them.
This fixes a regression in a previous patch where the most recent
instead of the least recent devices would be removed, making some
devices unpairable in extremely busy environments.
This is a backport of http://r.android.com/210955 and
http://r.android.com/212838 together.
Bug: 26071376
Change-Id: If7ee9d960f70c836bf08b78da5f3fc852ba60a85
diff --git a/btif/include/btif_storage.h b/btif/include/btif_storage.h
index 54dc5d6..7763d3a3 100644
--- a/btif/include/btif_storage.h
+++ b/btif/include/btif_storage.h
@@ -93,11 +93,11 @@
**
** Function btif_storage_add_remote_device
**
-** Description BTIF storage API - Adds a newly discovered device to NVRAM
-** along with the timestamp. Also, stores the various
+** Description BTIF storage API - Adds a newly discovered device to
+** track along with the timestamp. Also, stores the various
** properties - RSSI, BDADDR, NAME (if found in EIR)
**
-** Returns BT_STATUS_SUCCESS if the store was successful,
+** Returns BT_STATUS_SUCCESS if successful,
** BT_STATUS_FAIL otherwise
**
*******************************************************************************/
diff --git a/btif/src/btif_config.c b/btif/src/btif_config.c
index dfd308e..ad5b607 100644
--- a/btif/src/btif_config.c
+++ b/btif/src/btif_config.c
@@ -44,7 +44,7 @@
static void timer_config_save_cb(void *data);
static void btif_config_write(void);
-static void btif_config_devcache_cleanup(void);
+static void btif_config_remove_unpaired(config_t *config);
// TODO(zachoverflow): Move these two functions out, because they are too specific for this file
// {grumpy-cat/no, monty-python/you-make-me-sad}
@@ -109,7 +109,7 @@
unlink(LEGACY_CONFIG_FILE_PATH);
}
- btif_config_devcache_cleanup();
+ btif_config_remove_unpaired(config);
// TODO(sharvil): use a non-wake alarm for this once we have
// API support for it. There's no need to wake the system to
@@ -358,7 +358,6 @@
assert(alarm_timer != NULL);
alarm_cancel(alarm_timer);
-
btif_config_write();
}
@@ -390,43 +389,35 @@
assert(config != NULL);
assert(alarm_timer != NULL);
- btif_config_devcache_cleanup();
-
pthread_mutex_lock(&lock);
- config_save(config, CONFIG_FILE_PATH);
+ config_t *config_paired = config_new_clone(config);
+ btif_config_remove_unpaired(config_paired);
+ config_save(config_paired, CONFIG_FILE_PATH);
+ config_free(config_paired);
pthread_mutex_unlock(&lock);
}
-static void btif_config_devcache_cleanup(void) {
- assert(config != NULL);
+static void btif_config_remove_unpaired(config_t *conf) {
+ assert(conf != NULL);
- // The config accumulates cached information about remote
- // devices during regular inquiry scans. We remove some of these
- // so the cache doesn't grow indefinitely.
- // We don't remove information about bonded devices (which have link keys).
- static const size_t ADDRS_MAX = 512;
- size_t total_addrs = 0;
-
- pthread_mutex_lock(&lock);
- const config_section_node_t *snode = config_section_begin(config);
- while (snode != config_section_end(config)) {
+ // The paired config used to carry information about
+ // discovered devices during regular inquiry scans.
+ // We remove these now and cache them in memory instead.
+ const config_section_node_t *snode = config_section_begin(conf);
+ while (snode != config_section_end(conf)) {
const char *section = config_section_name(snode);
if (string_is_bdaddr(section)) {
- ++total_addrs;
-
- if ((total_addrs > ADDRS_MAX) &&
- !config_has_key(config, section, "LinkKey") &&
- !config_has_key(config, section, "LE_KEY_PENC") &&
- !config_has_key(config, section, "LE_KEY_PID") &&
- !config_has_key(config, section, "LE_KEY_PCSRK") &&
- !config_has_key(config, section, "LE_KEY_LENC") &&
- !config_has_key(config, section, "LE_KEY_LCSRK")) {
+ if (!config_has_key(conf, section, "LinkKey") &&
+ !config_has_key(conf, section, "LE_KEY_PENC") &&
+ !config_has_key(conf, section, "LE_KEY_PID") &&
+ !config_has_key(conf, section, "LE_KEY_PCSRK") &&
+ !config_has_key(conf, section, "LE_KEY_LENC") &&
+ !config_has_key(conf, section, "LE_KEY_LCSRK")) {
snode = config_section_next(snode);
- config_remove_section(config, section);
+ config_remove_section(conf, section);
continue;
}
}
snode = config_section_next(snode);
}
- pthread_mutex_unlock(&lock);
}
diff --git a/osi/include/config.h b/osi/include/config.h
index ab3096d..d7c8ff9 100644
--- a/osi/include/config.h
+++ b/osi/include/config.h
@@ -37,6 +37,15 @@
// file on the filesystem.
config_t *config_new(const char *filename);
+// Clones |src|, including all of it's sections, keys, and values.
+// Returns a new config which is a copy and separated from the original;
+// changes to the new config are not reflected in any way in the original.
+//
+// |src| must not be NULL
+// This function will not return NULL.
+// Clients must call config_free on the returned object.
+config_t *config_new_clone(const config_t *src);
+
// Frees resources associated with the config file. No further operations may
// be performed on the |config| object after calling this function. |config|
// may be NULL.
diff --git a/osi/src/config.c b/osi/src/config.c
index 11a5baf..c87f531 100644
--- a/osi/src/config.c
+++ b/osi/src/config.c
@@ -96,6 +96,30 @@
return config;
}
+config_t *config_new_clone(const config_t *src) {
+ assert(src != NULL);
+
+ config_t *ret = config_new_empty();
+
+ assert(ret != NULL);
+
+ for (const list_node_t *node = list_begin(src->sections);
+ node != list_end(src->sections);
+ node = list_next(node)) {
+ section_t *sec = list_node(node);
+
+ for (const list_node_t *node_entry = list_begin(sec->entries);
+ node_entry != list_end(sec->entries);
+ node_entry = list_next(node_entry)) {
+ entry_t *entry = list_node(node_entry);
+
+ config_set_string(ret, sec->name, entry->key, entry->value);
+ }
+ }
+
+ return ret;
+}
+
void config_free(config_t *config) {
if (!config)
return;
diff --git a/osi/test/config_test.cpp b/osi/test/config_test.cpp
index 70e5a24..0419ddc 100644
--- a/osi/test/config_test.cpp
+++ b/osi/test/config_test.cpp
@@ -80,6 +80,19 @@
config_free(NULL);
}
+TEST_F(ConfigTest, config_new_clone) {
+ config_t *config = config_new(CONFIG_FILE);
+ config_t *clone = config_new_clone(config);
+
+ config_set_string(clone, CONFIG_DEFAULT_SECTION, "first_key", "not_value");
+
+ EXPECT_STRNE(config_get_string(config, CONFIG_DEFAULT_SECTION, "first_key", "one"),
+ config_get_string(clone, CONFIG_DEFAULT_SECTION, "first_key", "one"));
+
+ config_free(config);
+ config_free(clone);
+}
+
TEST_F(ConfigTest, config_has_section) {
config_t *config = config_new(CONFIG_FILE);
EXPECT_TRUE(config_has_section(config, "DID"));
