patch for hci_bcsp.c(2.6.31.-rc1)

patch for hci_bcsp.c(2.6.31.-rc1)

[Weng, Wending]

>From 69596948eb2172080bb950d99dc90678ead08305 Mon Sep 17 00:00:00 2001
From: root <root@SBC_PC_3.localdomain>
Date: Wed, 19 Aug 2009 08:59:56 -0400
Subject: [PATCH] The routine bcsp_pkt_cull doesn't ack the packets properly
if multiple packets are queued. The counter i must increase before
doing comparison.

Signed-off-by: Wending Weng <wweng@rheinmetall.ca>

---
 drivers/bluetooth/hci_bcsp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
index 894b2cb..cd30f39 100644
--- a/drivers/bluetooth/hci_bcsp.c
+++ b/drivers/bluetooth/hci_bcsp.c
@@ -373,7 +373,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)

        i = 0;
        skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
-               if (i++ >= pkts_to_be_removed)
+               if (++i >= pkts_to_be_removed)
                        break;

                __skb_unlink(skb, &bcsp->unack);
--
1.5.2.1

Re: patch for hci_bcsp.c(2.6.31.-rc1)

[Marcel Holtmann]

Hi Wending,

> From 69596948eb2172080bb950d99dc90678ead08305 Mon Sep 17 00:00:00 2001
> From: root <root@SBC_PC_3.localdomain>
> Date: Wed, 19 Aug 2009 08:59:56 -0400
> Subject: [PATCH] The routine bcsp_pkt_cull doesn't ack the packets properly
> if multiple packets are queued. The counter i must increase before
> doing comparison.
> 
> Signed-off-by: Wending Weng <wweng@rheinmetall.ca>
> 
> ---
>  drivers/bluetooth/hci_bcsp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 894b2cb..cd30f39 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -373,7 +373,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
> 
>         i = 0;
>         skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
> -               if (i++ >= pkts_to_be_removed)
> +               if (++i >= pkts_to_be_removed)
>                         break;

wouldn't be if (i++ > pkts_to_be_removed) the better way to test this?

Regards

Marcel

RE: patch for hci_bcsp.c(2.6.31.-rc1)

[Weng, Wending]

Hi Marcel,

        If I keep "if (i++ >=3D pkts_to_be_removed)", the loop will end up =
with
        i=3D2 if pkts_to_be_removed=3D=3D1 and the second test skb_queue_wa=
lk_safe is true,

        it will cause error message later on:

        if (i !=3D pkts_to_be_removed) {
                BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_remo=
ved);
        }

        I think we should add another line for i++.

Regards

Wending Weng

-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org
[mailto:linux-bluetooth-owner@vger.kernel.org]On Behalf Of Marcel
Holtmann
Sent: August 22, 2009 4:06 PM
To: Weng, Wending
Cc: 'linux-bluetooth@vger.kernel.org'
Subject: Re: patch for hci_bcsp.c(2.6.31.-rc1)


Hi Wending,

> From 69596948eb2172080bb950d99dc90678ead08305 Mon Sep 17 00:00:00 2001
> From: root <root@SBC_PC_3.localdomain>
> Date: Wed, 19 Aug 2009 08:59:56 -0400
> Subject: [PATCH] The routine bcsp_pkt_cull doesn't ack the packets proper=
ly
> if multiple packets are queued. The counter i must increase before
> doing comparison.
>
> Signed-off-by: Wending Weng <wweng@rheinmetall.ca>
>
> ---
>  drivers/bluetooth/hci_bcsp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 894b2cb..cd30f39 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -373,7 +373,7 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
>
>         i =3D 0;
>         skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
> -               if (i++ >=3D pkts_to_be_removed)
> +               if (++i >=3D pkts_to_be_removed)
>                         break;

wouldn't be if (i++ > pkts_to_be_removed) the better way to test this?

Regards

Marcel


--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: patch for hci_bcsp.c(2.6.31.-rc1)

[Marcel Holtmann]

Hi Wending,

please don't top post. Use proper inline quoting.

>         If I keep "if (i++ >= pkts_to_be_removed)", the loop will end up with
>         i=2 if pkts_to_be_removed==1 and the second test skb_queue_walk_safe is true,
> 
>         it will cause error message later on:
> 
>         if (i != pkts_to_be_removed) {
>                 BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
>         }
> 
>         I think we should add another line for i++.

Not following? I am saying replace >= with >. Would that work too?
Otherwise please send a patch with a more detailed commit message.

Regards

Marcel

RE: patch for hci_bcsp.c(2.6.31.-rc1)

[Weng, Wending]

Hi Marcel,

>Not following? I am saying replace >=3D with >. Would that work too?
>Otherwise please send a patch with a more detailed commit message.
>
>Regards
>
>Marcel

        replace >=3D with > will not work. Below is the new patch.

From: root <root@SBC_PC_3.localdomain>
Date: Mon, 24 Aug 2009 14:06:18 -0400
Subject: [PATCH] The routine bcsp_pkt_cull displays the false error message
"Removed only %u out of %u pkts" when multiple to be acked packets are queu=
ed.
As if (i++ >=3D pkts_to_be_removed)
        break;
   will breaks the loop and increase the counter i when i=3D=3Dpkts_to_be_r=
emoved,
   the loop ends up with i=3Dpkts_to_be_removed+1. The following line:
   if (i !=3D pkts_to_be_removed) {
        BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
   }
   will display the false message.
The counter i must not increase on the same line.
---
 drivers/bluetooth/hci_bcsp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
index 894b2cb..40aec0f 100644
--- a/drivers/bluetooth/hci_bcsp.c
+++ b/drivers/bluetooth/hci_bcsp.c
@@ -373,8 +373,9 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)

        i =3D 0;
        skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
-               if (i++ >=3D pkts_to_be_removed)
+               if (i >=3D pkts_to_be_removed)
                        break;
+               i++;

                __skb_unlink(skb, &bcsp->unack);
                kfree_skb(skb);
--
1.5.2.1

Regards

Wending

RE: patch for hci_bcsp.c(2.6.31.-rc1)

[Marcel Holtmann]

Hi Wending,

>         replace >= with > will not work. Below is the new patch.
> 
> From: root <root@SBC_PC_3.localdomain>
> Date: Mon, 24 Aug 2009 14:06:18 -0400
> Subject: [PATCH] The routine bcsp_pkt_cull displays the false error message
> "Removed only %u out of %u pkts" when multiple to be acked packets are queued.
> As if (i++ >= pkts_to_be_removed)
>         break;
>    will breaks the loop and increase the counter i when i==pkts_to_be_removed,
>    the loop ends up with i=pkts_to_be_removed+1. The following line:
>    if (i != pkts_to_be_removed) {
>         BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
>    }
>    will display the false message.
> The counter i must not increase on the same line.
> ---
>  drivers/bluetooth/hci_bcsp.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 894b2cb..40aec0f 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -373,8 +373,9 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)
> 
>         i = 0;
>         skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
> -               if (i++ >= pkts_to_be_removed)
> +               if (i >= pkts_to_be_removed)
>                         break;
> +               i++;
> 
>                 __skb_unlink(skb, &bcsp->unack);
>                 kfree_skb(skb);

looks good, but I need a patch that applies with bluetooth-testing.git
tree. And please create it with git format-patch.

Regards

Marcel

RE: patch for hci_bcsp.c(bluetooth-testing)

[Weng, Wending]

Hi Marcel,

>Hi Wending,

>looks good, but I need a patch that applies with bluetooth-testing.git
>tree. And please create it with git format-patch.
>
>Regards
>
>Marcel

Below is the patch created with bluetooth-testing.git tree, let me know if it's not done properly, I'm not very experienced with open source.

>From 5c6e77cb6ea3ba6fa5c777d151c480451602bfc8 Mon Sep 17 00:00:00 2001
From: root <root@SBC_PC_3.localdomain>
Date: Mon, 24 Aug 2009 16:05:17 -0400
Subject: [PATCH] The routine bcsp_pkt_cull displays the false error message
"Removed only %u out of %u pkts" when multiple to be acked packets are queued.
As if (i++ >= pkts_to_be_removed)
        break;
   will break the loop and increase the counter i when i==pkts_to_be_removed,
   the loop ends up with i=pkts_to_be_removed+1. The following line:
   if (i != pkts_to_be_removed) {
        BT_ERR("Removed only %u out of %u pkts", i, pkts_to_be_removed);
   }
   will display the false message.
The counter i must not increase on the same line.

signed-off-by: Wending Weng wweng@rheinmetall.ca
---
 drivers/bluetooth/hci_bcsp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
index 894b2cb..40aec0f 100644
--- a/drivers/bluetooth/hci_bcsp.c
+++ b/drivers/bluetooth/hci_bcsp.c
@@ -373,8 +373,9 @@ static void bcsp_pkt_cull(struct bcsp_struct *bcsp)

        i = 0;
        skb_queue_walk_safe(&bcsp->unack, skb, tmp) {
-               if (i++ >= pkts_to_be_removed)
+               if (i >= pkts_to_be_removed)
                        break;
+               i++;

                __skb_unlink(skb, &bcsp->unack);
                kfree_skb(skb);
--
1.5.2.1

regards

Wending

RE: patch for hci_bcsp.c(bluetooth-testing)

[Marcel Holtmann]

Hi Wending,

> >looks good, but I need a patch that applies with bluetooth-testing.git
> >tree. And please create it with git format-patch.
> >
>
> Below is the patch created with bluetooth-testing.git tree, let me know if it's not done properly, I'm not very experienced with open source.

your mailer is fully broken. It messes up tabes and whitespaces :(

It did fix it manually now, but next time it is up to you.

Regards

Marcel