[PATCH 1/3] Bluetooth: Rename mgmt_inquiry_failed()

[PATCH 1/3] Bluetooth: Rename mgmt_inquiry_failed()

[Andre Guedes]

This patch renames the function mgmt_inquiry_failed() to
mgmt_start_discovery_failed(). This function is more related
to MGMT_OP_START_DISCOVERY command handling than to inquiry.
Besides, this functions will be reused by LE based discovery
procedures in case of failure.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_event.c        |    2 +-
 net/bluetooth/mgmt.c             |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0a5a05d..a1c7bea 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -938,7 +938,7 @@ int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
 int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type,
 					u8 *dev_class, s8 rssi, u8 *eir);
 int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 *name);
-int mgmt_inquiry_failed(struct hci_dev *hdev, u8 status);
+int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status);
 int mgmt_discovering(struct hci_dev *hdev, u8 discovering);
 int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr);
 int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a89cf1f..0c10780 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1014,7 +1014,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		hci_conn_check_pending(hdev);
 		hci_dev_lock(hdev);
 		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_inquiry_failed(hdev, status);
+			mgmt_start_discovery_failed(hdev, status);
 		hci_dev_unlock(hdev);
 		return;
 	}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a6720c6..ec89997 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2374,7 +2374,7 @@ int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 *name)
 	return mgmt_event(MGMT_EV_REMOTE_NAME, hdev, &ev, sizeof(ev), NULL);
 }
 
-int mgmt_inquiry_failed(struct hci_dev *hdev, u8 status)
+int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
 {
 	struct pending_cmd *cmd;
 	int err;
-- 
1.7.7.1

[PATCH 2/3] Bluetooth: mgmt_stop_discovery_failed()

[Andre Guedes]

This patches creates mgmt_stop_discovery_failed() which removes
pending MGMT_OP_STOP_DISCOVERY commands and sends proper command
status events.

This patch also fixes the MGMT_OP_STOP_DISCOVERY command leak in
case cancel inquiry fails.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_event.c        |    6 +++++-
 net/bluetooth/mgmt.c             |   15 +++++++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a1c7bea..9fedd9e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -939,6 +939,7 @@ int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type,
 					u8 *dev_class, s8 rssi, u8 *eir);
 int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 *name);
 int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status);
+int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status);
 int mgmt_discovering(struct hci_dev *hdev, u8 discovering);
 int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr);
 int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0c10780..d1277e2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -55,8 +55,12 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
-	if (status)
+	if (status) {
+		hci_dev_lock(hdev);
+		mgmt_stop_discovery_failed(hdev, status);
+		hci_dev_unlock(hdev);
 		return;
+	}
 
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ec89997..60306bc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2389,6 +2389,21 @@ int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
 	return err;
 }
 
+int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status)
+{
+	struct pending_cmd *cmd;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, hdev);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_status(cmd->sk, hdev->id, cmd->opcode, status);
+	mgmt_pending_remove(cmd);
+
+	return err;
+}
+
 int mgmt_discovering(struct hci_dev *hdev, u8 discovering)
 {
 	struct pending_cmd *cmd;
-- 
1.7.7.1

Re: [PATCH 2/3] Bluetooth: mgmt_stop_discovery_failed()

[Marcel Holtmann]

Hi Andre,

> This patches creates mgmt_stop_discovery_failed() which removes
> pending MGMT_OP_STOP_DISCOVERY commands and sends proper command
> status events.
> 
> This patch also fixes the MGMT_OP_STOP_DISCOVERY command leak in
> case cancel inquiry fails.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/hci_event.c        |    6 +++++-
>  net/bluetooth/mgmt.c             |   15 +++++++++++++++
>  3 files changed, 21 insertions(+), 1 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 2/3] Bluetooth: mgmt_stop_discovery_failed()

[Gustavo Padovan]

Hi Andre,

* Andre Guedes <andre.guedes@openbossa.org> [2011-11-09 17:14:26 -0300]:

> This patches creates mgmt_stop_discovery_failed() which removes
> pending MGMT_OP_STOP_DISCOVERY commands and sends proper command
> status events.
> 
> This patch also fixes the MGMT_OP_STOP_DISCOVERY command leak in
> case cancel inquiry fails.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/hci_event.c        |    6 +++++-
>  net/bluetooth/mgmt.c             |   15 +++++++++++++++
>  3 files changed, 21 insertions(+), 1 deletions(-)

Patches 1 and 2 have been applied, patch 3 doesn't apply anymore. Thanks.

	Gustavo

[PATCH 3/3] Bluetooth: Code refactoring

