[PATCH] Bluetooth: Remove needless HCI_MGMT flag check

Subsystems: bluetooth subsystem, the rest

STALE5367d

4 messages, 3 authors, 2012-02-14 · open the first message on its own page

[PATCH] Bluetooth: Remove needless HCI_MGMT flag check

From: Andre Guedes <hidden>
Date: 2012-02-10 13:49:38

This patch removes HCI_MGMT flag check in hci_cs_inquiry. There is
no need to check this flag before calling mgmt_start_discovery_failed
since if the hdev is not managed by mgmt interface this function does
nothing.

Signed-off-by: Andre Guedes <redacted>
---
 net/bluetooth/hci_event.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b0784ee..5f93fdd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1143,8 +1143,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 		hci_conn_check_pending(hdev);
 		hci_dev_lock(hdev);
-		if (test_bit(HCI_MGMT, &hdev->dev_flags))
-			mgmt_start_discovery_failed(hdev, status);
+		mgmt_start_discovery_failed(hdev, status);
 		hci_dev_unlock(hdev);
 		return;
 	}
-- 
1.7.9

Re: [PATCH] Bluetooth: Remove needless HCI_MGMT flag check

From: Gustavo Padovan <hidden>
Date: 2012-02-10 16:21:05

Hi Andre,

* Andre Guedes [off-list ref] [2012-02-10 10:49:38 -0300]:
quoted hunk
This patch removes HCI_MGMT flag check in hci_cs_inquiry. There is
no need to check this flag before calling mgmt_start_discovery_failed
since if the hdev is not managed by mgmt interface this function does
nothing.

Signed-off-by: Andre Guedes <redacted>
---
 net/bluetooth/hci_event.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b0784ee..5f93fdd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1143,8 +1143,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 		hci_conn_check_pending(hdev);
 		hci_dev_lock(hdev);
-		if (test_bit(HCI_MGMT, &hdev->dev_flags))
-			mgmt_start_discovery_failed(hdev, status);
+		mgmt_start_discovery_failed(hdev, status);
 		hci_dev_unlock(hdev);
 		return;
 	}
Acked-by: Gustavo F. Padovan <redacted>

	Gustavo

Re: [PATCH] Bluetooth: Remove needless HCI_MGMT flag check

From: Johan Hedberg <hidden>
Date: 2012-02-14 12:24:42

Hi Andre,

On Fri, Feb 10, 2012, Andre Guedes wrote:
quoted hunk
This patch removes HCI_MGMT flag check in hci_cs_inquiry. There is
no need to check this flag before calling mgmt_start_discovery_failed
since if the hdev is not managed by mgmt interface this function does
nothing.

Signed-off-by: Andre Guedes <redacted>
---
 net/bluetooth/hci_event.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b0784ee..5f93fdd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1143,8 +1143,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 		hci_conn_check_pending(hdev);
 		hci_dev_lock(hdev);
-		if (test_bit(HCI_MGMT, &hdev->dev_flags))
-			mgmt_start_discovery_failed(hdev, status);
+		mgmt_start_discovery_failed(hdev, status);
 		hci_dev_unlock(hdev);
 		return;
 	}
There are lots of checks like this in hci_event.c. If there's going to
be a patch for them I'd remove all of them in one go instead of
addressing individual ones like this.

There is some benefit of keeping them though: the code doesn't need to
check for pending mgmt commands, encode event messages and iterate
through all HCI sockets just to figure out that that that nothing needs
to be done.

Johan

Re: [PATCH] Bluetooth: Remove needless HCI_MGMT flag check

From: Andre Guedes <hidden>
Date: 2012-02-14 14:09:59

Hi Johan,

On Tue, Feb 14, 2012 at 9:24 AM, Johan Hedberg [off-list ref] wrote:
Hi Andre,

On Fri, Feb 10, 2012, Andre Guedes wrote:
quoted
This patch removes HCI_MGMT flag check in hci_cs_inquiry. There is
no need to check this flag before calling mgmt_start_discovery_failed
since if the hdev is not managed by mgmt interface this function does
nothing.

Signed-off-by: Andre Guedes <redacted>
---
 net/bluetooth/hci_event.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b0784ee..5f93fdd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1143,8 +1143,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
              hci_req_complete(hdev, HCI_OP_INQUIRY, status);
              hci_conn_check_pending(hdev);
              hci_dev_lock(hdev);
-             if (test_bit(HCI_MGMT, &hdev->dev_flags))
-                     mgmt_start_discovery_failed(hdev, status);
+             mgmt_start_discovery_failed(hdev, status);
              hci_dev_unlock(hdev);
              return;
      }
There are lots of checks like this in hci_event.c. If there's going to
be a patch for them I'd remove all of them in one go instead of
addressing individual ones like this.

There is some benefit of keeping them though: the code doesn't need to
check for pending mgmt commands, encode event messages and iterate
through all HCI sockets just to figure out that that that nothing needs
to be done.
Year, I see the benefit. However, in the long run this benefit will
be very small since mgmt interface will be used by default.

Anyway, if we wanna keep this check, perhaps it would be a good idea
we move it inside mgmt functions.

So, what you guys think, should we simply remove this check?

BR,

Andre
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help