Bluetooth: GATT: Fix CCC handling

The 'valid' member of struct bt_gatt_ccc_cfg was redundant, since
setting 'peer' to BT_ADDR_LE_ANY does the same job. What's worse, the
handling of 'valid' was also buggy in that some places looking for
valid CCC structs only matched the address, meaning it might yield a
positive match for invalid entries.

Fix these issues by removing the 'valid' struct member, and solely
using the 'peer' member to identify valid entries. Also simplify the
code by acknowledging that no CCC entry is essentially the same as the
value '0' written to CCC.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
This commit is contained in:
Johan Hedberg 2018-04-30 20:27:36 +01:00 committed by Johan Hedberg
commit 6a71a69f8d
2 changed files with 18 additions and 14 deletions

View file

@ -488,13 +488,11 @@ ssize_t bt_gatt_attr_read_chrc(struct bt_conn *conn,
#define BT_GATT_CCC_MAX (CONFIG_BT_MAX_PAIRED + CONFIG_BT_MAX_CONN) #define BT_GATT_CCC_MAX (CONFIG_BT_MAX_PAIRED + CONFIG_BT_MAX_CONN)
/** @brief GATT CCC configuration entry. /** @brief GATT CCC configuration entry.
* @param valid Valid flag
* @param peer Remote peer address * @param peer Remote peer address
* @param value Configuration value. * @param value Configuration value.
* @param data Configuration pointer data. * @param data Configuration pointer data.
*/ */
struct bt_gatt_ccc_cfg { struct bt_gatt_ccc_cfg {
u8_t valid;
bt_addr_le_t peer; bt_addr_le_t peer;
u16_t value; u16_t value;
u8_t data[4] __aligned(4); u8_t data[4] __aligned(4);

View file

@ -533,18 +533,21 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn,
} }
if (i == ccc->cfg_len) { if (i == ccc->cfg_len) {
/* If there's no existing entry, but the new value is zero,
* we don't need to do anything, since a disabled CCC is
* behavioraly the same as no written CCC.
*/
if (!value) {
return len;
}
for (i = 0; i < ccc->cfg_len; i++) { for (i = 0; i < ccc->cfg_len; i++) {
/* Check for unused configuration */ /* Check for unused configuration */
if (ccc->cfg[i].valid) { if (bt_addr_le_cmp(&ccc->cfg[i].peer, BT_ADDR_LE_ANY)) {
continue; continue;
} }
bt_addr_le_copy(&ccc->cfg[i].peer, &conn->le.dst); bt_addr_le_copy(&ccc->cfg[i].peer, &conn->le.dst);
if (value) {
ccc->cfg[i].valid = true;
}
break; break;
} }
@ -552,9 +555,6 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn,
BT_WARN("No space to store CCC cfg"); BT_WARN("No space to store CCC cfg");
return BT_GATT_ERR(BT_ATT_ERR_INSUFFICIENT_RESOURCES); return BT_GATT_ERR(BT_ATT_ERR_INSUFFICIENT_RESOURCES);
} }
} else if (!value) {
/* free existing configuration for default value */
ccc->cfg[i].valid = false;
} }
ccc->cfg[i].value = value; ccc->cfg[i].value = value;
@ -566,6 +566,12 @@ ssize_t bt_gatt_attr_write_ccc(struct bt_conn *conn,
gatt_ccc_changed(attr, ccc); gatt_ccc_changed(attr, ccc);
} }
/* Disabled CCC is the same as no configured CCC, so clear the entry */
if (!value) {
bt_addr_le_copy(&ccc->cfg[i].peer, BT_ADDR_LE_ANY);
ccc->cfg[i].value = 0;
}
return len; return len;
} }
@ -926,9 +932,9 @@ static u8_t disconnected_cb(const struct bt_gatt_attr *attr, void *user_data)
} else { } else {
/* Clear value if not paired */ /* Clear value if not paired */
if (!bt_addr_le_is_bonded(&conn->le.dst)) { if (!bt_addr_le_is_bonded(&conn->le.dst)) {
ccc->cfg[i].valid = false; bt_addr_le_copy(&ccc->cfg[i].peer,
memset(&ccc->cfg[i].value, 0, BT_ADDR_LE_ANY);
sizeof(ccc->cfg[i].value)); ccc->cfg[i].value = 0;
} else { } else {
/* Update address in case it has changed */ /* Update address in case it has changed */
bt_addr_le_copy(&ccc->cfg[i].peer, bt_addr_le_copy(&ccc->cfg[i].peer,