2012-07-18 12:06:04

by Malovany, Ram

Subject: [PATCH 1/3] Bluetooth: Do not resolve name if entry was not found.

From: Ram Malovany <[email protected]>

If entry wasn't found in the hci_inquiry_cache_lookup_resolve do not
resolve the name.

Signed-off-by: Ram Malovany <[email protected]>
---
net/bluetooth/hci_event.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 41ff978..2de521f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1365,6 +1365,9 @@ static bool hci_resolve_next_name(struct hci_dev *hdev)
return false;

e = hci_inquiry_cache_lookup_resolve(hdev, BDADDR_ANY, NAME_NEEDED);
+ if (!e)
+ return false;
+
if (hci_resolve_name(hdev, e) == 0) {
e->name_state = NAME_PENDING;
return true;
--
1.7.4.1

2012-07-18 12:43:52

by Malovany, Ram

Subject: RE: [PATCH 1/3] Bluetooth: Do not resolve name if entry was not found.

Hi Johan,

> -----Original Message-----
> From: Johan Hedberg [mailto:[email protected]]
> Sent: Wednesday, July 18, 2012 3:40 PM
> To: Malovany, Ram
> Cc: [email protected]
> Subject: Re: [PATCH 1/3] Bluetooth: Do not resolve name if entry was not
> found.
>
> Hi Ram,
>
> On Wed, Jul 18, 2012, [email protected] wrote:
> > From: Ram Malovany <[email protected]>
> >
> > If entry wasn't found in the hci_inquiry_cache_lookup_resolve do not
> > resolve the name.
> >
> > Signed-off-by: Ram Malovany <[email protected]>
> > ---
> > net/bluetooth/hci_event.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
>
> I'm fine with all of these three patches but you might consider
> improving the commit messages a bit. Especially with kernel patches it
> doesn't hurt to have a bit more verbosity.
>
> Also, formulating the summary line as "Fix ..." would be good since they
> *are* all fixes (this can help fast-tracking them upstream). It's
> particularly important for this first one to be clearly identifiable as
> a fix since it's fixing a NULL pointer dereference (something that
> should also have been mentioned in the commit message imo).
>
> Another smaller cosmetic issue with the messages: usually the summary
> lines do not end with a period.
>

Thanks for the remarks ,I will send an updates regarding the commit messages according to your comments.

> Johan

2012-07-18 12:39:56

by Johan Hedberg

Subject: Re: [PATCH 1/3] Bluetooth: Do not resolve name if entry was not found.

Hi Ram,

On Wed, Jul 18, 2012, [email protected] wrote:
> From: Ram Malovany <[email protected]>
>
> If entry wasn't found in the hci_inquiry_cache_lookup_resolve do not
> resolve the name.
>
> Signed-off-by: Ram Malovany <[email protected]>
> ---
> net/bluetooth/hci_event.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)

I'm fine with all of these three patches but you might consider
improving the commit messages a bit. Especially with kernel patches it
doesn't hurt to have a bit more verbosity.

Also, formulating the summary line as "Fix ..." would be good since they
*are* all fixes (this can help fast-tracking them upstream). It's
particularly important for this first one to be clearly identifiable as
a fix since it's fixing a NULL pointer dereference (something that
should also have been mentioned in the commit message imo).

Another smaller cosmetic issue with the messages: usually the summary
lines do not end with a period.

Johan

2012-07-18 12:06:06

by Malovany, Ram

Subject: [PATCH 3/3] Bluetooth: Set name_state to unknown when entry name is empty

From: Ram Malovany <[email protected]>

When the name of the given entry is empty , the state needs to be
updated accordingly.

Signed-off-by: Ram Malovany <[email protected]>
---
net/bluetooth/hci_event.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3224dcc..f799b46 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1408,6 +1408,8 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
e->name_state = NAME_KNOWN;
mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
e->data.rssi, name, name_len);
+ } else {
+ e->name_state = NAME_NOT_KNOWN;
}

if (hci_resolve_next_name(hdev))
--
1.7.4.1

2012-07-18 12:06:05

by Malovany, Ram

Subject: [PATCH 2/3] Bluetooth: Do not resolve next name if the entry was not found.

From: Ram Malovany <[email protected]>

If the device was not found in a list of found devices names
of which are pending.This may happen in a case when HCI Remote
Name Request was sent as a part of incoming connection
establishment procedure.Hence there is no need to continue resolving
a next name as it will be done upon receiving another Remote
Name Request Complete Event.

Signed-off-by: Ram Malovany <[email protected]>
---
net/bluetooth/hci_event.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2de521f..3224dcc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1396,12 +1396,18 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
return;

e = hci_inquiry_cache_lookup_resolve(hdev, bdaddr, NAME_PENDING);
- if (e) {
+ /* If the device was not found in a list of found devices names of which
+ * are pending. there is no need to continue resolving a next name as it
+ * will be done upon receiving another Remote Name Request Complete
+ * Event */
+ if (!e)
+ return;
+
+ list_del(&e->list);
+ if (name) {
e->name_state = NAME_KNOWN;
- list_del(&e->list);
- if (name)
- mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
- e->data.rssi, name, name_len);
+ mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
+ e->data.rssi, name, name_len);
}

if (hci_resolve_next_name(hdev))
--
1.7.4.1