2015-04-03 16:42:17

by Amitoj Kaur Chawla

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

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 <[email protected]>
---
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


2015-04-03 16:51:25

by Dan Carpenter

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

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

2015-04-03 17:01:16

by Joe Perches

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

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 !=

2015-04-03 17:03:21

by Amitoj Kaur Chawla

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

On Fri, Apr 3, 2015 at 10:21 PM, Dan Carpenter <[email protected]> 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

2015-04-03 21:05:19

by Dan Carpenter

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

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

2015-04-04 13:58:36

by Dan Carpenter

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

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

2015-04-04 15:52:26

by Joe Perches

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

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