Hi Marcel,
On 10/29/2013 07:52 PM, Marcel Holtmann wrote:
Hi Andre,
hci_disconn_complete_evt() logic is more complicated than what it
should be, making it hard to follow and add new features.
So this patch does some code refactoring by handling the error cases
in the beginning of the function and by moving the main flow into the
first level of function scope. No change is done in the event handling
logic itself.
Besides organizing this messy code, this patch makes easier to add
code for handling LE auto connection (which will be added in a further
patch).
Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
---
net/bluetooth/hci_event.c | 62 +++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5935f74..8b7cd37 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1783,6 +1783,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_disconn_complete *ev = (void *) skb->data;
struct hci_conn *conn;
+ u8 type;
+ bool send_mgmt_event = false;
BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
@@ -1792,44 +1794,46 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (!conn)
goto unlock;
- if (ev->status == 0)
- conn->state = BT_CLOSED;
-
if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
- (conn->type == ACL_LINK || conn->type == LE_LINK)) {
- if (ev->status) {
+ (conn->type == ACL_LINK || conn->type == LE_LINK))
+ send_mgmt_event = true;
+
+ if (ev->status) {
+ if (send_mgmt_event)
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
- } else {
- u8 reason = hci_to_mgmt_reason(ev->reason);
-
- mgmt_device_disconnected(hdev, &conn->dst, conn->type,
- conn->dst_type, reason);
- }
+ return;
}
so I get the feeling that mgmt_disconnect_failed should check the link type all by itself. Since we are handing it over to the function anyway. And then this send_mgmt_event could be just removed.
- if (ev->status == 0) {
- u8 type = conn->type;
+ conn->state = BT_CLOSED;
- if (type == ACL_LINK && conn->flush_key)
- hci_remove_link_key(hdev, &conn->dst);
- hci_proto_disconn_cfm(conn, ev->reason);
- hci_conn_del(conn);
+ if (send_mgmt_event) {
+ u8 reason = hci_to_mgmt_reason(ev->reason);
- /* Re-enable advertising if necessary, since it might
- * have been disabled by the connection. From the
- * HCI_LE_Set_Advertise_Enable command description in
- * the core specification (v4.0):
- * "The Controller shall continue advertising until the Host
- * issues an LE_Set_Advertise_Enable command with
- * Advertising_Enable set to 0x00 (Advertising is disabled)
- * or until a connection is created or until the Advertising
- * is timed out due to Directed Advertising."
- */
- if (type == LE_LINK)
- mgmt_reenable_advertising(hdev);
+ mgmt_device_disconnected(hdev, &conn->dst, conn->type,
+ conn->dst_type, reason);
}
Same here. Just make sure that mgmt_device_disconneted checks the link type by itself and call it unconditional.
+ if (conn->type == ACL_LINK && conn->flush_key)
+ hci_remove_link_key(hdev, &conn->dst);
+
+ type = conn->type;
+ hci_proto_disconn_cfm(conn, ev->reason);
+ hci_conn_del(conn);
+
+ /* Re-enable advertising if necessary, since it might
+ * have been disabled by the connection. From the
+ * HCI_LE_Set_Advertise_Enable command description in
+ * the core specification (v4.0):
+ * "The Controller shall continue advertising until the Host
+ * issues an LE_Set_Advertise_Enable command with
+ * Advertising_Enable set to 0x00 (Advertising is disabled)
+ * or until a connection is created or until the Advertising
+ * is timed out due to Directed Advertising."
+ */
+ if (type == LE_LINK)
+ mgmt_reenable_advertising(hdev);
+
unlock:
hci_dev_unlock(hdev);
}
These comments were implemented in the patch set "[RFC 0/5] Disconnect
complete refactoring". That patch set is already pushed upstream.
I'm currently working on a new version of this patch set. All your other
comments will be addressed in v3.
Thanks,
Andre
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
