Im seeing this building the kernel with gcc 4.6.0
CC [M] drivers/bluetooth/hci_bcsp.o
drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
Hopefully the below is a fix for this. Please let me know.
Signed-off-by: Justin P. Mattock <[email protected]>
---
drivers/bluetooth/hci_bcsp.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
index 40aec0f..0f892e7 100644
--- a/drivers/bluetooth/hci_bcsp.c
+++ b/drivers/bluetooth/hci_bcsp.c
@@ -182,7 +182,7 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
struct sk_buff *nskb;
u8 hdr[4], chan;
u16 BCSP_CRC_INIT(bcsp_txmsg_crc);
- int rel, i;
+ int rel, i, ret;
switch (pkt_type) {
case HCI_ACLDATA_PKT:
@@ -243,8 +243,8 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
if (rel) {
hdr[0] |= 0x80 + bcsp->msgq_txseq;
- BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
- bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
+ BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
+ ret = ++(bcsp->msgq_txseq) & 0x07;
}
if (bcsp->use_crc)
--
1.7.1.rc1.21.gf3bd6
From: "Gustavo F. Padovan" <[email protected]>
Date: Mon, 28 Jun 2010 10:12:30 -0300
> Hi David,
>
> * David Howells <[email protected]> [2010-06-28 13:57:52 +0100]:
>
>> Fix abuse of the preincrement operator as detected when building with gcc
>> 4.6.0:
>>
>> CC [M] drivers/bluetooth/hci_bcsp.o
>> drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
>> drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
>>
>> Reported-by: Justin P. Mattock <[email protected]>
>> Signed-off-by: David Howells <[email protected]>
...
> Acked-by: Gustavo F. Padovan <[email protected]>
Applied, thanks everyone.
On 06/28/2010 05:57 AM, David Howells wrote:
> Fix abuse of the preincrement operator as detected when building with gcc
> 4.6.0:
>
> CC [M] drivers/bluetooth/hci_bcsp.o
> drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
> drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
>
> Reported-by: Justin P. Mattock<[email protected]>
> Signed-off-by: David Howells<[email protected]>
> ---
>
> 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 40aec0f..42d69d4 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -244,7 +244,7 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
> if (rel) {
> hdr[0] |= 0x80 + bcsp->msgq_txseq;
> BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq)& 0x07;
> + bcsp->msgq_txseq = (bcsp->msgq_txseq + 1)& 0x07;
> }
>
> if (bcsp->use_crc)
>
>
ahh.. so it's o.k. to add the value after bcsp->msgq_txseq instead of
before. Anyways build clean over here..
Thanks!
Justin P. Mattock
On 06/28/2010 05:52 AM, David Howells wrote:
> Justin P. Mattock<[email protected]> wrote:
>
>> - BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
>> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq)& 0x07;
>> + BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
>> + ret = ++(bcsp->msgq_txseq)& 0x07;
>
> I don't know what you're trying to do here, but you seem to be trying to send
> the computed value back in time.
>
I was under the impression that hdr[0] |= 0x80 + bcsp->msgq_txseq;
is computing a value for BT_DBG then ret = ++(bcsp->msgq_txseq)& 0x07
computes a value as well i.e.
BT_DBG("Sending packet with seqno %u", hdr[0] | ret);
> The problem is that the compiler is confused about why a '++' operator makes
> any sense here. It doesn't. It should be a '+ 1' instead. I think what you
> want is:
>
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq)& 0x07;
> + bcsp->msgq_txseq = (bcsp->msgq_txseq + 1)& 0x07;
>
> David
>
yeah I did play around with the ++ and noticed the compiler would be
satisfied if the ++ was not there, but didn't think to just add a + 1
Justin P. Mattock
Hi David,
* David Howells <[email protected]> [2010-06-28 13:57:52 +0100]:
> Fix abuse of the preincrement operator as detected when building with gcc
> 4.6.0:
>
> CC [M] drivers/bluetooth/hci_bcsp.o
> drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
> drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
>
> Reported-by: Justin P. Mattock <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> ---
>
> 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 40aec0f..42d69d4 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -244,7 +244,7 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
> if (rel) {
> hdr[0] |= 0x80 + bcsp->msgq_txseq;
> BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> + bcsp->msgq_txseq = (bcsp->msgq_txseq + 1) & 0x07;
> }
>
> if (bcsp->use_crc)
>
Acked-by: Gustavo F. Padovan <[email protected]>
--
Gustavo F. Padovan
http://padovan.org
On Mon, 2010-06-28 at 13:52 +0100, David Howells wrote:
> Justin P. Mattock <[email protected]> wrote:
>
> > - BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> > - bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> > + BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
> > + ret = ++(bcsp->msgq_txseq) & 0x07;
>
> I don't know what you're trying to do here, but you seem to be trying to send
> the computed value back in time.
>
> The problem is that the compiler is confused about why a '++' operator makes
It's even worse as that expression is explicitly undefined (and should
be fixed anyways and unconditionally).
> any sense here. It doesn't. It should be a '+ 1' instead. I think what you
> want is:
>
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> + bcsp->msgq_txseq = (bcsp->msgq_txseq + 1) & 0x07;
Yes, that's looks like the most probable intention of it - at least for
one who doesn't know the bluetooth code.
Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at
Fix abuse of the preincrement operator as detected when building with gcc
4.6.0:
CC [M] drivers/bluetooth/hci_bcsp.o
drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
Reported-by: Justin P. Mattock <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
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 40aec0f..42d69d4 100644
--- a/drivers/bluetooth/hci_bcsp.c
+++ b/drivers/bluetooth/hci_bcsp.c
@@ -244,7 +244,7 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
if (rel) {
hdr[0] |= 0x80 + bcsp->msgq_txseq;
BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
- bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
+ bcsp->msgq_txseq = (bcsp->msgq_txseq + 1) & 0x07;
}
if (bcsp->use_crc)
Justin P. Mattock <[email protected]> wrote:
> - BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> + BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
> + ret = ++(bcsp->msgq_txseq) & 0x07;
I don't know what you're trying to do here, but you seem to be trying to send
the computed value back in time.
The problem is that the compiler is confused about why a '++' operator makes
any sense here. It doesn't. It should be a '+ 1' instead. I think what you
want is:
- bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
+ bcsp->msgq_txseq = (bcsp->msgq_txseq + 1) & 0x07;
David
Hi Justin,
* Justin P. Mattock <[email protected]> [2010-06-26 23:47:26 -0700]:
> Im seeing this building the kernel with gcc 4.6.0
> CC [M] drivers/bluetooth/hci_bcsp.o
> drivers/bluetooth/hci_bcsp.c: In function 'bcsp_prepare_pkt':
> drivers/bluetooth/hci_bcsp.c:247:20: warning: operation on 'bcsp->msgq_txseq' may be undefined
>
> Hopefully the below is a fix for this. Please let me know.
> Signed-off-by: Justin P. Mattock <[email protected]>
>
> ---
> drivers/bluetooth/hci_bcsp.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 40aec0f..0f892e7 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -182,7 +182,7 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
> struct sk_buff *nskb;
> u8 hdr[4], chan;
> u16 BCSP_CRC_INIT(bcsp_txmsg_crc);
> - int rel, i;
> + int rel, i, ret;
>
> switch (pkt_type) {
> case HCI_ACLDATA_PKT:
> @@ -243,8 +243,8 @@ static struct sk_buff *bcsp_prepare_pkt(struct bcsp_struct *bcsp, u8 *data,
>
> if (rel) {
> hdr[0] |= 0x80 + bcsp->msgq_txseq;
> - BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> - bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> + BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
> + ret = ++(bcsp->msgq_txseq) & 0x07;
> }
What are trying to do here? That is completely wrong, you are losting
the next txseq to be sent.
And please do not bother the linux-bluetooth mailing list with patches
to other subsystems. Send them in a way that only Bluetooth patches will
come to linux-bluetooth.
Regards,
--
Gustavo F. Padovan
http://padovan.org