All of lore.kernel.org
 help / color / mirror / Atom feed
* [KJ] [PATCH] add new macro BIT_S to include/linux/kernel.h
@ 2005-08-13 11:25 Michael Veeck
  2005-08-13 20:26 ` Alexey Dobriyan
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michael Veeck @ 2005-08-13 11:25 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

description:
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 like it is already done in include/linux/input.h

patch is against latest git-tree from Linus

Signed-off-by: Michael Veeck <michael@veeck.de>


[-- Attachment #2: bitmacro_include-kernel.patch --]
[-- Type: text/plain, Size: 551 bytes --]

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -30,6 +30,11 @@ extern const char linux_banner[];
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 #define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))
 
+/*
+ * BIT macro that also handles a possible overflow
+ */
+#define BITS(x) (1UL << ((x) % BITS_PER_LONG))
+
 #define	KERN_EMERG	"<0>"	/* system is unusable			*/
 #define	KERN_ALERT	"<1>"	/* action must be taken immediately	*/
 #define	KERN_CRIT	"<2>"	/* critical conditions			*/

[-- Attachment #3: 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
                   ` (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

end of thread, other threads:[~2005-08-25 19:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

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.