[PATCH v2 1/3] staging: rtl8712: Place constants on the right side of '&'

[PATCH v2 1/3] staging: rtl8712: Place constants on the right side of '&'

[Bhaktipriya Shridhar]

Move constants to the right of binary operator '&'.
This issue was identified using the Coccinelle script:
@@
constant c,c1;
expression e,e1,e2;
local idexpression i;
binary operator b = {==,!=,&,|};
@@

(
c b (c1)
|
i b e1
|
c | e1 | e2 | ...
|
c | (e ? e1 : e2)
|
- c
+ e
b
- e
+ c
)

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 Changes in v2:
	-change subject line
	-inserted Coccinelle script
 drivers/staging/rtl8712/wifi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/wifi.h b/drivers/staging/rtl8712/wifi.h
index 7a352c4..ee41971 100644
--- a/drivers/staging/rtl8712/wifi.h
+++ b/drivers/staging/rtl8712/wifi.h
@@ -260,12 +260,12 @@ enum WIFI_REG_DOMAIN {
 	*(unsigned short *)((addr_t)(pbuf) + 22) = \
 	((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
 	le16_to_cpu((unsigned short)0x000f)) | \
-	le16_to_cpu((unsigned short)(0xfff0 & (num << 4))); \
+	le16_to_cpu((unsigned short)((num << 4) & 0xfff0)); \
 })

 #define SetDuration(pbuf, dur) ({ \
 	*(unsigned short *)((addr_t)(pbuf) + 2) |= \
-	cpu_to_le16(0xffff & (dur)); \
+	cpu_to_le16((dur) & 0xffff); \
 })

 #define SetPriority(pbuf, tid) ({ \
--
2.1.4

Re: [Outreachy kernel] [PATCH v2 1/3] staging: rtl8712: Place constants on the right side of '&'

[Greg KH]

On Sun, Feb 21, 2016 at 01:51:06PM +0530, Bhaktipriya Shridhar wrote:
> Move constants to the right of binary operator '&'.
> This issue was identified using the Coccinelle script:
> @@
> constant c,c1;
> expression e,e1,e2;
> local idexpression i;
> binary operator b = {==,!=,&,|};
> @@
> 
> (
> c b (c1)
> |
> i b e1
> |
> c | e1 | e2 | ...
> |
> c | (e ? e1 : e2)
> |
> - c
> + e
> b
> - e
> + c
> )
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	-change subject line
> 	-inserted Coccinelle script
>  drivers/staging/rtl8712/wifi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/wifi.h b/drivers/staging/rtl8712/wifi.h
> index 7a352c4..ee41971 100644
> --- a/drivers/staging/rtl8712/wifi.h
> +++ b/drivers/staging/rtl8712/wifi.h
> @@ -260,12 +260,12 @@ enum WIFI_REG_DOMAIN {
>  	*(unsigned short *)((addr_t)(pbuf) + 22) = \
>  	((*(unsigned short *)((addr_t)(pbuf) + 22)) & \
>  	le16_to_cpu((unsigned short)0x000f)) | \
> -	le16_to_cpu((unsigned short)(0xfff0 & (num << 4))); \
> +	le16_to_cpu((unsigned short)((num << 4) & 0xfff0)); \
>  })
> 
>  #define SetDuration(pbuf, dur) ({ \
>  	*(unsigned short *)((addr_t)(pbuf) + 2) |= \
> -	cpu_to_le16(0xffff & (dur)); \
> +	cpu_to_le16((dur) & 0xffff); \

Why?  What does this change really?  Is it needed?  The code is just as
readable before as after, which should be the only reason you do stuff
like this.

greg k-h