| Message ID | 20230316193634.1195948-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [BlueZ] shared/gatt-client: Fix not creating a request for notifications | expand |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=730956
---Test result---
Test Summary:
CheckPatch PASS 0.39 seconds
GitLint PASS 0.24 seconds
BuildEll PASS 34.11 seconds
BluezMake PASS 1172.60 seconds
MakeCheck PASS 13.10 seconds
MakeDistcheck PASS 192.25 seconds
CheckValgrind PASS 314.31 seconds
CheckSmatch PASS 428.40 seconds
bluezmakeextell PASS 127.66 seconds
IncrementalBuild PASS 967.69 seconds
ScanBuild WARNING 1390.19 seconds
Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:993:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1099:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1291:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1356:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1631:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2140:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2148:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3236:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3258:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.
---
Regards,
Linux Bluetooth
Hello: This patch was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Thu, 16 Mar 2023 12:36:34 -0700 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > Notifications were using bt_att_send directly instead of > bt_gatt_client_write_value thus it wouldn't create a request which > causes the instance to not be able to track it which in turn may cause > the client to trigger its idle callback too early. > > [...] Here is the summary with links: - [BlueZ] shared/gatt-client: Fix not creating a request for notifications https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=306a8083ad19 You are awesome, thank you!
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c index 9ffc10b0b44a..efc013a20dcf 100644 --- a/src/shared/gatt-client.c +++ b/src/shared/gatt-client.c @@ -1660,31 +1660,30 @@ static void complete_notify_request(void *data) } static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable, - bt_att_response_func_t callback) + bt_gatt_client_callback_t callback) { - uint8_t pdu[4]; unsigned int att_id; + uint16_t value; uint16_t properties = notify_data->chrc->properties; assert(notify_data->chrc->ccc_handle); - memset(pdu, 0, sizeof(pdu)); - put_le16(notify_data->chrc->ccc_handle, pdu); if (enable) { /* Try to enable notifications or indications based on * whatever the characteristic supports. */ if (properties & BT_GATT_CHRC_PROP_NOTIFY) - pdu[2] = 0x01; + value = cpu_to_le16(0x0001); else if (properties & BT_GATT_CHRC_PROP_INDICATE) - pdu[2] = 0x02; - - if (!pdu[2]) + value = cpu_to_le16(0x0002); + else return false; } - att_id = bt_att_send(notify_data->client->att, BT_ATT_OP_WRITE_REQ, - pdu, sizeof(pdu), callback, + att_id = bt_gatt_client_write_value(notify_data->client, + notify_data->chrc->ccc_handle, + (void *)&value, sizeof(value), + callback, notify_data_ref(notify_data), notify_data_unref); notify_data->chrc->ccc_write_id = notify_data->att_id = att_id; @@ -1714,8 +1713,8 @@ static bool notify_set_ecode(const void *data, const void *match_data) return true; } -static void enable_ccc_callback(uint8_t opcode, const void *pdu, - uint16_t length, void *user_data) +static void enable_ccc_callback(bool success, uint8_t att_ecode, + void *user_data) { struct notify_data *notify_data = user_data; @@ -1723,10 +1722,9 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu, notify_data->chrc->ccc_write_id = 0; - bt_gatt_client_ref(notify_data->client); + bt_gatt_client_ref_safe(notify_data->client); - if (opcode == BT_ATT_OP_ERROR_RSP) - notify_data->att_ecode = process_error(pdu, length); + notify_data->att_ecode = att_ecode; /* Notify for all remaining requests. */ complete_notify_request(notify_data); @@ -2165,8 +2163,8 @@ struct value_data { const void *data; }; -static void disable_ccc_callback(uint8_t opcode, const void *pdu, - uint16_t length, void *user_data) +static void disable_ccc_callback(bool success, uint8_t att_ecode, + void *user_data) { struct notify_data *notify_data = user_data; struct notify_data *next_data;
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Notifications were using bt_att_send directly instead of bt_gatt_client_write_value thus it wouldn't create a request which causes the instance to not be able to track it which in turn may cause the client to trigger its idle callback too early. Fixes: https://github.com/bluez/bluez/issues/490 --- src/shared/gatt-client.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)