[PATCH] Staging: rtl8192u: Simplify if condition

[PATCH] Staging: rtl8192u: Simplify if condition

[Ksenija Stanojevic]

This patch simplifies if condition using following Coccinelle script:
@@ bool a; symbol true; @@
(
- a != true
+ !a
|
- a == true
+ a
)
@@ bool a; symbol false; @@
(
- a != false
+ a
|
- a == false
+ !a
)

Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/r8192U_wx.c b/drivers/staging/rtl8192u/r8192U_wx.c
index b5a26f3..8359705 100644
--- a/drivers/staging/rtl8192u/r8192U_wx.c
+++ b/drivers/staging/rtl8192u/r8192U_wx.c
@@ -335,7 +335,7 @@ static int r8192_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
 	if (!priv->up)
 		return -ENETDOWN;
 
-	if (priv->ieee80211->LinkDetectInfo.bBusyTraffic == true)
+	if (priv->ieee80211->LinkDetectInfo.bBusyTraffic)
 		return -EAGAIN;
 	if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
 		struct iw_scan_req *req = (struct iw_scan_req *)b;
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition

[Jes Sorensen]

On 03/04/15 18:32, Ksenija Stanojevic wrote:
> This patch simplifies if condition using following Coccinelle script:
> @@ bool a; symbol true; @@
> (
> - a != true
> + !a
> |
> - a == true
> + a
> )
> @@ bool a; symbol false; @@
> (
> - a != false
> + a
> |
> - a == false
> + !a
> )
> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U_wx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

The patch is correct, however you should write a proper commit message
rather than just relying on the coccinelle script.

It's important to try and show in a commit message that you know why you
made the changes, and not just comply with an automated tool.

Cheers,
Jes


> diff --git a/drivers/staging/rtl8192u/r8192U_wx.c b/drivers/staging/rtl8192u/r8192U_wx.c
> index b5a26f3..8359705 100644
> --- a/drivers/staging/rtl8192u/r8192U_wx.c
> +++ b/drivers/staging/rtl8192u/r8192U_wx.c
> @@ -335,7 +335,7 @@ static int r8192_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
>  	if (!priv->up)
>  		return -ENETDOWN;
>  
> -	if (priv->ieee80211->LinkDetectInfo.bBusyTraffic == true)
> +	if (priv->ieee80211->LinkDetectInfo.bBusyTraffic)
>  		return -EAGAIN;
>  	if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
>  		struct iw_scan_req *req = (struct iw_scan_req *)b;
> 

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition

[Ksenija Stanojević]

On Thu, Mar 5, 2015 at 2:37 AM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
> On 03/04/15 18:32, Ksenija Stanojevic wrote:
>> This patch simplifies if condition using following Coccinelle script:
>> @@ bool a; symbol true; @@
>> (
>> - a != true
>> + !a
>> |
>> - a == true
>> + a
>> )
>> @@ bool a; symbol false; @@
>> (
>> - a != false
>> + a
>> |
>> - a == false
>> + !a
>> )
>>
>> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
>> ---
>>  drivers/staging/rtl8192u/r8192U_wx.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> The patch is correct, however you should write a proper commit message
> rather than just relying on the coccinelle script.
>
> It's important to try and show in a commit message that you know why you
> made the changes, and not just comply with an automated tool.
>
> Cheers,
> Jes

 Thanks for advice however it's not a message from automated tool, it was late
when I sent this patch and I was very tired. Also I previously sent
similar patches with
 better explanation.
But i will try to always include why I did ceratain change.

Ksenija

>
>> diff --git a/drivers/staging/rtl8192u/r8192U_wx.c b/drivers/staging/rtl8192u/r8192U_wx.c
>> index b5a26f3..8359705 100644
>> --- a/drivers/staging/rtl8192u/r8192U_wx.c
>> +++ b/drivers/staging/rtl8192u/r8192U_wx.c
>> @@ -335,7 +335,7 @@ static int r8192_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
>>       if (!priv->up)
>>               return -ENETDOWN;
>>
>> -     if (priv->ieee80211->LinkDetectInfo.bBusyTraffic == true)
>> +     if (priv->ieee80211->LinkDetectInfo.bBusyTraffic)
>>               return -EAGAIN;
>>       if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
>>               struct iw_scan_req *req = (struct iw_scan_req *)b;
>>
>

[PATCH] Staging: rtl8192u: Simplify if condition.