[Andre Guedes]

This patch adds the helper function cmd_failed() to handle mgmt
commands failures. This function sends the proper command status
event and removes the command from the pending list.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/mgmt.c |   53 +++++++++++++++++--------------------------------
 1 files changed, 19 insertions(+), 34 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 60306bc..f242be2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -290,6 +290,22 @@ static void mgmt_pending_remove(struct pending_cmd *cmd)
 	mgmt_pending_free(cmd);
 }
 
+static int cmd_failed(struct hci_dev *hdev, u16 cmd_opcode, u8 status)
+{
+	struct pending_cmd *cmd = mgmt_pending_find(cmd_opcode, hdev);
+	int err;
+
+	if (!cmd)
+		return -ENOENT;
+
+	BT_DBG("%s opcode %u status %u", hdev->name, cmd_opcode, status);
+
+	err = cmd_status(cmd->sk, hdev->id, cmd->opcode, status);
+	mgmt_pending_remove(cmd);
+
+	return err;
+}
+
 static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len)
 {
 	struct mgmt_mode *cp;
@@ -2135,18 +2151,7 @@ int mgmt_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
 
 int mgmt_disconnect_failed(struct hci_dev *hdev)
 {
-	struct pending_cmd *cmd;
-	int err;
-
-	cmd = mgmt_pending_find(MGMT_OP_DISCONNECT, hdev);
-	if (!cmd)
-		return -ENOENT;
-
-	err = cmd_status(cmd->sk, hdev->id, MGMT_OP_DISCONNECT, EIO);
-
-	mgmt_pending_remove(cmd);
-
-	return err;
+	return cmd_failed(hdev, MGMT_OP_DISCONNECT, EIO);
 }
 
 int mgmt_connect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type,
@@ -2376,32 +2381,12 @@ int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 *name)
 
 int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
 {
-	struct pending_cmd *cmd;
-	int err;
-
-	cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
-	if (!cmd)
-		return -ENOENT;
-
-	err = cmd_status(cmd->sk, hdev->id, cmd->opcode, status);
-	mgmt_pending_remove(cmd);
-
-	return err;
+	return cmd_failed(hdev, MGMT_OP_START_DISCOVERY, status);
 }
 
 int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status)
 {
-	struct pending_cmd *cmd;
-	int err;
-
-	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, hdev);
-	if (!cmd)
-		return -ENOENT;
-
-	err = cmd_status(cmd->sk, hdev->id, cmd->opcode, status);
-	mgmt_pending_remove(cmd);
-
-	return err;
+	return cmd_failed(hdev, MGMT_OP_STOP_DISCOVERY, status);
 }
 
 int mgmt_discovering(struct hci_dev *hdev, u8 discovering)
-- 
1.7.7.1

Re: [PATCH 3/3] Bluetooth: Code refactoring

[Marcel Holtmann]

Hi Andre,

> This patch adds the helper function cmd_failed() to handle mgmt
> commands failures. This function sends the proper command status
> event and removes the command from the pending list.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c |   53 +++++++++++++++++--------------------------------
>  1 files changed, 19 insertions(+), 34 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

<snip>
 
>  int mgmt_disconnect_failed(struct hci_dev *hdev)
>  {
> -	struct pending_cmd *cmd;
> -	int err;
> -
> -	cmd = mgmt_pending_find(MGMT_OP_DISCONNECT, hdev);
> -	if (!cmd)
> -		return -ENOENT;
> -
> -	err = cmd_status(cmd->sk, hdev->id, MGMT_OP_DISCONNECT, EIO);
> -
> -	mgmt_pending_remove(cmd);
> -
> -	return err;
> +	return cmd_failed(hdev, MGMT_OP_DISCONNECT, EIO);
>  }

So the only left-over question now is if we need to keep these empty
stub functions around or can just call cmd_failed directly.

It might be actually cleaner to just call cmd_failed directly.

Regards

Marcel

Re: [PATCH 3/3] Bluetooth: Code refactoring

[Andre Guedes]

Hi Marcel,

On Nov 9, 2011, at 7:18 PM, Marcel Holtmann wrote:

> Hi Andre,
> 
>> This patch adds the helper function cmd_failed() to handle mgmt
>> commands failures. This function sends the proper command status
>> event and removes the command from the pending list.
>> 
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> net/bluetooth/mgmt.c |   53 +++++++++++++++++--------------------------------
>> 1 files changed, 19 insertions(+), 34 deletions(-)
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> <snip>
> 
>> int mgmt_disconnect_failed(struct hci_dev *hdev)
>> {
>> -	struct pending_cmd *cmd;
>> -	int err;
>> -
>> -	cmd = mgmt_pending_find(MGMT_OP_DISCONNECT, hdev);
>> -	if (!cmd)
>> -		return -ENOENT;
>> -
>> -	err = cmd_status(cmd->sk, hdev->id, MGMT_OP_DISCONNECT, EIO);
>> -
>> -	mgmt_pending_remove(cmd);
>> -
>> -	return err;
>> +	return cmd_failed(hdev, MGMT_OP_DISCONNECT, EIO);
>> }
> 
> So the only left-over question now is if we need to keep these empty
> stub functions around or can just call cmd_failed directly.
> 
> It might be actually cleaner to just call cmd_failed directly.

I've asked myself the same question. If we wanna have hci_core layer
calling cmd_failed() we need to include mgmt.h header so we have access
to mgmt commands opcodes.

BR,

Andre

Re: [PATCH 3/3] Bluetooth: Code refactoring

[Marcel Holtmann]

Hi Andre,

> >> This patch adds the helper function cmd_failed() to handle mgmt
> >> commands failures. This function sends the proper command status
> >> event and removes the command from the pending list.
> >> 
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >> net/bluetooth/mgmt.c |   53 +++++++++++++++++--------------------------------
> >> 1 files changed, 19 insertions(+), 34 deletions(-)
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > 
> > <snip>
> > 
> >> int mgmt_disconnect_failed(struct hci_dev *hdev)
> >> {
> >> -	struct pending_cmd *cmd;
> >> -	int err;
> >> -
> >> -	cmd = mgmt_pending_find(MGMT_OP_DISCONNECT, hdev);
> >> -	if (!cmd)
> >> -		return -ENOENT;
> >> -
> >> -	err = cmd_status(cmd->sk, hdev->id, MGMT_OP_DISCONNECT, EIO);
> >> -
> >> -	mgmt_pending_remove(cmd);
> >> -
> >> -	return err;
> >> +	return cmd_failed(hdev, MGMT_OP_DISCONNECT, EIO);
> >> }
> > 
> > So the only left-over question now is if we need to keep these empty
> > stub functions around or can just call cmd_failed directly.
> > 
> > It might be actually cleaner to just call cmd_failed directly.
> 
> I've asked myself the same question. If we wanna have hci_core layer
> calling cmd_failed() we need to include mgmt.h header so we have access
> to mgmt commands opcodes.

fair enough. Leave it as it is for now.

Regards

Marcel

Re: [PATCH 3/3] Bluetooth: Code refactoring

[Gustavo Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2011-11-10 07:18:05 +0900]:

> Hi Andre,
> 
> > This patch adds the helper function cmd_failed() to handle mgmt
> > commands failures. This function sends the proper command status
> > event and removes the command from the pending list.
> > 
> > Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> > ---
> >  net/bluetooth/mgmt.c |   53 +++++++++++++++++--------------------------------
> >  1 files changed, 19 insertions(+), 34 deletions(-)
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> <snip>
>  
> >  int mgmt_disconnect_failed(struct hci_dev *hdev)
> >  {
> > -	struct pending_cmd *cmd;
> > -	int err;
> > -
> > -	cmd = mgmt_pending_find(MGMT_OP_DISCONNECT, hdev);
> > -	if (!cmd)
> > -		return -ENOENT;
> > -
> > -	err = cmd_status(cmd->sk, hdev->id, MGMT_OP_DISCONNECT, EIO);
> > -
> > -	mgmt_pending_remove(cmd);
> > -
> > -	return err;
> > +	return cmd_failed(hdev, MGMT_OP_DISCONNECT, EIO);
> >  }
> 
> So the only left-over question now is if we need to keep these empty
> stub functions around or can just call cmd_failed directly.
> 
> It might be actually cleaner to just call cmd_failed directly.

We can convert everything to macros and get rid of these stubs. IMHO this is a
cleaner solution.

	Gustavo

Re: [PATCH 1/3] Bluetooth: Rename mgmt_inquiry_failed()

[Marcel Holtmann]

Hi Andre,

> This patch renames the function mgmt_inquiry_failed() to
> mgmt_start_discovery_failed(). This function is more related
> to MGMT_OP_START_DISCOVERY command handling than to inquiry.
> Besides, this functions will be reused by LE based discovery
> procedures in case of failure.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 +-
>  net/bluetooth/hci_event.c        |    2 +-
>  net/bluetooth/mgmt.c             |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

And please just carry my ACK around when re-sending patches if I already
acked it. No need to keep doing this over and over again.

Regards

Marcel