(bug report) b43: precendence error

(bug report) b43: precendence error

[Dan Carpenter]

[ All the old wireless Smatch warnings are showing up as new ones
  because of the path reshuffle in linux-next.  I'm going through and
  reporting the extra suspicious ones.  -dan ]

Hello Rafał Miłecki,

The patch 6f98e62a9f1b: "b43: update cordic code to match current
specs" from Jan 25, 2010, leads to the following static checker
warning:

	drivers/net/wireless/broadcom/b43/phy_lp.c:1803 lpphy_start_tx_tone()
	warn: mask and shift to zero

drivers/net/wireless/broadcom/b43/phy_lp.c
  1800          for (i = 0; i < samples; i++) {
  1801                  sample = b43_cordic(angle);
  1802                  angle += rotation;
  1803                  buf[i] = CORDIC_CONVERT((sample.i * max) & 0xFF) << 8;
  1804                  buf[i] |= CORDIC_CONVERT((sample.q * max) & 0xFF);

Maybe the intention was:

			buf[i] = (CORDIC_CONVERT(sample.i * max) & 0xFF) << 8;
			buf[i] |= CORDIC_CONVERT((sample.q * max) & 0xFF;

  1805          }
  1806  

regards,
dan carpenter

Re: (bug report) b43: precendence error

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

On Thu, 26 Nov 2015 14:58:43 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> [ All the old wireless Smatch warnings are showing up as new ones
>   because of the path reshuffle in linux-next.  I'm going through and
>   reporting the extra suspicious ones.  -dan ]
> 
> Hello Rafał Miłecki,
> 
> The patch 6f98e62a9f1b: "b43: update cordic code to match current
> specs" from Jan 25, 2010, leads to the following static checker
> warning:
> 
> 	drivers/net/wireless/broadcom/b43/phy_lp.c:1803 lpphy_start_tx_tone()
> 	warn: mask and shift to zero
> 
> drivers/net/wireless/broadcom/b43/phy_lp.c
>   1800          for (i = 0; i < samples; i++) {
>   1801                  sample = b43_cordic(angle);
>   1802                  angle += rotation;
>   1803                  buf[i] = CORDIC_CONVERT((sample.i * max) & 0xFF) << 8;
>   1804                  buf[i] |= CORDIC_CONVERT((sample.q * max) & 0xFF);
> 
> Maybe the intention was:
> 
> 			buf[i] = (CORDIC_CONVERT(sample.i * max) & 0xFF) << 8;
> 			buf[i] |= CORDIC_CONVERT((sample.q * max) & 0xFF;
> 
>   1805          }
>   1806  


This looks like a bug indeed.
Rafał, do you have hw to check this?

 			buf[i] = (CORDIC_CONVERT(sample.i * max) & 0xFF) << 8;
 			buf[i] |= CORDIC_CONVERT(sample.q * max) & 0xFF;

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]