* Re: [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h
2005-08-13 11:25 [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h Michael Veeck
@ 2005-08-13 20:26 ` Alexey Dobriyan
2005-08-13 23:11 ` Michael Veeck
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Alexey Dobriyan @ 2005-08-13 20:26 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 547 bytes --]
On Sat, Aug 13, 2005 at 01:25:15PM +0200, Michael Veeck wrote:
> adds a new macro for doing a bitshift. right now every subsystem defines
> its own macro (which all look the same) but this one also does overflow
> checking
Checking?
$ cat -n test.c
1 #define BIT(x) (1ULL << (x))
2 #define BITS(x) (1ULL << ((x) % 64))
3
4 int main(void)
5 {
6 BIT(65);
7 BITS(65);
8 return 0;
9 }
$ gcc -o test test.c
test.c: In function `main':
test.c:6: warning: left shift count >= width of type
<=== ???
[-- Attachment #2: Type: text/plain, Size: 168 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h
2005-08-13 11:25 [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h Michael Veeck
2005-08-13 20:26 ` Alexey Dobriyan
@ 2005-08-13 23:11 ` Michael Veeck
2005-08-25 8:46 ` Domen Puncer
2005-08-25 19:37 ` Michael Veeck
3 siblings, 0 replies; 5+ messages in thread
From: Michael Veeck @ 2005-08-13 23:11 UTC (permalink / raw)
To: kernel-janitors
Alexey Dobriyan wrote:
> On Sat, Aug 13, 2005 at 01:25:15PM +0200, Michael Veeck wrote:
>
>>adds a new macro for doing a bitshift. right now every subsystem defines
>>its own macro (which all look the same) but this one also does overflow
>>checking
>
>
> Checking?
>
> $ cat -n test.c
> 1 #define BIT(x) (1ULL << (x))
> 2 #define BITS(x) (1ULL << ((x) % 64))
> 3
> 4 int main(void)
> 5 {
> 6 BIT(65);
> 7 BITS(65);
> 8 return 0;
> 9 }
> $ gcc -o test test.c
> test.c: In function `main':
> test.c:6: warning: left shift count >= width of type
> <== ???
>
okay, good point, but does that work even when the parameter of BIT isnt
known at compile time?
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h
2005-08-13 11:25 [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h Michael Veeck
2005-08-13 20:26 ` Alexey Dobriyan
2005-08-13 23:11 ` Michael Veeck
@ 2005-08-25 8:46 ` Domen Puncer
2005-08-25 19:37 ` Michael Veeck
3 siblings, 0 replies; 5+ messages in thread
From: Domen Puncer @ 2005-08-25 8:46 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 978 bytes --]
On 14/08/05 01:11 +0200, Michael Veeck wrote:
> Alexey Dobriyan wrote:
> >On Sat, Aug 13, 2005 at 01:25:15PM +0200, Michael Veeck wrote:
> >
> >>adds a new macro for doing a bitshift. right now every subsystem defines
> >>its own macro (which all look the same) but this one also does overflow
> >>checking
> >
> >
> >Checking?
> >
> >$ cat -n test.c
> > 1 #define BIT(x) (1ULL << (x))
> > 2 #define BITS(x) (1ULL << ((x) % 64))
> > 3
> > 4 int main(void)
> > 5 {
> > 6 BIT(65);
> > 7 BITS(65);
> > 8 return 0;
> > 9 }
> >$ gcc -o test test.c
> >test.c: In function `main':
> >test.c:6: warning: left shift count >= width of type
> > <=== ???
> >
>
> okay, good point, but does that work even when the parameter of BIT isnt
> known at compile time?
How about something like:
#define BITS(x) (1ULL << (__builtin_constant_p(x) ? (x) : (x) % 64)
Or maybe a WARN_ON would be nice, since (x >= 64) probably means there's
a bug.
Domen
[-- Attachment #2: Type: text/plain, Size: 168 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h
2005-08-13 11:25 [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h Michael Veeck
` (2 preceding siblings ...)
2005-08-25 8:46 ` Domen Puncer
@ 2005-08-25 19:37 ` Michael Veeck
3 siblings, 0 replies; 5+ messages in thread
From: Michael Veeck @ 2005-08-25 19:37 UTC (permalink / raw)
To: kernel-janitors
Domen Puncer wrote:
> On 14/08/05 01:11 +0200, Michael Veeck wrote:
>
>>Alexey Dobriyan wrote:
>>
>>>On Sat, Aug 13, 2005 at 01:25:15PM +0200, Michael Veeck wrote:
>>>
>>>
>>>>adds a new macro for doing a bitshift. right now every subsystem defines
>>>>its own macro (which all look the same) but this one also does overflow
>>>>checking
>>>
>>>
>>>Checking?
>>>
>>>$ cat -n test.c
>>> 1 #define BIT(x) (1ULL << (x))
>>> 2 #define BITS(x) (1ULL << ((x) % 64))
>>> 3
>>> 4 int main(void)
>>> 5 {
>>> 6 BIT(65);
>>> 7 BITS(65);
>>> 8 return 0;
>>> 9 }
>>>$ gcc -o test test.c
>>>test.c: In function `main':
>>>test.c:6: warning: left shift count >= width of type
>>> <== ???
>>>
>>
>>okay, good point, but does that work even when the parameter of BIT isnt
>>known at compile time?
>
>
> How about something like:
> #define BITS(x) (1ULL << (__builtin_constant_p(x) ? (x) : (x) % 64)
>
> Or maybe a WARN_ON would be nice, since (x >= 64) probably means there's
> a bug.
>
>
> Domen
I would put your warning around BIT. When it is used wrongly (x to high)
then it really seems to be a bug. Since most definitions use this, it
maybe unravels one or two bugs.
BIT_S does not check, that might be misleading word, but rather do a
wrap. Everybody who will use BIT_S should know that. Maybe BIT_WRAP
would be a better word for the macro then.
Veeck
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread