Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a64dfa5d authored by Marie Janssen's avatar Marie Janssen
Browse files

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
parent 8ba94f68

system/btif/include/btif_storage.h

+3 −3
Original line number Original line Diff line number Diff line
@@ -93,11 +93,11 @@ bt_status_t btif_storage_set_remote_device_property(bt_bdaddr_t *remote_bd_addr, 
**

**

** Function         btif_storage_add_remote_device

** Function         btif_storage_add_remote_device

**

**

** Description      BTIF storage API - Adds a newly discovered device to NVRAM

** Description      BTIF storage API - Adds a newly discovered device to

**                  along with the timestamp. Also, stores the various

**                  track along with the timestamp. Also, stores the various

**                  properties - RSSI, BDADDR, NAME (if found in EIR)

**                  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

**                  BT_STATUS_FAIL otherwise

**

**

*******************************************************************************/

*******************************************************************************/

system/btif/src/btif_config.c

+20 −29
Original line number Original line Diff line number Diff line
@@ -44,7 +44,7 @@ static const period_ms_t CONFIG_SETTLE_PERIOD_MS = 3000; 




static void timer_config_save_cb(void *data);

static void timer_config_save_cb(void *data);

static void btif_config_write(void);

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

// TODO(zachoverflow): Move these two functions out, because they are too specific for this file

// {grumpy-cat/no, monty-python/you-make-me-sad}

// {grumpy-cat/no, monty-python/you-make-me-sad}
@@ -109,7 +109,7 @@ static future_t *init(void) { 
      unlink(LEGACY_CONFIG_FILE_PATH);

      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

  // 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

  // API support for it. There's no need to wake the system to
@@ -358,7 +358,6 @@ void btif_config_flush(void) { 
  assert(alarm_timer != NULL);

  assert(alarm_timer != NULL);





  alarm_cancel(alarm_timer);

  alarm_cancel(alarm_timer);



  btif_config_write();

  btif_config_write();

}

}
@@ -390,43 +389,35 @@ static void btif_config_write(void) { 
  assert(config != NULL);

  assert(config != NULL);

  assert(alarm_timer != NULL);

  assert(alarm_timer != NULL);





  btif_config_devcache_cleanup();



  pthread_mutex_lock(&lock);

  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);

  pthread_mutex_unlock(&lock);

}

}





static void btif_config_devcache_cleanup(void) {

static void btif_config_remove_unpaired(config_t *conf) {

  assert(config != NULL);

  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);

  // The paired config used to carry information about

  const config_section_node_t *snode = config_section_begin(config);

  // discovered devices during regular inquiry scans.

  while (snode != config_section_end(config)) {

  // 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);

    const char *section = config_section_name(snode);

    if (string_is_bdaddr(section)) {

    if (string_is_bdaddr(section)) {

      ++total_addrs;

      if (!config_has_key(conf, section, "LinkKey") &&



          !config_has_key(conf, section, "LE_KEY_PENC") &&

      if ((total_addrs > ADDRS_MAX) &&

          !config_has_key(conf, section, "LE_KEY_PID") &&

          !config_has_key(config, section, "LinkKey") &&

          !config_has_key(conf, section, "LE_KEY_PCSRK") &&

          !config_has_key(config, section, "LE_KEY_PENC") &&

          !config_has_key(conf, section, "LE_KEY_LENC") &&

          !config_has_key(config, section, "LE_KEY_PID") &&

          !config_has_key(conf, section, "LE_KEY_LCSRK")) {

          !config_has_key(config, section, "LE_KEY_PCSRK") &&

          !config_has_key(config, section, "LE_KEY_LENC") &&

          !config_has_key(config, section, "LE_KEY_LCSRK")) {

        snode = config_section_next(snode);

        snode = config_section_next(snode);

        config_remove_section(config, section);

        config_remove_section(conf, section);

        continue;

        continue;

      }

      }

    }

    }

    snode = config_section_next(snode);

    snode = config_section_next(snode);

  }

  }

  pthread_mutex_unlock(&lock);

}
 
}

system/osi/include/config.h

+9 −0
Original line number Original line Diff line number Diff line
@@ -37,6 +37,15 @@ config_t *config_new_empty(void); 
// file on the filesystem.

// file on the filesystem.

config_t *config_new(const char *filename);

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

// Frees resources associated with the config file. No further operations may

// be performed on the |config| object after calling this function. |config|

// be performed on the |config| object after calling this function. |config|

// may be NULL.

// may be NULL.

system/osi/src/config.c

+24 −0
Original line number Original line Diff line number Diff line
@@ -96,6 +96,30 @@ config_t *config_new(const char *filename) { 
  return config;

  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) {

void config_free(config_t *config) {

  if (!config)

  if (!config)

    return;

    return;

system/osi/test/config_test.cpp

+13 −0
Original line number Original line Diff line number Diff line
@@ -80,6 +80,19 @@ TEST_F(ConfigTest, config_free_null) { 
  config_free(NULL);

  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) {

TEST_F(ConfigTest, config_has_section) {

  config_t *config = config_new(CONFIG_FILE);

  config_t *config = config_new(CONFIG_FILE);

  EXPECT_TRUE(config_has_section(config, "DID"));

  EXPECT_TRUE(config_has_section(config, "DID"));