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

[Richard Knutsson <ricknu-0@student.ltu.se>]

Dmitry Torokhov wrote:
> On 2/23/07, Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>> Milind Choudhary wrote:
>> > On 2/23/07, Richard Knutsson <ricknu-0@student.ltu.se> 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

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors