[PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Michael Wang]

From: Michael Wang <wangyun@linux.vnet.ibm.com>

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 <wangyun@linux.vnet.ibm.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   34
++++++++++++++--------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
b/drivers/net/ethernet/intel/e1000e/netdev.c
index a855db1..2e5e423 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -875,7 +875,7 @@ static bool e1000_clean_rx_irq(struct e1000_adapter
*adapter,
 	u32 length, staterr;
 	unsigned int i;
 	int cleaned_count = 0;
-	bool cleaned = 0;
+	bool cleaned = false;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;

 	i = rx_ring->next_to_clean;
@@ -904,7 +904,7 @@ static bool e1000_clean_rx_irq(struct e1000_adapter
*adapter,

 		next_buffer = &rx_ring->buffer_info[i];

-		cleaned = 1;
+		cleaned = true;
 		cleaned_count++;
 		dma_unmap_single(&pdev->dev,
 				 buffer_info->dma,
@@ -1150,7 +1150,7 @@ static bool e1000_clean_tx_irq(struct
e1000_adapter *adapter)
 		 * Detect a transmit hang in hardware, this serializes the
 		 * check with the clearing of time_stamp and movement of i
 		 */
-		adapter->detect_tx_hung = 0;
+		adapter->detect_tx_hung = false;
 		if (tx_ring->buffer_info[i].time_stamp &&
 		    time_after(jiffies, tx_ring->buffer_info[i].time_stamp
 			       + (adapter->tx_timeout_factor * HZ)) &&
@@ -1185,7 +1185,7 @@ static bool e1000_clean_rx_irq_ps(struct
e1000_adapter *adapter,
 	unsigned int i, j;
 	u32 length, staterr;
 	int cleaned_count = 0;
-	bool cleaned = 0;
+	bool cleaned = false;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;

 	i = rx_ring->next_to_clean;
@@ -1211,7 +1211,7 @@ static bool e1000_clean_rx_irq_ps(struct
e1000_adapter *adapter,

 		next_buffer = &rx_ring->buffer_info[i];

-		cleaned = 1;
+		cleaned = true;
 		cleaned_count++;
 		dma_unmap_single(&pdev->dev, buffer_info->dma,
 				 adapter->rx_ps_bsize0, DMA_FROM_DEVICE);
@@ -1628,7 +1628,7 @@ static irqreturn_t e1000_intr_msi(int irq, void *data)
 	 */

 	if (icr & E1000_ICR_LSC) {
-		hw->mac.get_link_status = 1;
+		hw->mac.get_link_status = true;
 		/*
 		 * ICH8 workaround-- Call gig speed drop workaround on cable
 		 * disconnect (LSC) before accessing any PHY registers
@@ -1694,7 +1694,7 @@ static irqreturn_t e1000_intr(int irq, void *data)
 	 */

 	if (icr & E1000_ICR_LSC) {
-		hw->mac.get_link_status = 1;
+		hw->mac.get_link_status = true;
 		/*
 		 * ICH8 workaround-- Call gig speed drop workaround on cable
 		 * disconnect (LSC) before accessing any PHY registers
@@ -1751,7 +1751,7 @@ static irqreturn_t e1000_msix_other(int irq, void
*data)
 	if (icr & E1000_ICR_OTHER) {
 		if (!(icr & E1000_ICR_LSC))
 			goto no_link_interrupt;
-		hw->mac.get_link_status = 1;
+		hw->mac.get_link_status = true;
 		/* guard against interrupt when we're going down */
 		if (!test_bit(__E1000_DOWN, &adapter->state))
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
@@ -3327,7 +3327,7 @@ void e1000e_reset(struct e1000_adapter *adapter)
 		fc->pause_time = 0xFFFF;
 	else
 		fc->pause_time = E1000_FC_PAUSE_TIME;
-	fc->send_xon = 1;
+	fc->send_xon = true;
 	fc->current_mode = fc->requested_mode;

 	switch (hw->mac.type) {
@@ -4183,7 +4183,7 @@ static void e1000_print_link_info(struct
e1000_adapter *adapter)
 static bool e1000e_has_link(struct e1000_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	bool link_active = 0;
+	bool link_active = false;
 	s32 ret_val = 0;

 	/*
@@ -4198,7 +4198,7 @@ static bool e1000e_has_link(struct e1000_adapter
*adapter)
 			ret_val = hw->mac.ops.check_for_link(hw);
 			link_active = !hw->mac.get_link_status;
 		} else {
-			link_active = 1;
+			link_active = true;
 		}
 		break;
 	case e1000_media_type_fiber:
@@ -4297,7 +4297,7 @@ static void e1000_watchdog_task(struct work_struct
*work)

 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
-			bool txb2b = 1;
+			bool txb2b = true;

 			/* Cancel scheduled suspend requests. */
 			pm_runtime_resume(netdev->dev.parent);
@@ -4333,11 +4333,11 @@ static void e1000_watchdog_task(struct
work_struct *work)
 			adapter->tx_timeout_factor = 1;
 			switch (adapter->link_speed) {
 			case SPEED_10:
-				txb2b = 0;
+				txb2b = false;
 				adapter->tx_timeout_factor = 16;
 				break;
 			case SPEED_100:
-				txb2b = 0;
+				txb2b = false;
 				adapter->tx_timeout_factor = 10;
 				break;
 			}
@@ -4473,7 +4473,7 @@ link_up:
 	e1000e_flush_descriptors(adapter);

 	/* Force detection of hung controller every watchdog period */
-	adapter->detect_tx_hung = 1;
+	adapter->detect_tx_hung = true;

 	/*
 	 * With 82571 controllers, LAA may be overwritten due to controller
@@ -6134,8 +6134,8 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	INIT_WORK(&adapter->print_hang_task, e1000_print_hw_hang);

 	/* Initialize link parameters. User can change them with ethtool */
-	adapter->hw.mac.autoneg = 1;
-	adapter->fc_autoneg = 1;
+	adapter->hw.mac.autoneg = true;
+	adapter->fc_autoneg = true;
 	adapter->hw.fc.requested_mode = e1000_fc_default;
 	adapter->hw.fc.current_mode = e1000_fc_default;
 	adapter->hw.phy.autoneg_advertised = 0x2f;
-- 
1.7.4.1

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Tue, 2011-12-06 at 18:33 -0800, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> 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 <wangyun@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |   34
> ++++++++++++++--------------
>  1 files changed, 17 insertions(+), 17 deletions(-) 

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Joe Perches]

On Tue, 2011-12-06 at 19:43 -0800, Jeff Kirsher wrote:
> On Tue, 2011-12-06 at 18:33 -0800, Michael Wang wrote:
> > From: Michael Wang <wangyun@linux.vnet.ibm.com>
> > 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;
@@

-b = 0;
+b = false;

@@
bool b;
@@

-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

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Michael Wang]

On 12/07/2011 12:14 PM, Joe Perches wrote:

> On Tue, 2011-12-06 at 19:43 -0800, Jeff Kirsher wrote:
>> On Tue, 2011-12-06 at 18:33 -0800, Michael Wang wrote:
>>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>> 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;
> @@
> 
> -b = 0;
> +b = false;
> 
> @@
> bool b;
> @@
> 
> -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
> 
> 

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.

Thanks,
Michael Wang

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Joe Perches]

On Wed, 2011-12-07 at 12:49 +0800, Michael Wang wrote:
> On 12/07/2011 12:14 PM, Joe Perches wrote:
> 
> > On Tue, 2011-12-06 at 19:43 -0800, Jeff Kirsher wrote:
> >> On Tue, 2011-12-06 at 18:33 -0800, Michael Wang wrote:
> >>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> >>> 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;
> > @@
> > 
> > -b = 0;
> > +b = false;
> > 
> > @@
> > bool b;
> > @@
> > 
> > -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
> > 
> > 
> 
> 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.

> 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.

> 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(-)

> 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.

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Michael Wang]

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

> On Wed, 2011-12-07 at 12:49 +0800, Michael Wang wrote:
>> On 12/07/2011 12:14 PM, Joe Perches wrote:
>>
>>> On Tue, 2011-12-06 at 19:43 -0800, Jeff Kirsher wrote:
>>>> On Tue, 2011-12-06 at 18:33 -0800, Michael Wang wrote:
>>>>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>>>> 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;
>>> @@
>>>
>>> -b = 0;
>>> +b = false;
>>>
>>> @@
>>> bool b;
>>> @@
>>>
>>> -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
>>>
>>>
>>
>> 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.
> 
>> 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.

Hi, Joe

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.

Thanks,
Michael Wang

> 
>> 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(-)
> 
>> 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.
> 

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Joe Perches]

On Wed, 2011-12-07 at 14:08 +0800, Michael Wang wrote:
> 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(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 82f4ef1..5526881 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index 247cf92..7fc6073 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -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 */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1b28ed9..e8da825 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -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);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 9a56fd7..2c2a341 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -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;
 }



------------------------------------------------------------------------------
Cloud Services Checklist: Pricing and Packaging Optimization
This white paper is intended to serve as a reference, checklist and point of 
discussion for anyone considering optimizing the pricing and packaging model 
of a cloud services business. Read Now!
http://www.accelacomm.com/jaw/sfnl/114/51491232/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Michael Wang]

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

> On Wed, 2011-12-07 at 14:08 +0800, Michael Wang wrote:
>> 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.
> 

Hi, Joe

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

> 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

> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index 82f4ef1..5526881 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -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;
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> index 247cf92..7fc6073 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> @@ -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 */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1b28ed9..e8da825 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -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);
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> index 9a56fd7..2c2a341 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> @@ -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;
>  }
> 
> 

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Al Viro]

On Wed, Dec 07, 2011 at 02:36:20PM +0800, Michael Wang wrote:
> On 12/07/2011 02:19 PM, Joe Perches wrote:

[snip]
> >  	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?

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Joe Perches]

