[PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Amitoj Kaur Chawla]

Removes variable comparison with 0 by using !. 
Done using following coccinelle script.

@ disable is_zero,isnt_zero @
expression *E;
expression E1,f;
@@

E = f(...)
<...
(
- E == 0
+ !E
|
- E != 0
+ E
|
- 0 == E
+ !E
|
- 0 != E
+ E
)
...>
?E = E1

@ disable is_zero,isnt_zero @
expression *E;
@@

(
  E ==
- 0
+ NULL
|
  E !=
- 0
+ NULL
|
- 0
+ NULL
  == E
|
- 0
+ NULL
  != E
)

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/staging/rtl8188eu/hal/odm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c
index 28b5e7b..bc2dca4 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -650,7 +650,7 @@ void odm_DIG(struct odm_dm_struct *pDM_Odm)
 
 	} else {
 		/* Recovery mechanism for IGI lower bound */
-		if (pDM_DigTable->Recover_cnt != 0) {
+		if (pDM_DigTable->Recover_cnt) {
 			pDM_DigTable->Recover_cnt--;
 		} else {
 			if (pDM_DigTable->LargeFAHit < 3) {
@@ -851,7 +851,7 @@ void ODM_RF_Saving(struct odm_dm_struct *pDM_Odm, u8 bForceInNormal)
 		Rssi_Up_bound = 50;
 		Rssi_Low_bound = 45;
 	}
-	if (pDM_PSTable->initialize == 0) {
+	if (!pDM_PSTable->initialize) {
 		pDM_PSTable->Reg874 = (phy_query_bb_reg(adapter, 0x874, bMaskDWord)&0x1CC000)>>14;
 		pDM_PSTable->RegC70 = (phy_query_bb_reg(adapter, 0xc70, bMaskDWord)&BIT3)>>3;
 		pDM_PSTable->Reg85C = (phy_query_bb_reg(adapter, 0x85c, bMaskDWord)&0xFF000000)>>24;
@@ -1180,7 +1180,7 @@ void odm_RSSIMonitorCheckCE(struct odm_dm_struct *pDM_Odm)
 		}
 	}
 
-	if (tmpEntryMaxPWDB != 0)	/*  If associated entry is found */
+	if (tmpEntryMaxPWDB)	/*  If associated entry is found */
 		pdmpriv->EntryMaxUndecoratedSmoothedPWDB = tmpEntryMaxPWDB;
 	else
 		pdmpriv->EntryMaxUndecoratedSmoothedPWDB = 0;
-- 
1.9.1

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Dan Carpenter]

On Fri, Apr 03, 2015 at 10:12:11PM +0530, Amitoj Kaur Chawla wrote:
> Removes variable comparison with 0 by using !. 

Sometimes testing for zero makes sense.  When you write code, you are
telling a story.  If you are talking about zero as a number then it
can make sense.  If it's zero as a boolean then it doesn't make sense.

Also strcmp() and similar should always be done as == 0, < 0 or != 0
because that is the idiom:

	if name != "foo" then

becomes:

	if (strcmpt(name, "foo") != 0) {

The != from the first is shifted in the second.

So I don't really think this approach is the right thing.  You have to
read the code and understand the story it is telling.  Then change it
if needed.

regards,
dan carpenter

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Joe Perches]

On Fri, 2015-04-03 at 19:51 +0300, Dan Carpenter wrote:
> On Fri, Apr 03, 2015 at 10:12:11PM +0530, Amitoj Kaur Chawla wrote:
> > Removes variable comparison with 0 by using !. 
> 
> Sometimes testing for zero makes sense.
> When you write code, you are
> telling a story.  If you are talking about zero as a number then it
> can make sense.  If it's zero as a boolean then it doesn't make sense.

Very true.

> Also strcmp() and similar should always be done as == 0, < 0 or != 0
> because that is the idiom:

Less true.

When testing for equality, !strcmp is very common.

There are ~2500 uses of !strcmp in the kernel tree vs
~1500 uses of strcmp() == or !=

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Dan Carpenter]

On Fri, Apr 03, 2015 at 10:01:10AM -0700, Joe Perches wrote:
> On Fri, 2015-04-03 at 19:51 +0300, Dan Carpenter wrote:
> > Also strcmp() and similar should always be done as == 0, < 0 or != 0
> > because that is the idiom:
> 
> Less true.
> 
> When testing for equality, !strcmp is very common.
> 
> There are ~2500 uses of !strcmp in the kernel tree vs
> ~1500 uses of strcmp() == or !=

Bugs with reversed strcmp() tests are almost always caught in testing so
it's not an issue.  But == 0 is more correct.  ;)

1)  It's more clear when read in English.  "if not strcmp then" or
"if strcmp NOT EQUAL zero".  In the second one I've emphasized the
NOT EQUAL because the strings are not eqaul.

2)  Also if works for the other compares too.

	if (strcmp(x, y) < 0)  <-- means x is less than y.
	if (strcmp(x, y) == 0) <-- means x == y.

regards,
dan carpenter

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Dan Carpenter]

On Sat, Apr 04, 2015 at 12:04:59AM +0300, Dan Carpenter wrote:

> 1)  It's more clear when read in English.  "if not strcmp then" or
> "if strcmp NOT EQUAL zero".

Oops.  I got that reversed.  I meant "if strcmp then".  Only serves to
emphasize my point though.  :)

regards,
dan carpenter

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Joe Perches]

On Sat, 2015-04-04 at 16:58 +0300, Dan Carpenter wrote:
> On Sat, Apr 04, 2015 at 12:04:59AM +0300, Dan Carpenter wrote:
> > 1)  It's more clear when read in English.  "if not strcmp then" or
> > "if strcmp NOT EQUAL zero".
> Oops.  I got that reversed.  I meant "if strcmp then".  Only serves to
> emphasize my point though.  :)

When I read it I understood what you meant, but
I thought the point was everybody makes mistakes.

!strcmp to test equality is still pretty idiomatic.
!memcmp too.

cheers, Joe

Re: [PATCH] Staging: rtl8188eu: Remove zero testing pointer typed value

[Amitoj Kaur Chawla]

On Fri, Apr 3, 2015 at 10:21 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Fri, Apr 03, 2015 at 10:12:11PM +0530, Amitoj Kaur Chawla wrote:
> > Removes variable comparison with 0 by using !.
>
> Sometimes testing for zero makes sense.  When you write code, you are
> telling a story.  If you are talking about zero as a number then it
> can make sense.  If it's zero as a boolean then it doesn't make sense.
>
> Also strcmp() and similar should always be done as == 0, < 0 or != 0
> because that is the idiom:
>
>         if name != "foo" then
>
> becomes:
>
>         if (strcmpt(name, "foo") != 0) {
>
> The != from the first is shifted in the second.
>
> So I don't really think this approach is the right thing.  You have to
> read the code and understand the story it is telling.  Then change it
> if needed.
>

Thank you for the advice! I will look into it!

-- 
Regards,

Amitoj Kaur Chawla