Skip to contentSkip to conversation

Bluetooth: i reaches pkts_to_be_removed + 1

PATCH In review Roel Kluin 3 participants 6 messages Feb 25, 2009 – Mar 2, 2009 Bluetooth drivers

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 Bluetooth: i reaches pkts_to_be_removed + 1

i reaches pkts_to_be_removed + 1

Signed-off-by Roel Kluin <[email protected]>

---

drivers/bluetooth/hci_bcsp.c +1 -1
index 894b2cb..177f34b 100644
@@ -385,7 +385,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
 
-	if (i != pkts_to_be_removed)
+	if (i <= pkts_to_be_removed)
 		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }
maintainer
#1 lore raw

Hi Roel,

Quoted 9 lines

i reaches pkts_to_be_removed + 1

Signed-off-by Roel Kluin <[email protected]>

---

drivers/bluetooth/hci_bcsp.c +0 -0
index 894b2cb..177f34b 100644

drivers/bluetooth/hci_bcsp.c static void bcsp_pkt_cull(struct bcsp_struct *bcsp) · line 390

Quoted 7 lines
drivers/bluetooth/hci_bcsp.c +1 -1
@@ -385,7 +385,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
 
-	if (i != pkts_to_be_removed)
+	if (i <= pkts_to_be_removed)
 		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }

patch looks good, but can you please fill in the commit message with a little bit more details and background.

Regards

Marcel

author
#2 lore raw
Quoted 4 lines
Quoted 1 line

i reaches pkts_to_be_removed + 1

patch looks good, but can you please fill in the commit message with a little bit more details and background.

This was found by code inspection by the way, and not tested.

how about:
------------------------------>8-------------8<--------------------------------- When all packets are removed 'i' reaches pkts_to_be_removed + 1, so the error message was printed upon success, and not when the last packet wasn't removed.

Signed-off-by Roel Kluin <[email protected]>

---

drivers/bluetooth/hci_bcsp.c +1 -1
index 894b2cb..177f34b 100644
@@ -385,7 +385,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
 
-	if (i != pkts_to_be_removed)
+	if (i <= pkts_to_be_removed)
 		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }
#3 lore raw
In reply to Marcel Holtmann
Quoted 15 lines · Marcel Holtmann

Hi Roel,

Quoted 12 lines

i reaches pkts_to_be_removed + 1

Signed-off-by Roel Kluin <[email protected]>

---

drivers/bluetooth/hci_bcsp.c +0 -0
index 894b2cb..177f34b 100644
@@ -385,7 +385,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
Quoted 9 lines

drivers/bluetooth/hci_bcsp.c static void bcsp_pkt_cull(struct bcsp_struct *bcsp) · line 391

Quoted 5 lines
drivers/bluetooth/hci_bcsp.c +1 -1
-	if (i != pkts_to_be_removed)
+	if (i <= pkts_to_be_removed)
 		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }

patch looks good, but can you please fill in the commit message with a little bit more details and background.

Isn't there something wrong with this? The patched code looks like it will output apparently nonsensical messages like "Removed only 10 out of 10 pkts".

maintainer
#4 lore raw

Hi David,

Quoted 18 lines
Quoted 8 lines
Quoted 8 lines
drivers/bluetooth/hci_bcsp.c +0 -0
index 894b2cb..177f34b 100644
@@ -385,7 +385,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
Quoted 9 lines

drivers/bluetooth/hci_bcsp.c static void bcsp_pkt_cull(struct bcsp_struct *bcsp) · line 391

Quoted 5 lines
drivers/bluetooth/hci_bcsp.c +1 -1
-	if (i != pkts_to_be_removed)
+	if (i <= pkts_to_be_removed)
 		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }

patch looks good, but can you please fill in the commit message with a little bit more details and background.

Quoted 3 lines

Isn't there something wrong with this? The patched code looks like it will output apparently nonsensical messages like "Removed only 10 out of 10 pkts".

yeah, that would make no real sense. I actually just thought about why we have that error message at all. We might should remove the message all together.

Regards

Marcel

author
#5 lore raw
Quoted 7 lines
Quoted 4 lines

Isn't there something wrong with this? The patched code looks like it will output apparently nonsensical messages like "Removed only 10 out of 10 pkts".

yeah, that would make no real sense. I actually just thought about why we have that error message at all. We should remove the message all together.

------------------------------>8-------------8<--------------------------------- When all packets are removed 'i' reaches pkts_to_be_removed + 1, so the error message was printed upon success, and not when the last packet wasn't removed. Since it results in nonsensical messages, it was decided to remove the test altogether.

Signed-off-by Roel Kluin <[email protected]>

---

drivers/bluetooth/hci_bcsp.c +0 -3
index 894b2cb..52c6bdc 100644
@@ -384,9 +384,6 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
 		del_timer(&bcsp->tbcsp);
 
 	spin_unlock_irqrestore(&bcsp->unack.lock, flags);
-
-	if (i != pkts_to_be_removed)
-		BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
 }
 
 /* Handle BCSP link-establishment packets. When we