[Ksenija Stanojevic]

Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
bDynamicTxHighPower are of bool type so such change is correct.

Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
 drivers/staging/rtl8192u/r8190_rtl8256.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8190_rtl8256.c b/drivers/staging/rtl8192u/r8190_rtl8256.c
index 1868352..e000329 100644
--- a/drivers/staging/rtl8192u/r8190_rtl8256.c
+++ b/drivers/staging/rtl8192u/r8190_rtl8256.c
@@ -225,7 +225,7 @@ void PHY_SetRF8256CCKTxPower(struct net_device *dev, u8 powerlevel)
 	struct r8192_priv *priv = ieee80211_priv(dev);
 	TxAGC = powerlevel;
 
-	if (priv->bDynamicTxLowPower == TRUE) {
+	if (priv->bDynamicTxLowPower) {
 		if (priv->CustomerID == RT_CID_819x_Netcore)
 			TxAGC = 0x22;
 		else
@@ -275,7 +275,7 @@ void PHY_SetRF8256OFDMTxPower(struct net_device *dev, u8 powerlevel)
 			priv->Pwr_Track = writeVal_tmp;
 		}
 
-		if (priv->bDynamicTxHighPower == TRUE) {
+		if (priv->bDynamicTxHighPower) {
 			/*Add by Jacken 2008/03/06
 			 *Emily, 20080613. Set low tx power for both MCS and legacy OFDM
 			 */
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition.

[Julia Lawall]

On Tue, 24 Feb 2015, Ksenija Stanojevic wrote:

> Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
> bDynamicTxHighPower are of bool type so such change is correct.

I'm not sure that the compiler actually enforces that a bool variable is 
only ever assigned to 0 or 1.  The kernel contains the following in 
types.h:

typedef _Bool                   bool;

The following (non-kernel) C program compiles with gcc with no errors:

_Bool x;

int main () {
  x = 12;
}

To be on the safe side, you should look at each place where the field is 
initialized and check that the value is true or false.  If that is what 
you have already done, then you should expand on the commit message to 
make that more apparent.

julia

> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8190_rtl8256.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8190_rtl8256.c b/drivers/staging/rtl8192u/r8190_rtl8256.c
> index 1868352..e000329 100644
> --- a/drivers/staging/rtl8192u/r8190_rtl8256.c
> +++ b/drivers/staging/rtl8192u/r8190_rtl8256.c
> @@ -225,7 +225,7 @@ void PHY_SetRF8256CCKTxPower(struct net_device *dev, u8 powerlevel)
>  	struct r8192_priv *priv = ieee80211_priv(dev);
>  	TxAGC = powerlevel;
>  
> -	if (priv->bDynamicTxLowPower == TRUE) {
> +	if (priv->bDynamicTxLowPower) {
>  		if (priv->CustomerID == RT_CID_819x_Netcore)
>  			TxAGC = 0x22;
>  		else
> @@ -275,7 +275,7 @@ void PHY_SetRF8256OFDMTxPower(struct net_device *dev, u8 powerlevel)
>  			priv->Pwr_Track = writeVal_tmp;
>  		}
>  
> -		if (priv->bDynamicTxHighPower == TRUE) {
> +		if (priv->bDynamicTxHighPower) {
>  			/*Add by Jacken 2008/03/06
>  			 *Emily, 20080613. Set low tx power for both MCS and legacy OFDM
>  			 */
> -- 
> 1.9.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1424811248-8150-1-git-send-email-ksenija.stanojevic%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition.

[Arnd Bergmann]

On Tuesday 24 February 2015 22:24:23 Julia Lawall wrote:
> On Tue, 24 Feb 2015, Ksenija Stanojevic wrote:
> 
> > Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
> > bDynamicTxHighPower are of bool type so such change is correct.
> 
> I'm not sure that the compiler actually enforces that a bool variable is 
> only ever assigned to 0 or 1.  The kernel contains the following in 
> types.h:
> 
> typedef _Bool                   bool;
> 
> The following (non-kernel) C program compiles with gcc with no errors:
> 
> _Bool x;
> 
> int main () {
>   x = 12;
> }
> 
> To be on the safe side, you should look at each place where the field is 
> initialized and check that the value is true or false.  If that is what 
> you have already done, then you should expand on the commit message to 
> make that more apparent.

Actually C enforces this:

_Bool x;
int main()
{ 
        x = 12;
 
        if (x == 1)
                return 1;
        else
                return 2;
}

The compiler ensures that this program returns 1, not 2.

	Arnd

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition.

[Julia Lawall]

On Tue, 24 Feb 2015, Arnd Bergmann wrote:

> On Tuesday 24 February 2015 22:24:23 Julia Lawall wrote:
> > On Tue, 24 Feb 2015, Ksenija Stanojevic wrote:
> > 
> > > Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
> > > bDynamicTxHighPower are of bool type so such change is correct.
> > 
> > I'm not sure that the compiler actually enforces that a bool variable is 
> > only ever assigned to 0 or 1.  The kernel contains the following in 
> > types.h:
> > 
> > typedef _Bool                   bool;
> > 
> > The following (non-kernel) C program compiles with gcc with no errors:
> > 
> > _Bool x;
> > 
> > int main () {
> >   x = 12;
> > }
> > 
> > To be on the safe side, you should look at each place where the field is 
> > initialized and check that the value is true or false.  If that is what 
> > you have already done, then you should expand on the commit message to 
> > make that more apparent.
> 
> Actually C enforces this:
> 
> _Bool x;
> int main()
> { 
>         x = 12;
>  
>         if (x == 1)
>                 return 1;
>         else
>                 return 2;
> }
> 
> The compiler ensures that this program returns 1, not 2.

Amazing.  Thanks.

julia

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition.

[Julia Lawall]

On Tue, 24 Feb 2015, Arnd Bergmann wrote:

> On Tuesday 24 February 2015 22:24:23 Julia Lawall wrote:
> > On Tue, 24 Feb 2015, Ksenija Stanojevic wrote:
> > 
> > > Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
> > > bDynamicTxHighPower are of bool type so such change is correct.
> > 
> > I'm not sure that the compiler actually enforces that a bool variable is 
> > only ever assigned to 0 or 1.  The kernel contains the following in 
> > types.h:
> > 
> > typedef _Bool                   bool;
> > 
> > The following (non-kernel) C program compiles with gcc with no errors:
> > 
> > _Bool x;
> > 
> > int main () {
> >   x = 12;
> > }
> > 
> > To be on the safe side, you should look at each place where the field is 
> > initialized and check that the value is true or false.  If that is what 
> > you have already done, then you should expand on the commit message to 
> > make that more apparent.
> 
> Actually C enforces this:
> 
> _Bool x;
> int main()
> { 
>         x = 12;
>  
>         if (x == 1)
>                 return 1;
>         else
>                 return 2;
> }
> 
> The compiler ensures that this program returns 1, not 2.

Ksenija, in this case you can do it easily with Coccinelle.  One rule 
would be:

@@
bool x;
@@

- x == true
+ x

There are three other cases to consider.

Of course, you could also try sed, but then you wouldn't have the 
verification of the bool type. 

julia

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Simplify if condition.

[Ksenija Stanojević]

On Wed, Feb 25, 2015 at 7:40 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Tue, 24 Feb 2015, Arnd Bergmann wrote:
>
>> On Tuesday 24 February 2015 22:24:23 Julia Lawall wrote:
>> > On Tue, 24 Feb 2015, Ksenija Stanojevic wrote:
>> >
>> > > Remove unnecessary TRUE statement. Fields bDynamicTxLowPower and
>> > > bDynamicTxHighPower are of bool type so such change is correct.
>> >
>> > I'm not sure that the compiler actually enforces that a bool variable is
>> > only ever assigned to 0 or 1.  The kernel contains the following in
>> > types.h:
>> >
>> > typedef _Bool                   bool;
>> >
>> > The following (non-kernel) C program compiles with gcc with no errors:
>> >
>> > _Bool x;
>> >
>> > int main () {
>> >   x = 12;
>> > }
>> >
>> > To be on the safe side, you should look at each place where the field is
>> > initialized and check that the value is true or false.  If that is what
>> > you have already done, then you should expand on the commit message to
>> > make that more apparent.
>>
>> Actually C enforces this:
>>
>> _Bool x;
>> int main()
>> {
>>         x = 12;
>>
>>         if (x == 1)
>>                 return 1;
>>         else
>>                 return 2;
>> }
>>
>> The compiler ensures that this program returns 1, not 2.
>
> Ksenija, in this case you can do it easily with Coccinelle.  One rule
> would be:
>
> @@
> bool x;
> @@
>
> - x == true
> + x
>
> There are three other cases to consider.
>
> Of course, you could also try sed, but then you wouldn't have the
> verification of the bool type.
>
> julia

Thank you.
I will try with Coccinelle script on other files too.