Re: [PATCH] Bluetooth: Send Discovery Stopped event when discovery fails

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Hemant,

On Thu, Apr 05, 2012, Hemant Gupta wrote:
> This patch sends MGMT_EV_DISCOVERING event to manamgement interface of
> BlueZ to indicate that discovery is stopped in case of discovery failure.
> Without this patch discovery session of BlueZ was not getting freed.
> This event was not sent from kernel in case discovery state is still
> DISCOVERY_STARTING.
> 
> Signed-off-by: Hemant Gupta <hemant.gupta@stericsson.com>
> ---
>  net/bluetooth/hci_core.c |    2 +-
>  net/bluetooth/mgmt.c     |   16 +---------------
>  2 files changed, 2 insertions(+), 16 deletions(-)

This patch in general doesn't make much sense to me.

> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9629645..b97a7dc 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -384,7 +384,7 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
>  
>  	switch (state) {
>  	case DISCOVERY_STOPPED:
> -		if (hdev->discovery.state != DISCOVERY_STARTING)
> +		if (hdev->discovery.state != DISCOVERY_STOPPED)
>  			mgmt_discovering(hdev, 0);
>  		break;

This is wrong in several different ways. Firstly it's wrong since we
only do mgmt_discovering(hdev, 1) when going to DISCOVERY_FINDING state
so mgmt_discovering(hdev, 0) should not be called before that. Secondly
it's wrong because the function will return if hdev->discovery.state ==
state, i.e. your if statement would always evaluate to true and
therefore be redundant.

> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3572,23 +3572,9 @@ int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>  
>  int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
>  {
> -	struct pending_cmd *cmd;
> -	u8 type;
> -	int err;
> -
>  	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
>  
> -	cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
> -	if (!cmd)
> -		return -ENOENT;
> -
> -	type = hdev->discovery.type;
> -
> -	err = cmd_complete(cmd->sk, hdev->id, cmd->opcode, mgmt_status(status),
> -			   &type, sizeof(type));
> -	mgmt_pending_remove(cmd);
> -
> -	return err;
> +	return 0;
>  }

So who sends the appropriate command complete event to start_discovery
now? I don't see any other place that would do it.

Johan