2011-11-09 20:14:25

by Andre Guedes

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

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 <[email protected]>
---
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



2011-11-16 17:41:36

by Gustavo Padovan

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

Hi Andre,

* Andre Guedes <[email protected]> [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 <[email protected]>
> ---
> 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

2011-11-09 23:16:42

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 3/3] Bluetooth: Code refactoring

Hi Marcel,

* Marcel Holtmann <[email protected]> [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 <[email protected]>
> > ---
> > net/bluetooth/mgmt.c | 53 +++++++++++++++++--------------------------------
> > 1 files changed, 19 insertions(+), 34 deletions(-)
>
> Acked-by: Marcel Holtmann <[email protected]>
>
> <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

2011-11-09 22:41:06

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH 3/3] Bluetooth: Code refactoring

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 <[email protected]>
>> ---
>> net/bluetooth/mgmt.c | 53 +++++++++++++++++--------------------------------
>> 1 files changed, 19 insertions(+), 34 deletions(-)
>
> Acked-by: Marcel Holtmann <[email protected]>
>
> <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


2011-11-09 22:44:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/3] Bluetooth: Code refactoring

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 <[email protected]>
> >> ---
> >> net/bluetooth/mgmt.c | 53 +++++++++++++++++--------------------------------
> >> 1 files changed, 19 insertions(+), 34 deletions(-)
> >
> > Acked-by: Marcel Holtmann <[email protected]>
> >
> > <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



2011-11-09 22:18:05

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/3] Bluetooth: Code refactoring

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 <[email protected]>
> ---
> net/bluetooth/mgmt.c | 53 +++++++++++++++++--------------------------------
> 1 files changed, 19 insertions(+), 34 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

<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



2011-11-09 22:16:18

by Marcel Holtmann

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

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards

Marcel



2011-11-09 22:15:01

by Marcel Holtmann

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

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 <[email protected]>
> ---
> 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 <[email protected]>

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



2011-11-09 20:14:27

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 3/3] Bluetooth: Code refactoring

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 <[email protected]>
---
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


2011-11-09 20:14:26

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 2/3] Bluetooth: mgmt_stop_discovery_failed()

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 <[email protected]>
---
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