Re: [KJ][RFC][PATCH] BIT macro cleanup

[Richard Knutsson]

Dmitry Torokhov wrote:
> On 2/23/07, Richard Knutsson <ricknu-0@xxxxxxxxxxxxxx> wrote:
> > Milind Choudhary wrote:
> > > On 2/23/07, Richard Knutsson <ricknu-0@xxxxxxxxxxxxxx> wrote:
> > >> > +#define BITWRAP(nr)    (1UL << ((nr) % BITS_PER_LONG))
> > >> >
> > >> > & make the whole input subsystem use it
> > >> > The change is huge, more than 125 files using input.h
> > >> > & almost all use the BIT macro.
> > >> It is as a big of change, but have you dismissed the "BIT(nr %
> > >> BITS_PER_LONG)" approach?
> > >
> > > no..
> > > but just looking at the number of places it is being used,
> > > it seems that adding a new  macro would be good
> > > which makes it look short n sweet
> > You have a point there but I still don't think it should be in bitops.h.
> > Why should we favor long-wrap before byte-wrap, so what do you think
> > about doing:
> >
> > #define BITWRAP(x)      BIT((x) % BITS_PER_LONG)
> >
> > in input.h? Otherwise I think it should be call LBITWRAP (or something)
> > to both show what kind it is and enable us to add others later.
>
> Why would you not want to have what you call bitwrap as a standard
> behavior? Most placed to not use modulus because they know the kind of
> data they are working with but should still be fine if generic
> implementation did that.
Both because I find the name not as expressive as simple "BIT(x % 
something)", but mainly since it only enables wrapping of the long-type. 
But that is just my opinion.
Just to test:

> grep -Enr "BIT\(.*\%" *
include/asm-arm/arch-h720x/irqs.h:114:#define IRQ_TO_BIT(irq) (1 << ((irq - NR_GLBL_IRQS) % 32))
include/asm-arm/arch-omap/irqs.h:274:#define OMAP_IRQ_BIT(irq)  (1 << ((irq) % 32))
include/asm-i386/mach-visws/piix4.h:17:#define  GPIBIT(x)               (1 << ((x)%8))
include/linux/netfilter/xt_conntrack.h:11:#define XT_CONNTRACK_STATE_BIT(ctinfo) (1 << ((ctinfo)%IP_CT_IS_REPLY+1))
include/linux/netfilter/xt_state.h:4:#define XT_STATE_BIT(ctinfo) (1 << ((ctinfo)%IP_CT_IS_REPLY+1))
...

So it seems there are some instances who wrap but they don't seem to 
like BITSWAP (well, maybe those "% 32" on an appropriate arch).

So I think that if an subsystem uses something like this much (like 
input.h), then it is up to that subsystem to provide it. When more then 
one sub-system have a need for it, then it should be a common define (as 
BIT is now). But as I said, that is just my opinion.

Richard Knutsson

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/