All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>,
	jiri@resnulli.us, netdev@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] checkpatch: Add --strict preference for #defines using BIT(foo)
Date: Mon, 10 Nov 2014 15:53:31 -0800	[thread overview]
Message-ID: <1415663611.8868.25.camel@perches.com> (raw)
In-Reply-To: <20141110153647.f98f7d60fa24bf3bf7cbc215@linux-foundation.org>

On Mon, 2014-11-10 at 15:36 -0800, Andrew Morton wrote:
> On Fri, 07 Nov 2014 13:15:39 -0800 Joe Perches <joe@perches.com> wrote:
> 
> > Using BIT(foo) and BIT_ULL(bar) is more common now.
> > Suggest using these macros over #defines with 1<<value.
> 
> urgh.  I'm counting eightish implementations of BIT(), an unknown
> number of which are actually being used.  Many use 1<<n, some use
> 1UL<<N, another uses 1ULL<<n.  I'm a bit reluctant to recommend that
> anyone should use BIT() until it has has some vigorous scrubbing :(
>
> Is it actually an improvement?  If I see
> 
> #define X	(1U << 7)
> 
> then I know exactly what it does.  Whereas when I see
> 
> #define X	BIT(7)
> 
> I know neither the size or the signedness of X so I have to go look it
> up.

I'm not sure the signedness or type of X matters much
as the non-64bit comparisons are done by promotion to
at least int or unsigned int anyway.

The BIT macro makes sure a single bit is set.

The 'good' one is in bitops.h.  It also has #define BIT_ULL

include/linux/bitops.h:#define BIT(nr)                  (1UL << (nr))
include/linux/bitops.h-#define BIT_ULL(nr)              (1ULL << (nr))

The ones in tools/ are independent and should not be changed.
Excluding tools/, the others should probably be removed

$ git grep -E "define\s+BIT\b" 
arch/arm/mach-davinci/sleep.S:#define BIT(nr)                   (1 << (nr))
drivers/staging/rtl8188eu/include/rtl8188e_spec.h:#define BIT(x)                (1 << (x))
drivers/staging/rtl8188eu/include/wifi.h:#define BIT(x) (1 << (x))
drivers/staging/rtl8192e/rtl8192e/rtl_core.h:#define BIT(_i)                            (1<<(_i))
drivers/staging/rtl8712/osdep_service.h:        #define BIT(x)  (1 << (x))
drivers/staging/rtl8712/wifi.h:#define BIT(x)   (1 << (x))

> I have no strong feelings either way, but I'm wondering what might have
> inspired this change?

David Miller commented on a netdev patch where 1<<foo was
being used in a #define and suggested using BIT.

http://article.gmane.org/gmane.linux.network/337535/match=bit

Using BIT _is_ more common in recent patches too.



  reply	other threads:[~2014-11-10 23:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06  9:20 [patch net-next 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload Jiri Pirko
2014-11-06  9:20 ` [patch net-next 01/10] net: rename netdev_phys_port_id to more generic name Jiri Pirko
2014-11-06  9:20 ` [patch net-next 02/10] net: introduce generic switch devices support Jiri Pirko
2014-11-06  9:20 ` [patch net-next 03/10] rtnl: expose physical switch id for particular device Jiri Pirko
2014-11-06  9:20 ` [patch net-next 04/10] net-sysfs: " Jiri Pirko
2014-11-06  9:20 ` [patch net-next 05/10] rocker: introduce rocker switch driver Jiri Pirko
2014-11-07 20:08   ` David Miller
2014-11-06  9:20 ` [patch net-next 06/10] bridge: introduce fdb offloading via switchdev Jiri Pirko
2014-11-07 20:12   ` David Miller
2014-11-06  9:20 ` [patch net-next 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes Jiri Pirko
2014-11-06 16:59   ` Florian Fainelli
2014-11-06 18:57     ` Scott Feldman
     [not found]     ` <CAE4R7bBarGQPsuM5js20RwPLOBF24AtiiHM-f2EvkHNObp7NDQ@mail.gmail.com>
2014-11-07  8:32       ` Jiri Pirko
2014-11-06  9:20 ` [patch net-next 08/10] bridge: add API to notify bridge driver of learned FBD on offloaded device Jiri Pirko
2014-11-06  9:20 ` [patch net-next 09/10] rocker: implement rocker ofdpa flow table manipulation Jiri Pirko
2014-11-07 20:16   ` David Miller
2014-11-07 21:15     ` [PATCH] checkpatch: Add --strict preference for #defines using BIT(foo) Joe Perches
2014-11-09  9:50       ` Jiri Pirko
2014-11-09 14:22         ` Joe Perches
2014-11-10 23:36       ` Andrew Morton
2014-11-10 23:53         ` Joe Perches [this message]
2014-11-06  9:20 ` [patch net-next 10/10] rocker: implement L2 bridge offloading Jiri Pirko
2014-11-07 20:16   ` David Miller
2014-11-06 14:11 ` [patch net-next 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload Thomas Graf
2014-11-06 14:32   ` Jiri Pirko

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=1415663611.8868.25.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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.