On Wed, 2011-12-07 at 07:20 +0000, Al Viro wrote:
> On Wed, Dec 07, 2011 at 02:36:20PM +0800, Michael Wang wrote:
> > On 12/07/2011 02:19 PM, Joe Perches wrote:
> 
> [snip]
> > >  	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?

tool has multiple meanings... :)

In this case though it's spatch (coccinelle)

I looked at it and thought the same thing as you.

The point though was not to do the idiomatic
transform that you correctly describe, but to
get Michael Wang to look at spatch as a tool
to do those transforms.

cheers, Joe

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

On Tue, 2011-12-06 at 22:36 -0800, Michael Wang wrote:
> On 12/07/2011 02:19 PM, Joe Perches wrote:
> 
> > On Wed, 2011-12-07 at 14:08 +0800, Michael Wang wrote:
> >> 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.
> > 
> 
> Hi, Joe
> 
> I have not used coccinelle and spatch before, I will study on them
> later.
> 
> > 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 

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Michael Wang]

On 12/07/2011 05:04 PM, Jeff Kirsher wrote:

> On Tue, 2011-12-06 at 22:36 -0800, Michael Wang wrote:
>> On 12/07/2011 02:19 PM, Joe Perches wrote:
>>
>>> On Wed, 2011-12-07 at 14:08 +0800, Michael Wang wrote:
>>>> 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.
>>>
>>
>> Hi, Joe
>>
>> I have not used coccinelle and spatch before, I will study on them
>> later.
>>
>>> 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 
> 
> 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.

> 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

> 
> Cheers,
> Jeff



------------------------------------------------------------------------------
Cloud Services Checklist: Pricing and Packaging Optimization
This white paper is intended to serve as a reference, checklist and point of 
discussion for anyone considering optimizing the pricing and packaging model 
of a cloud services business. Read Now!
http://www.accelacomm.com/jaw/sfnl/114/51491232/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH v2] e1000e: Assign true and false to bool type variable instead of 1 and 0

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Wed, 2011-12-07 at 01:15 -0800, Michael Wang wrote:
> On 12/07/2011 05:04 PM, Jeff Kirsher wrote:
> 
> > On Tue, 2011-12-06 at 22:36 -0800, Michael Wang wrote:
> >> On 12/07/2011 02:19 PM, Joe Perches wrote:
> >>
> >>> On Wed, 2011-12-07 at 14:08 +0800, Michael Wang wrote:
> >>>> 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.
> >>>
> >>
> >> Hi, Joe
> >>
> >> I have not used coccinelle and spatch before, I will study on them
> >> later.
> >>
> >>> 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 
> > 
> > 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.

Ok sounds fine to me.

> 
> > 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
> 
> > 
> > Cheers,
> > Jeff
> 
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]