* [patch 2/5] vfs: O_* bit numbers uniqueness check
@ 2010-05-24 19:24 akpm
0 siblings, 0 replies; 6+ messages in thread
From: akpm @ 2010-05-24 19:24 UTC (permalink / raw)
To: viro
Cc: linux-fsdevel, akpm, fengguang.wu, davem, eparis, hch, jamie,
rdreier, schwab, sfr
From: Wu Fengguang <fengguang.wu@intel.com>
The O_* bit numbers are defined in 20+ arch/*, and can silently overlap.
Add a compile time check to ensure the uniqueness as suggested by David
Miller.
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Eric Paris <eparis@redhat.com>
Cc: Roland Dreier <rdreier@cisco.com>
Cc: Jamie Lokier <jamie@shareable.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/fcntl.c | 15 +++++++++++++--
include/asm-generic/fcntl.h | 4 ++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff -puN fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check fs/fcntl.c
--- a/fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check
+++ a/fs/fcntl.c
@@ -762,11 +762,22 @@ void kill_fasync(struct fasync_struct **
}
EXPORT_SYMBOL(kill_fasync);
-static int __init fasync_init(void)
+static int __init fcntl_init(void)
{
+ /* please add new bits here to ensure allocation uniqueness */
+ BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ O_RDONLY | O_WRONLY | O_RDWR |
+ O_CREAT | O_EXCL | O_NOCTTY |
+ O_TRUNC | O_APPEND | O_NONBLOCK |
+ __O_SYNC | O_DSYNC | FASYNC |
+ O_DIRECT | O_LARGEFILE | O_DIRECTORY |
+ O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
+ FMODE_EXEC
+ ));
+
fasync_cache = kmem_cache_create("fasync_cache",
sizeof(struct fasync_struct), 0, SLAB_PANIC, NULL);
return 0;
}
-module_init(fasync_init)
+module_init(fcntl_init)
diff -puN include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check include/asm-generic/fcntl.h
--- a/include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check
+++ a/include/asm-generic/fcntl.h
@@ -11,6 +11,10 @@
* -Eric Paris
*/
+/*
+ * When introducing new O_* bits, please check its uniqueness in fcntl_init().
+ */
+
#define O_ACCMODE 00000003
#define O_RDONLY 00000000
#define O_WRONLY 00000001
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2/5] vfs: O_* bit numbers uniqueness check
@ 2010-07-20 22:29 akpm
2010-08-26 23:22 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: akpm @ 2010-07-20 22:29 UTC (permalink / raw)
To: viro
Cc: linux-fsdevel, akpm, fengguang.wu, davem, eparis, hch, jamie,
rdreier, schwab, sfr
From: Wu Fengguang <fengguang.wu@intel.com>
The O_* bit numbers are defined in 20+ arch/*, and can silently overlap.
Add a compile time check to ensure the uniqueness as suggested by David
Miller.
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Eric Paris <eparis@redhat.com>
Cc: Roland Dreier <rdreier@cisco.com>
Cc: Jamie Lokier <jamie@shareable.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/fcntl.c | 15 +++++++++++++--
include/asm-generic/fcntl.h | 4 ++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff -puN fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check fs/fcntl.c
--- a/fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check
+++ a/fs/fcntl.c
@@ -767,11 +767,22 @@ void kill_fasync(struct fasync_struct **
}
EXPORT_SYMBOL(kill_fasync);
-static int __init fasync_init(void)
+static int __init fcntl_init(void)
{
+ /* please add new bits here to ensure allocation uniqueness */
+ BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ O_RDONLY | O_WRONLY | O_RDWR |
+ O_CREAT | O_EXCL | O_NOCTTY |
+ O_TRUNC | O_APPEND | O_NONBLOCK |
+ __O_SYNC | O_DSYNC | FASYNC |
+ O_DIRECT | O_LARGEFILE | O_DIRECTORY |
+ O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
+ FMODE_EXEC
+ ));
+
fasync_cache = kmem_cache_create("fasync_cache",
sizeof(struct fasync_struct), 0, SLAB_PANIC, NULL);
return 0;
}
-module_init(fasync_init)
+module_init(fcntl_init)
diff -puN include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check include/asm-generic/fcntl.h
--- a/include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check
+++ a/include/asm-generic/fcntl.h
@@ -11,6 +11,10 @@
* -Eric Paris
*/
+/*
+ * When introducing new O_* bits, please check its uniqueness in fcntl_init().
+ */
+
#define O_ACCMODE 00000003
#define O_RDONLY 00000000
#define O_WRONLY 00000001
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
2010-07-20 22:29 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm
@ 2010-08-26 23:22 ` James Bottomley
2010-08-27 1:27 ` Jamie Lokier
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-08-26 23:22 UTC (permalink / raw)
To: akpm
Cc: viro, linux-fsdevel, fengguang.wu, davem, eparis, hch, jamie,
rdreier, schwab, sfr
On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> From: Wu Fengguang <fengguang.wu@intel.com>
>
> The O_* bit numbers are defined in 20+ arch/*, and can silently overlap.
> Add a compile time check to ensure the uniqueness as suggested by David
> Miller.
Can we get this reverted or fixed? It's causing the parisc compiles to
fail. The reason is O_NONBLOCK on parisc has a dual value:
#define O_NONBLOCK 000200004 /* HPUX has separate NDELAY & NONBLOCK */
The fix would be to take O_NONBLOCK out.
James
---
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 6769fd0..aeb02aa 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -770,10 +770,11 @@ EXPORT_SYMBOL(kill_fasync);
static int __init fcntl_init(void)
{
/* please add new bits here to ensure allocation uniqueness */
- BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ BUILD_BUG_ON(18 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
O_RDONLY | O_WRONLY | O_RDWR |
O_CREAT | O_EXCL | O_NOCTTY |
- O_TRUNC | O_APPEND | O_NONBLOCK |
+ /* remove O_NONBLOCK it's a two bit define on parisc */
+ O_TRUNC | O_APPEND | /* O_NONBLOCK | */
__O_SYNC | O_DSYNC | FASYNC |
O_DIRECT | O_LARGEFILE | O_DIRECTORY |
O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
2010-08-26 23:22 ` James Bottomley
@ 2010-08-27 1:27 ` Jamie Lokier
2010-08-27 5:53 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Jamie Lokier @ 2010-08-27 1:27 UTC (permalink / raw)
To: James Bottomley
Cc: akpm, viro, linux-fsdevel, fengguang.wu, davem, eparis, hch,
rdreier, schwab, sfr
James Bottomley wrote:
> On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> > From: Wu Fengguang <fengguang.wu@intel.com>
> >
> > The O_* bit numbers are defined in 20+ arch/*, and can silently overlap.
> > Add a compile time check to ensure the uniqueness as suggested by David
> > Miller.
>
> Can we get this reverted or fixed? It's causing the parisc compiles to
> fail. The reason is O_NONBLOCK on parisc has a dual value:
>
> #define O_NONBLOCK 000200004 /* HPUX has separate NDELAY & NONBLOCK */
>
> The fix would be to take O_NONBLOCK out.
A more thoroughly checking fix would be
BUILD_BUG_ON(18 - 1 /* For O_RDONLY being 0 */
+ HWEIGHT32(O_NONBLOCK) /* Because it's 2 bits on parisc */
!= HWEIGHT32( .... all the bits .... ));
Am I allowed to Sign-off handwavy pseudocode? ;-)
-- Jamie
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
2010-08-27 1:27 ` Jamie Lokier
@ 2010-08-27 5:53 ` James Bottomley
2010-09-04 1:22 ` [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test Wu Fengguang
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-08-27 5:53 UTC (permalink / raw)
To: Jamie Lokier
Cc: akpm, viro, linux-fsdevel, fengguang.wu, davem, eparis, hch,
rdreier, schwab, sfr
On Fri, 2010-08-27 at 02:27 +0100, Jamie Lokier wrote:
> James Bottomley wrote:
> > On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> > > From: Wu Fengguang <fengguang.wu@intel.com>
> > >
> > > The O_* bit numbers are defined in 20+ arch/*, and can silently overlap.
> > > Add a compile time check to ensure the uniqueness as suggested by David
> > > Miller.
> >
> > Can we get this reverted or fixed? It's causing the parisc compiles to
> > fail. The reason is O_NONBLOCK on parisc has a dual value:
> >
> > #define O_NONBLOCK 000200004 /* HPUX has separate NDELAY & NONBLOCK */
> >
> > The fix would be to take O_NONBLOCK out.
>
> A more thoroughly checking fix would be
>
> BUILD_BUG_ON(18 - 1 /* For O_RDONLY being 0 */
> + HWEIGHT32(O_NONBLOCK) /* Because it's 2 bits on parisc */
> != HWEIGHT32( .... all the bits .... ));
Well, it works, but is it wise? There are several missing flags in this
expression: O_NDELAY being the most noticeable one. It's defined to be
the sames as O_NONBLOCK on some platforms and not on others. Our
O_NONBLOCK problem is essentially the same class.
> Am I allowed to Sign-off handwavy pseudocode? ;-)
Nope ... you'd have to code it as a real patch ...
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test
2010-08-27 5:53 ` James Bottomley
@ 2010-09-04 1:22 ` Wu Fengguang
0 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2010-09-04 1:22 UTC (permalink / raw)
To: James Bottomley
Cc: Jamie Lokier, akpm@linux-foundation.org, viro@zeniv.linux.org.uk,
linux-fsdevel@vger.kernel.org, davem@davemloft.net,
eparis@redhat.com, hch@infradead.org, rdreier@cisco.com,
schwab@linux-m68k.org, sfr@canb.auug.org.au
I tend to agree with James' approach. How about this patch?
Compile tested on x86.
===
vfs: take O_NONBLOCK out of the O_* uniqueness test
From: James Bottomley <James.Bottomley@suse.de>
O_NONBLOCK on parisc has a dual value:
#define O_NONBLOCK 000200004 /* HPUX has separate NDELAY & NONBLOCK */
It is caught by the O_* bits uniqueness check and leads to a parisc
compile error. The fix would be to take O_NONBLOCK out.
Cc: Jamie Lokier <jamie@shareable.org>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
---
fs/fcntl.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- linux-next.orig/fs/fcntl.c 2010-08-20 06:57:11.000000000 +0800
+++ linux-next/fs/fcntl.c 2010-09-04 09:16:09.000000000 +0800
@@ -769,11 +769,15 @@ EXPORT_SYMBOL(kill_fasync);
static int __init fcntl_init(void)
{
- /* please add new bits here to ensure allocation uniqueness */
- BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ /*
+ * Please add new bits here to ensure allocation uniqueness.
+ * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
+ * is defined as O_NONBLOCK on some platforms and not on others.
+ */
+ BUILD_BUG_ON(18 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
O_RDONLY | O_WRONLY | O_RDWR |
O_CREAT | O_EXCL | O_NOCTTY |
- O_TRUNC | O_APPEND | O_NONBLOCK |
+ O_TRUNC | O_APPEND | /* O_NONBLOCK | */
__O_SYNC | O_DSYNC | FASYNC |
O_DIRECT | O_LARGEFILE | O_DIRECTORY |
O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-04 1:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-20 22:29 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm
2010-08-26 23:22 ` James Bottomley
2010-08-27 1:27 ` Jamie Lokier
2010-08-27 5:53 ` James Bottomley
2010-09-04 1:22 ` [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test Wu Fengguang
-- strict thread matches above, loose matches on Subject: below --
2010-05-24 19:24 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).