All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <chengang@emindsoft.com.cn>
To: Joe Perches <joe@perches.com>,
	akpm@linux-foundation.org, trivial@kernel.org
Cc: kuleshovmail@gmail.com, tony.luck@intel.com,
	tangchen@cn.fujitsu.com, will.deacon@arm.com, holt@sgi.com,
	ard.biesheuvel@linaro.org, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org,
	Chen Gang <gang.chen.5i5j@gmail.com>
Subject: Re: [PATCH trivial] include/linux/memblock.h: Clean up code for several trivial details
Date: Tue, 31 May 2016 10:06:13 +0800	[thread overview]
Message-ID: <574CF195.60803@emindsoft.com.cn> (raw)
In-Reply-To: <1464618832.14627.23.camel@perches.com>



On 2016年05月30日 22:33, Joe Perches wrote:
> On Mon, 2016-05-30 at 22:21 +0800, Chen Gang wrote:
>> No, they are not necessary. But for me, it will be more clearer, since
>> in our kernel (at least in include/linux/), almost all Boolean functions
>> use Boolean value or expression for return (and "!!" are often used).
> 
> Opinions vary.
> 
> There seem to be fewer than 20 !! uses in bool return
> functions in include/linux/
>

  ide.h:854
  mlx4/driver.h:76
  mlx5/device.h:744
  mmzone.h:793
  mtd/mtd.h:412
  nilfs2_fs.h:568
  nilfs2_fs.h:602
  nilfs2_fs.h:667
  nilfs2_fs.h:771
  page-flags.h:711
  pagemap.h:54

For me, we can change the related functions to Boolean functions
directly.
 
> Finding the quantity of bool conversions in include/linux
> from something other than 0, 1, true, or false to 0 or 1
> is not trivial, but it is non-zero and seems rather more
> than 20.
> 

[root@localhost linux]# grep -rn "\<return\>" * | grep ' & ' | grep -v "==" | grep -v '||' | grep -v 'struct' | grep -v '!=' | grep -v ',' | grep -v '?' | grep -v '!' | grep -v '&&' | wc -l
259

After give a glance, more than 60% are not for bool functions (so I
guess about 100 area hint).

But I guess, quite a few of none Boolean functions above can be changed
to Boolean functions, too (but have to read more code).

So I guess, about 200+ matches the hint (not use "!!" for & operation in
Boolean functions) in the 1000+ Boolean functions in include/linux.


All together, for me:

The return statement is much special than normal statements:

 - It is related with function's type (non-return function, Boolean
   function, or normal function), not only related with type cast within
   the statement itself.

 - Even for normal function, the type cast in return statement is also
   better: when reading source code, return statements have much more
   chances to be read than the function return type.

 - For finding Boolean function in existing normal functions, we often
   read the return value to know about whether it is a Boolean function
   or not.

So, I still suggest to add type cast explicitly in return statement.

Welcome any ideas, suggestions, and completions.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

      parent reply	other threads:[~2016-05-31  2:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-29 14:36 [PATCH trivial] include/linux/memblock.h: Clean up code for several trivial details chengang
2016-05-29 15:08 ` Joe Perches
2016-05-30 14:21   ` Chen Gang
2016-05-30 14:33     ` Joe Perches
2016-05-30 22:14       ` Chen Gang
2016-05-31  2:06       ` Chen Gang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=574CF195.60803@emindsoft.com.cn \
    --to=chengang@emindsoft.com.cn \
    --cc=akpm@linux-foundation.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=gang.chen.5i5j@gmail.com \
    --cc=holt@sgi.com \
    --cc=joe@perches.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kuleshovmail@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tangchen@cn.fujitsu.com \
    --cc=tony.luck@intel.com \
    --cc=trivial@kernel.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.