Re: [PATCH 1/2] Bluetooth: Remove unneeded check in hci_disconn_complete_evt()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andre,

On Mon, Nov 04, 2013, Andre Guedes wrote:
> According to b644ba336 (patch that introduced HCI_CONN_MGMT_CONNECTED
> flag), the HCI_CONN_MGMT_CONNECTED flag tracks when mgmt has been
> notified about the connection.
> 
> That being said, there is no point in checking this flag in hci_
> disconn_complete_evt() since neither mgmt_disconnect_failed() nor
> mgmt_device_disconnected() depend on it. Below follows more details:
>   * mgmt_disconnect_failed() removes pending MGMT_OP_DISCONNECT
>     commands, it doesn't matter if that connection was notified or not.
>   * mgmt_device_disconnected() sends the mgmt event only if the link
>     type is ACL_LINK or LE_LINK. For those connection type, the flag is
>     always set.
> 
> So this patch removes the HCI_CONN_MGMT_CONNECTED check.
> 
> Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
> ---
>  net/bluetooth/hci_event.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 142aa61..560296d 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1792,16 +1792,14 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  	if (ev->status == 0)
>  		conn->state = BT_CLOSED;
>  
> -	if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) {
> -		if (ev->status) {
> -			mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
> -					       conn->dst_type, ev->status);
> -		} else {
> -			u8 reason = hci_to_mgmt_reason(ev->reason);
> +	if (ev->status) {
> +		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);
> -		}
> +		mgmt_device_disconnected(hdev, &conn->dst, conn->type,
> +					 conn->dst_type, reason);
>  	}
>  
>  	if (ev->status == 0) {

It looks to me like this would cause an invalid mgmt_device_disconnected
event to be sent of the ACL goes down before we notify over mgmt that
it's connected. This could e.g. happen if the ACL disconnection happens
before we complete the implicit name resolving (which is the main reason
why this flag exists to begin with). Am I missing something here?

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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux