e1000e: Assign true and false to bool type variable instead of 1 and 0

From: Michael Wang <[email protected]>

Use true and false instead of 1 and 0 when assign value to a bool type variable.

This patch is try to keep the style of driver, and according to David's
suggestion on patch "e1000e: Avoid wrong check on TX hang":

http://marc.info/?l=linux-netdev&m=132296931201839&w=2

v2: add more description

Signed-off-by Michael Wang <[email protected]>

---
 drivers/net/ethernet/intel/e1000e/netdev.c |   34
++++++++++++++--------------
 1 files changed, 17 insertions(+), 17 deletions(-) 

Quoted 3 lines · Michael Wang

From: Michael Wang <[email protected]>
Use true and false instead of 1 and 0 when assign value to a bool type variable.

Thanks Michael, I have added your patch to my queue of e1000e patches.

Quoted 5 lines · Jeff Kirsher

Quoted 3 lines · Michael Wang

From: Michael Wang <[email protected]>
Use true and false instead of 1 and 0 when assign value to a bool type variable.

Thanks Michael, I have added your patch to my queue of e1000e patches.

There are more of these uses in intel drivers.

Perhaps you could run this cocci/spatch
on drivers/net/ethernet/intel/...

$ cat bool.cocci
@@
bool b;
@@

(quoted diff) +1 -1

-b = 0;
+b = false;

@@
bool b;
@@

(quoted diff) +1 -1

-b = 1;
+b = true;

$ git ls-files drivers/net/ethernet/intel/ | grep "\.c$" | while read file ; do spatch -in_place -sp_file bool.cocci $file ; done

Quoted 27 lines · Joe Perches

Quoted 5 lines · Jeff Kirsher

Quoted 3 lines · Michael Wang

From: Michael Wang <[email protected]>
Use true and false instead of 1 and 0 when assign value to a bool type variable.

Thanks Michael, I have added your patch to my queue of e1000e patches.

There are more of these uses in intel drivers.

Perhaps you could run this cocci/spatch
on drivers/net/ethernet/intel/...

$ cat bool.cocci
@@
bool b;
@@

(quoted diff) +1 -1

-b = 0;
+b = false;

@@
bool b;
@@

(quoted diff) +1 -1

-b = 1;
+b = true;

Quoted 3 lines

$ git ls-files drivers/net/ethernet/intel/ | grep "\.c$" | while read file ; do spatch -in_place -sp_file bool.cocci $file ; done

Hi, Joe

I think there are lots of such cases in kernel, and I think it is a legacy issue with some story in it.

The reason I only change the e1000e is that the patch I send before will broken the style of e1000e, because it's using true and false, not 1 and 0.

I think this will be a huge work if we want to correct all these cases,

and I think the good way is to separate the work to small pieces and finish them slowly.

Quoted 29 lines · Michael Wang

Quoted 27 lines · Joe Perches

Quoted 5 lines · Jeff Kirsher

Quoted 3 lines · Michael Wang

From: Michael Wang <[email protected]>
Use true and false instead of 1 and 0 when assign value to a bool type variable.

Thanks Michael, I have added your patch to my queue of e1000e patches.

There are more of these uses in intel drivers.

Perhaps you could run this cocci/spatch
on drivers/net/ethernet/intel/...

$ cat bool.cocci
@@
bool b;
@@

(quoted diff) +1 -1

-b = 0;
+b = false;

@@
bool b;
@@

(quoted diff) +1 -1

-b = 1;
+b = true;

Quoted 8 lines

Quoted 3 lines

$ git ls-files drivers/net/ethernet/intel/ | grep "\.c$" | while read file ; do spatch -in_place -sp_file bool.cocci $file ; done

Hi, Joe

I think there are lots of such cases in kernel, and I think it is a legacy issue with some story in it.

Actually, there are relatively few bool = 0|1; uses.

Quoted 2 lines

The reason I only change the e1000e is that the patch I send before will broken the style of e1000e, because it's using true and false, not 1 and 0.

Scriptable fixes are easy.

Quoted 2 lines

I think this will be a huge work if we want to correct all these cases,

Perhaps you're thinking of int uses that could be
converted to bool? There are a lot of those.

This is the diffstat I get for drivers/net/ethernet for the current uses of bool foo = 0|1;

$ git diff --stat drivers/net
 drivers/net/ethernet/broadcom/tg3.c           |    8 ++++----
 drivers/net/ethernet/brocade/bna/bnad.c       |    4 ++--
 drivers/net/ethernet/dec/tulip/de4x5.c        |    4 ++--
 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 +++++-----
 drivers/net/ethernet/intel/e1000e/netdev.c    |   16 ++++++++--------
 drivers/net/ethernet/intel/ixgb/ixgb_main.c   |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |    4 ++--
 drivers/net/ethernet/sfc/falcon.c             |    2 +-
 drivers/net/ethernet/sfc/mtd.c                |    6 +++---
 drivers/net/ethernet/tile/tilepro.c           |    4 ++--
 drivers/net/ethernet/xilinx/xilinx_emaclite.c |    4 ++--
 12 files changed, 33 insertions(+), 33 deletions(-)

Quoted 2 lines

and I think the good way is to separate the work to small pieces and finish them slowly.

I think that's true for the int->bool conversions.

I'm not good at script, if you are interested, we can work together to fix all the remain cases, and send out some patches.

Quoted 2 lines · Michael Wang

I'm not good at script, if you are interested, we can work together to fix all the remain cases, and send out some patches.

You should try coccinelle and spatch.

If you run the script, this is the intel output.

 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 +++++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c   |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |    4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

drivers/net/ethernet/intel/e1000/e1000_main.c +5 -5

index 82f4ef1..5526881 100644
@@ -1185,7 +1185,7 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 		if (global_quad_port_a != 0)
 			adapter->eeprom_wol = 0;
 		else
-			adapter->quad_port_a = 1;
+			adapter->quad_port_a = true;
 		/* Reset for multiple quad port adapters */
 		if (++global_quad_port_a == 4)
 			global_quad_port_a = 0;

@@ -1679,7 +1679,7 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
 	 * need this to apply a workaround later in the send path. */
 	if (hw->mac_type == e1000_82544 &&
 	    hw->bus_type == e1000_bus_type_pcix)
-		adapter->pcix_82544 = 1;
+		adapter->pcix_82544 = true;
 
 	ew32(TCTL, tctl);
 

@@ -2002,7 +2002,7 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
 
 	tx_ring->next_to_use = 0;
 	tx_ring->next_to_clean = 0;
-	tx_ring->last_tx_tso = 0;
+	tx_ring->last_tx_tso = false;
 
 	writel(0, hw->hw_addr + tx_ring->tdh);
 	writel(0, hw->hw_addr + tx_ring->tdt);

@@ -2851,7 +2851,7 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 		 * DMA'd to the controller */
 		if (!skb->data_len && tx_ring->last_tx_tso &&
 		    !skb_is_gso(skb)) {
-			tx_ring->last_tx_tso = 0;
+			tx_ring->last_tx_tso = false;
 			size -= 4;
 		}
 

@@ -3219,7 +3219,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 
 	if (likely(tso)) {
 		if (likely(hw->mac_type != e1000_82544))
-			tx_ring->last_tx_tso = 1;
+			tx_ring->last_tx_tso = true;
 		tx_flags |= E1000_TX_FLAGS_TSO;
 	} else if (likely(e1000_tx_csum(adapter, tx_ring, skb)))
 		tx_flags |= E1000_TX_FLAGS_CSUM;

drivers/net/ethernet/intel/ixgb/ixgb_main.c +1 -1

index 247cf92..7fc6073 100644
@@ -228,7 +228,7 @@ ixgb_up(struct ixgb_adapter *adapter)
 	if (IXGB_READ_REG(&adapter->hw, STATUS) & IXGB_STATUS_PCIX_MODE) {
 		err = pci_enable_msi(adapter->pdev);
 		if (!err) {
-			adapter->have_msi = 1;
+			adapter->have_msi = true;
 			irq_flags = 0;
 		}
 		/* proceed to try to request regular interrupt */

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1

index 1b28ed9..e8da825 100644
@@ -4019,7 +4019,7 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 
 		/* Mark all the VFs as inactive */
 		for (i = 0 ; i < adapter->num_vfs; i++)
-			adapter->vfinfo[i].clear_to_send = 0;
+			adapter->vfinfo[i].clear_to_send = false;
 
 		/* ping all the active vfs to let them know we are going down */
 		ixgbe_ping_all_vfs(adapter);

drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c +2 -2

index 9a56fd7..2c2a341 100644
@@ -1646,9 +1646,9 @@ static bool ixgbe_get_i2c_data(u32 *i2cctl)
 	bool data;
 
 	if (*i2cctl & IXGBE_I2C_DATA_IN)
-		data = 1;
+		data = true;
 	else
-		data = 0;
+		data = false;
 
 	return data;
 }

On 12/07/2011 02:19 PM, Joe Perches wrote:

drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c static bool ixgbe_get_i2c_data(u32 *i2cctl) · line 1653

Quoted 8 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c +2 -2

 	if (*i2cctl & IXGBE_I2C_DATA_IN)
-		data = 1;
+		data = true;
 	else
-		data = 0;
+		data = false;
 
 	return data;

Quoted 1 line · Michael Wang

On 12/07/2011 02:19 PM, Joe Perches wrote:

[snip]

Quoted 8 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c static bool ixgbe_get_i2c_data(u32 *i2cctl) · line 1653

Quoted 8 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c +2 -2

 	if (*i2cctl & IXGBE_I2C_DATA_IN)
-		data = 1;
+		data = true;
 	else
-		data = 0;
+		data = false;
 
 	return data;

Yuck...

bool f(whatever)

{
	bool data;
	if (expression)
		data = true;
	else
		data = false;
	return data;
}

is amazingly unidiomatic. Hell, if nothing else it's

	bool data = expression;
	return data;
if not an outright
	return expression;
or
	return (bool)expression;

if you want to underline that it's treated as boolean... What tool has produced that patch?

Quoted 2 lines · Joe Perches

Quoted 1 line · Michael Wang

I'm not good at script, if you are interested, we can work together

to

Quoted 4 lines

Quoted 1 line

fix all the remain cases, and send out some patches.

You should try coccinelle and spatch.

Hi, Joe

I have not used coccinelle and spatch before, I will study on them later.

Quoted 7 lines

If you run the script, this is the intel output.

 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 +++++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c   |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |    4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

We can try to fix them, thanks for the suggestion.

Regards,
Michael Wang

Quoted 24 lines · Michael Wang

Quoted 2 lines · Joe Perches

Quoted 1 line · Michael Wang

I'm not good at script, if you are interested, we can work together

to

Quoted 4 lines

Quoted 1 line

fix all the remain cases, and send out some patches.

You should try coccinelle and spatch.

Hi, Joe

I have not used coccinelle and spatch before, I will study on them later.

Quoted 7 lines

If you run the script, this is the intel output.

 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 +++++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c   |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |    4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

Quoted 4 lines

We can try to fix them, thanks for the suggestion.

Regards,
Michael Wang

So should I expect a v3 of this patch?

Also I am fine with lumping all the changes for all our drivers in one patch as long as the changes for each driver are somewhat small. If the changes for a particular driver gets too extensive, please create a separate patch.

Cheers,
Jeff

Quoted 30 lines · Jeff Kirsher

Quoted 28 lines · Michael Wang

Quoted 2 lines · Joe Perches

Quoted 1 line · Michael Wang

I'm not good at script, if you are interested, we can work together

to

Quoted 4 lines

Quoted 1 line

fix all the remain cases, and send out some patches.

You should try coccinelle and spatch.

Hi, Joe

I have not used coccinelle and spatch before, I will study on them later.

Quoted 7 lines

If you run the script, this is the intel output.

 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 +++++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c   |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |    4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

We can try to fix them, thanks for the suggestion.

Regards,
Michael Wang

Quoted 2 lines

So should I expect a v3 of this patch?

Hi, Jeff

Please use the v2 first, because what Joe described will have a different meaning and edit different files.

Quoted 5 lines

Also I am fine with lumping all the changes for all our drivers in one patch as long as the changes for each driver are somewhat small. If the changes for a particular driver gets too extensive, please create a separate patch.

I think separate such patch according to files will be better.

Thanks,
Michael Wang

Quoted 3 lines

Cheers,
Jeff