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

[Michael Wang <wangyun@linux.vnet.ibm.com>]

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