2013-09-21 22:29:02

by Peter Senna Tschudin

Subject: [PATCH 12/19] wireless: Change variable type to bool

The variable continual is only assigned the values true and false.
Change its type to bool.

The simplified semantic patch that find this problem is as
follows (http://coccinelle.lip6.fr/):

@exists@
type T;
identifier b;
@@
- T
+ bool
b = ...;
... when any
b = \(true\|false\)

Signed-off-by: Peter Senna Tschudin <[email protected]>
---
drivers/net/wireless/rtlwifi/efuse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rtlwifi/efuse.c b/drivers/net/wireless/rtlwifi/efuse.c
index 838a1ed..63d370d 100644
--- a/drivers/net/wireless/rtlwifi/efuse.c
+++ b/drivers/net/wireless/rtlwifi/efuse.c
@@ -1203,7 +1203,7 @@ static void efuse_power_switch(struct ieee80211_hw *hw, u8 write, u8 pwrstate)

static u16 efuse_get_current_size(struct ieee80211_hw *hw)
{
- int continual = true;
+ bool continual = true;
u16 efuse_addr = 0;
u8 hworden;
u8 efuse_data, word_cnts;
--
1.8.3.1

2013-09-24 10:54:53

by Kalle Valo

Subject: Re: [PATCH 12/19] wireless: Change variable type to bool

Peter Senna Tschudin <[email protected]> writes:

> The variable continual is only assigned the values true and false.
> Change its type to bool.
>
> The simplified semantic patch that find this problem is as
> follows (http://coccinelle.lip6.fr/):
>
> @exists@
> type T;
> identifier b;
> @@
> - T
> + bool
> b = ...;
> ... when any
> b = \(true\|false\)
>
> Signed-off-by: Peter Senna Tschudin <[email protected]>
> ---
> drivers/net/wireless/rtlwifi/efuse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Please prefix the patch title with "rtlwifi:". We use "wireless:" for
changes to net/wireless.

--
Kalle Valo

2013-09-22 08:22:15

by Joe Perches

Subject: Re: [PATCH 12/19] wireless: Change variable type to bool

On Sun, 2013-09-22 at 00:27 +0200, Peter Senna Tschudin wrote:
> The variable continual is only assigned the values true and false.
> Change its type to bool.
[]
> diff --git a/drivers/net/wireless/rtlwifi/efuse.c b/drivers/net/wireless/rtlwifi/efuse.c
[]
> @@ -1203,7 +1203,7 @@ static void efuse_power_switch(struct ieee80211_hw *hw, u8 write, u8 pwrstate)
>
> static u16 efuse_get_current_size(struct ieee80211_hw *hw)
> {
> - int continual = true;
> + bool continual = true;
> u16 efuse_addr = 0;
> u8 hworden;
> u8 efuse_data, word_cnts;

Yes, this could use bool, but would probably be better
written without continual at all

as it is before this patch:

static u16 efuse_get_current_size(struct ieee80211_hw *hw)
{
int continual = true;
u16 efuse_addr = 0;
u8 hworden;
u8 efuse_data, word_cnts;

while (continual && efuse_one_byte_read(hw, efuse_addr, &efuse_data)
&& (efuse_addr < EFUSE_MAX_SIZE)) {
if (efuse_data != 0xFF) {
hworden = efuse_data & 0x0F;
word_cnts = efuse_calculate_word_cnts(hworden);
efuse_addr = efuse_addr + (word_cnts * 2) + 1;
} else {
continual = false;
}
}

return efuse_addr;
}

I think writing it without continual, which is effectively
an ersatz "break", would be better

Something like:

static u16 efuse_get_current_size(struct ieee80211_hw *hw)
{
u16 efuse_addr = 0;
u8 hworden;
u8 efuse_data, word_cnts;

while (efuse_one_byte_read(hw, efuse_addr, &efuse_data) &&
(efuse_addr < EFUSE_MAX_SIZE) {
if (efuse_data == 0xff)
break;
hworden = efuse_data & 0x0F;
word_cnts = efuse_calculate_word_cnts(hworden);
efuse_addr = efuse_addr + (word_cnts * 2) + 1;
}

return efuse_addr;
}