[PATCH] include/linux/msdos_fs.h: Cleanup include/linux/msdos_fs.h for userspace.

[PATCH] include/linux/msdos_fs.h: Cleanup include/linux/msdos_fs.h for userspace.

[S.Çağlar Onur]

include/linux/msdos_fs.h header should use __u## rather than u## types else any userspace code (like syslinux) includes that header fails to compile like following;

[...]
gcc -Wp,-MT,bootsect_bin.o,-MMD,.bootsect_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o bootsect_bin.o ../bootsect_bin.c
gcc -Wp,-MT,ldlinux_bin.o,-MMD,.ldlinux_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o ldlinux_bin.o ../ldlinux_bin.c
In file included from syslinux.c:52:
/usr/include/linux/msdos_fs.h:61: error: expected ')' before 'media'
[...]

Noticed by Onur Küçük <onur@pardus.org.tr>

Cc: Frank Seidel <fseidel@suse.de>
CC: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
CC: Onur Küçük <onur@pardus.org.tr>

Signed-off-by: S.Çağlar Onur <caglar@pardus.org.tr>

 include/linux/msdos_fs.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/msdos_fs.h b/include/linux/msdos_fs.h
index b03b274..3825b1c 100644
--- a/include/linux/msdos_fs.h
+++ b/include/linux/msdos_fs.h
@@ -58,7 +58,7 @@
 #define MSDOS_DOTDOT	"..         "	/* "..", padded to MSDOS_NAME chars */
 
 /* media of boot sector */
-static inline int fat_valid_media(u8 media)
+static inline int fat_valid_media(__u8 media)
 {
 	return 0xf8 <= media || media == 0xf0;
 }

Cheers
-- 
S.Çağlar Onur <caglar@pardus.org.tr>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

Re: [PATCH] include/linux/msdos_fs.h: Cleanup include/linux/msdos_fs.h for userspace.

[OGAWA Hirofumi]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=iso-2022-jp-2, Size: 1606 bytes --]

"S.^[$(D*.^[(Ba^[$(D+;^[(Blar Onur" <caglar@pardus.org.tr> writes:

> include/linux/msdos_fs.h header should use __u## rather than u## types else any userspace code (like syslinux) includes that header fails to compile like following;
>
> [...]
> gcc -Wp,-MT,bootsect_bin.o,-MMD,.bootsect_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o bootsect_bin.o ../bootsect_bin.c
> gcc -Wp,-MT,ldlinux_bin.o,-MMD,.ldlinux_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o ldlinux_bin.o ../ldlinux_bin.c
> In file included from syslinux.c:52:
> /usr/include/linux/msdos_fs.h:61: error: expected ')' before 'media'
> [...]
>
> Noticed by Onur K^[$(D+d+.+d^[(Bk <onur@pardus.org.tr>
>
> Cc: Frank Seidel <fseidel@suse.de>
> CC: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> CC: Onur K^[$(D+d+.+d^[(Bk <onur@pardus.org.tr>
>
> Signed-off-by: S.^[$(D*.^[(Ba^[$(D+;^[(Blar Onur <caglar@pardus.org.tr>

Looks good to me. Thanks.

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

>  include/linux/msdos_fs.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/msdos_fs.h b/include/linux/msdos_fs.h
> index b03b274..3825b1c 100644
> --- a/include/linux/msdos_fs.h
> +++ b/include/linux/msdos_fs.h
> @@ -58,7 +58,7 @@
>  #define MSDOS_DOTDOT	"..         "	/* "..", padded to MSDOS_NAME chars */
>  
>  /* media of boot sector */
> -static inline int fat_valid_media(u8 media)
> +static inline int fat_valid_media(__u8 media)
>  {
>  	return 0xf8 <= media || media == 0xf0;
>  }
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[2.6.26 patch] fat_valid_media() isn't for userspace

[Adrian Bunk]

On Sun, Jun 08, 2008 at 09:16:48PM +0300, S.Çağlar Onur wrote:
> include/linux/msdos_fs.h header should use __u## rather than u## types else any userspace code (like syslinux) includes that header fails to compile like following;
> 
> [...]
> gcc -Wp,-MT,bootsect_bin.o,-MMD,.bootsect_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o bootsect_bin.o ../bootsect_bin.c
> gcc -Wp,-MT,ldlinux_bin.o,-MMD,.ldlinux_bin.o.d -W -Wall -D_FILE_OFFSET_BITS=64 -g -Os -I. -I.. -I../libinstaller -c -o ldlinux_bin.o ../ldlinux_bin.c
> In file included from syslinux.c:52:
> /usr/include/linux/msdos_fs.h:61: error: expected ')' before 'media'
> [...]
> 
> Noticed by Onur Küçük <onur@pardus.org.tr>
> 
> Cc: Frank Seidel <fseidel@suse.de>
> CC: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> CC: Onur Küçük <onur@pardus.org.tr>
> 
> Signed-off-by: S.Çağlar Onur <caglar@pardus.org.tr>
> 
>  include/linux/msdos_fs.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/msdos_fs.h b/include/linux/msdos_fs.h
> index b03b274..3825b1c 100644
> --- a/include/linux/msdos_fs.h
> +++ b/include/linux/msdos_fs.h
> @@ -58,7 +58,7 @@
>  #define MSDOS_DOTDOT	"..         "	/* "..", padded to MSDOS_NAME chars */
>  
>  /* media of boot sector */
> -static inline int fat_valid_media(u8 media)
> +static inline int fat_valid_media(__u8 media)
>  {
>  	return 0xf8 <= media || media == 0xf0;
>  }

This function isn't part of the kernel<->userspace API and therefore 
shouldn't have been added at this place.

I'd suggest the patch below instead.

> Cheers

cu
Adrian


<--  snip  -->


Commit 73f20e58b1d586e9f6d3ddc3aad872829aca7743
(FAT_VALID_MEDIA(): remove pointless test)
wrongly added the new fat_valid_media() function
to the userspace-visible part of include/linux/msdos_fs.h

Move it to the part of include/linux/msdos_fs.h that is not exported to 
userspace.

Reported-by: Onur Küçük <onur@pardus.org.tr>
Reported-by: S.Çağlar Onur <caglar@pardus.org.tr>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 include/linux/msdos_fs.h |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

4a56896ef4ef839cdc9d59fd7118c5051231c61e diff --git a/include/linux/msdos_fs.h b/include/linux/msdos_fs.h
index b03b274..81cd36b 100644
--- a/include/linux/msdos_fs.h
+++ b/include/linux/msdos_fs.h
@@ -57,12 +57,6 @@
 #define MSDOS_DOT	".          "	/* ".", padded to MSDOS_NAME chars */
 #define MSDOS_DOTDOT	"..         "	/* "..", padded to MSDOS_NAME chars */
 
-/* media of boot sector */
-static inline int fat_valid_media(u8 media)
-{
-	return 0xf8 <= media || media == 0xf0;
-}
-
 #define FAT_FIRST_ENT(s, x)	((MSDOS_SB(s)->fat_bits == 32 ? 0x0FFFFF00 : \
 	MSDOS_SB(s)->fat_bits == 16 ? 0xFF00 : 0xF00) | (x))
 
@@ -334,6 +328,12 @@ static inline void fatwchar_to16(__u8 *dst, const wchar_t *src, size_t len)
 #endif
 }
 
+/* media of boot sector */
+static inline int fat_valid_media(u8 media)
+{
+	return 0xf8 <= media || media == 0xf0;
+}
+
 /* fat/cache.c */
 extern void fat_cache_inval_inode(struct inode *inode);
 extern int fat_get_cluster(struct inode *inode, int cluster,

Re: [2.6.26 patch] fat_valid_media() isn't for userspace

[OGAWA Hirofumi]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=iso-2022-jp-2, Size: 2025 bytes --]

Adrian Bunk <bunk@kernel.org> writes:

> This function isn't part of the kernel<->userspace API and therefore 
> shouldn't have been added at this place.
>
> I'd suggest the patch below instead.

Yes. This is new one, so it shouldn't have any users of that.
Looks more better. Thanks.

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

> Commit 73f20e58b1d586e9f6d3ddc3aad872829aca7743
> (FAT_VALID_MEDIA(): remove pointless test)
> wrongly added the new fat_valid_media() function
> to the userspace-visible part of include/linux/msdos_fs.h
>
> Move it to the part of include/linux/msdos_fs.h that is not exported to 
> userspace.
>
> Reported-by: Onur K^[$(D+d+.+d^[(Bk <onur@pardus.org.tr>
> Reported-by: S.^[$(D*.^[(Ba^[$(D+;^[(Blar Onur <caglar@pardus.org.tr>
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
>
> ---
>
>  include/linux/msdos_fs.h |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> 4a56896ef4ef839cdc9d59fd7118c5051231c61e diff --git a/include/linux/msdos_fs.h b/include/linux/msdos_fs.h
> index b03b274..81cd36b 100644
> --- a/include/linux/msdos_fs.h
> +++ b/include/linux/msdos_fs.h
> @@ -57,12 +57,6 @@
>  #define MSDOS_DOT	".          "	/* ".", padded to MSDOS_NAME chars */
>  #define MSDOS_DOTDOT	"..         "	/* "..", padded to MSDOS_NAME chars */
>  
> -/* media of boot sector */
> -static inline int fat_valid_media(u8 media)
> -{
> -	return 0xf8 <= media || media == 0xf0;
> -}
> -
>  #define FAT_FIRST_ENT(s, x)	((MSDOS_SB(s)->fat_bits == 32 ? 0x0FFFFF00 : \
>  	MSDOS_SB(s)->fat_bits == 16 ? 0xFF00 : 0xF00) | (x))
>  
> @@ -334,6 +328,12 @@ static inline void fatwchar_to16(__u8 *dst, const wchar_t *src, size_t len)
>  #endif
>  }
>  
> +/* media of boot sector */
> +static inline int fat_valid_media(u8 media)
> +{
> +	return 0xf8 <= media || media == 0xf0;
> +}
> +
>  /* fat/cache.c */
>  extern void fat_cache_inval_inode(struct inode *inode);
>  extern int fat_get_cluster(struct inode *inode, int cluster,
>

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [2.6.26 patch] fat_valid_media() isn't for userspace

[H. Peter Anvin]

OGAWA Hirofumi wrote:
> 
> Yes. This is new one, so it shouldn't have any users of that.
> Looks more better. Thanks.
> 

The other thing about this header that needs to be fixed is the 
definition of the following ioctls:

#define VFAT_IOCTL_READDIR_BOTH         _IOR('r', 1, struct dirent [2])
#define VFAT_IOCTL_READDIR_SHORT        _IOR('r', 2, struct dirent [2])

"struct dirent" is a kernel type here, but is a **different type** in 
userspace!  This means both the structure and the IOCTL number is wrong!

We need to make this either __kernel_dirent_t[*] or something like 
struct __msdos_fs_dirent.

	-hpa

[*] Yes, typedefs suck, but unfortunately C doesn't allow aliases in the 
structure tag namespace.

Re: [2.6.26 patch] fat_valid_media() isn't for userspace

[OGAWA Hirofumi]

"H. Peter Anvin" <hpa@kernel.org> writes:

> The other thing about this header that needs to be fixed is the 
> definition of the following ioctls:
>
> #define VFAT_IOCTL_READDIR_BOTH         _IOR('r', 1, struct dirent [2])
> #define VFAT_IOCTL_READDIR_SHORT        _IOR('r', 2, struct dirent [2])
>
> "struct dirent" is a kernel type here, but is a **different type** in 
> userspace!  This means both the structure and the IOCTL number is wrong!
>
> We need to make this either __kernel_dirent_t[*] or something like 
> struct __msdos_fs_dirent.

I see. "struct dirent" in linux/dirent.h has very few users in kernel,
and probably userland doesn't use it, so it seems it should be renamed.

Well, the patch is like this (sorry, other cleanup is in this patch)?
BTW, does typedef help it in this case?

Thanks.

> 	-hpa
>
> [*] Yes, typedefs suck, but unfortunately C doesn't allow aliases in the 
> structure tag namespace.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>



Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 include/linux/msdos_fs.h |   47 ++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff -puN include/linux/msdos_fs.h~fat_msdos_fs_h-cleanup include/linux/msdos_fs.h
--- linux-2.6/include/linux/msdos_fs.h~fat_msdos_fs_h-cleanup	2008-06-09 12:59:42.000000000 +0900
+++ linux-2.6-hirofumi/include/linux/msdos_fs.h	2008-06-09 13:06:21.000000000 +0900
@@ -2,11 +2,11 @@
 #define _LINUX_MSDOS_FS_H
 
 #include <linux/magic.h>
+#include <asm/byteorder.h>
 
 /*
  * The MS-DOS filesystem constants/structures
  */
-#include <asm/byteorder.h>
 
 #define SECTOR_SIZE	512		/* sector size (bytes) */
 #define SECTOR_BITS	9		/* log2(SECTOR_SIZE) */
@@ -92,21 +92,19 @@
 /*
  * ioctl commands
  */
-#define VFAT_IOCTL_READDIR_BOTH		_IOR('r', 1, struct dirent [2])
-#define VFAT_IOCTL_READDIR_SHORT	_IOR('r', 2, struct dirent [2])
+/* Copy of struct dirent in <linux/dirent.h> for userland. */
+struct __fat_fs_dirent {
+	long		d_ino;
+	__kernel_off_t	d_off;
+	unsigned short	d_reclen;
+	char		d_name[256]; /* We must not include limits.h! */
+};
+#define VFAT_IOCTL_READDIR_BOTH		_IOR('r', 1, struct __fat_fs_dirent[2])
+#define VFAT_IOCTL_READDIR_SHORT	_IOR('r', 2, struct __fat_fs_dirent[2])
 /* <linux/videotext.h> has used 0x72 ('r') in collision, so skip a few */
 #define FAT_IOCTL_GET_ATTRIBUTES	_IOR('r', 0x10, __u32)
 #define FAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u32)
 
-/*
- * vfat shortname flags
- */
-#define VFAT_SFN_DISPLAY_LOWER	0x0001 /* convert to lowercase for display */
-#define VFAT_SFN_DISPLAY_WIN95	0x0002 /* emulate win95 rule for display */
-#define VFAT_SFN_DISPLAY_WINNT	0x0004 /* emulate winnt rule for display */
-#define VFAT_SFN_CREATE_WIN95	0x0100 /* emulate win95 rule for create */
-#define VFAT_SFN_CREATE_WINNT	0x0200 /* emulate winnt rule for create */
-
 struct fat_boot_sector {
 	__u8	ignored[3];	/* Boot strap short or near jump */
 	__u8	system_id[8];	/* Name - can be used to special case
@@ -168,14 +166,6 @@ struct msdos_dir_slot {
 	__u8    name11_12[4];	/* last 2 characters in name */
 };
 
-struct fat_slot_info {
-	loff_t i_pos;		/* on-disk position of directory entry */
-	loff_t slot_off;	/* offset for slot or de start */
-	int nr_slots;		/* number of slots + 1(de) in filename */
-	struct msdos_dir_entry *de;
-	struct buffer_head *bh;
-};
-
 #ifdef __KERNEL__
 
 #include <linux/buffer_head.h>
@@ -184,6 +174,15 @@ struct fat_slot_info {
 #include <linux/fs.h>
 #include <linux/mutex.h>
 
+/*
+ * vfat shortname flags
+ */
+#define VFAT_SFN_DISPLAY_LOWER	0x0001 /* convert to lowercase for display */
+#define VFAT_SFN_DISPLAY_WIN95	0x0002 /* emulate win95 rule for display */
+#define VFAT_SFN_DISPLAY_WINNT	0x0004 /* emulate winnt rule for display */
+#define VFAT_SFN_CREATE_WIN95	0x0100 /* emulate win95 rule for create */
+#define VFAT_SFN_CREATE_WINNT	0x0200 /* emulate winnt rule for create */
+
 struct fat_mount_options {
 	uid_t fs_uid;
 	gid_t fs_gid;
@@ -267,6 +266,14 @@ struct msdos_inode_info {
 	struct inode vfs_inode;
 };
 
+struct fat_slot_info {
+	loff_t i_pos;		/* on-disk position of directory entry */
+	loff_t slot_off;	/* offset for slot or de start */
+	int nr_slots;		/* number of slots + 1(de) in filename */
+	struct msdos_dir_entry *de;
+	struct buffer_head *bh;
+};
+
 static inline struct msdos_sb_info *MSDOS_SB(struct super_block *sb)
 {
 	return sb->s_fs_info;
_

Re: [2.6.26 patch] fat_valid_media() isn't for userspace

[H. Peter Anvin]

OGAWA Hirofumi wrote:
>>
>> We need to make this either __kernel_dirent_t[*] or something like 
>> struct __msdos_fs_dirent.
> 
> I see. "struct dirent" in linux/dirent.h has very few users in kernel,
> and probably userland doesn't use it, so it seems it should be renamed.
> 
> Well, the patch is like this (sorry, other cleanup is in this patch)?
> BTW, does typedef help it in this case?
> 

If we're using a FAT-specific structure, then no, there is no reason to 
use a typedef.

	-hpa

Re: [2.6.26 patch] fat_valid_media() isn't for userspace

[Adrian Bunk]

On Mon, Jun 09, 2008 at 01:12:37PM +0900, OGAWA Hirofumi wrote:
> "H. Peter Anvin" <hpa@kernel.org> writes:
> 
> > The other thing about this header that needs to be fixed is the 
> > definition of the following ioctls:
> >
> > #define VFAT_IOCTL_READDIR_BOTH         _IOR('r', 1, struct dirent [2])
> > #define VFAT_IOCTL_READDIR_SHORT        _IOR('r', 2, struct dirent [2])
> >
> > "struct dirent" is a kernel type here, but is a **different type** in 
> > userspace!  This means both the structure and the IOCTL number is wrong!
> >
> > We need to make this either __kernel_dirent_t[*] or something like 
> > struct __msdos_fs_dirent.
> 
> I see. "struct dirent" in linux/dirent.h has very few users in kernel,
> and probably userland doesn't use it, so it seems it should be renamed.
> 
> Well, the patch is like this (sorry, other cleanup is in this patch)?
>...

I wanted to ask whether it could have had any users at all, but at least 
Wine contains a workaround for our bug...

> Thanks.
> 
> > 	-hpa
>...
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

[2.6 patch] fat/dir.c: switch to struct __fat_fs_dirent

[Adrian Bunk]

On Mon, Jun 09, 2008 at 01:12:37PM +0900, OGAWA Hirofumi wrote:
> "H. Peter Anvin" <hpa@kernel.org> writes:
> 
> > The other thing about this header that needs to be fixed is the 
> > definition of the following ioctls:
> >
> > #define VFAT_IOCTL_READDIR_BOTH         _IOR('r', 1, struct dirent [2])
> > #define VFAT_IOCTL_READDIR_SHORT        _IOR('r', 2, struct dirent [2])
> >
> > "struct dirent" is a kernel type here, but is a **different type** in 
> > userspace!  This means both the structure and the IOCTL number is wrong!
> >
> > We need to make this either __kernel_dirent_t[*] or something like 
> > struct __msdos_fs_dirent.
> 
> I see. "struct dirent" in linux/dirent.h has very few users in kernel,
> and probably userland doesn't use it, so it seems it should be renamed.
> 
> Well, the patch is like this (sorry, other cleanup is in this patch)?
>...

Can you apply the patch below after your patch?

Since fat was the only user of struct dirent in the kernel (there's an 
unused JFS #define I'll also kill) we can then get rid of the 
conflicting structs.

> Thanks.
>...

cu
Adrian


<--  snip  -->


struct __fat_fs_dirent is what was formerly the kernel struct dirent
(that was different from the userspace struct dirent).

Converting all fat users to struct __fat_fs_dirent will allow us to get 
rid of the conflicting struct dirent definition.

Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 fs/fat/dir.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

b8c595b4b225c2485d8dbc7ff4cbcdc8e6113672 diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 486725e..041a112 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -17,7 +17,6 @@
 #include <linux/slab.h>
 #include <linux/time.h>
 #include <linux/msdos_fs.h>
-#include <linux/dirent.h>
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/compat.h>
@@ -715,7 +714,7 @@ efault:									   \
 	return -EFAULT;							   \
 }
 
-FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, dirent)
+FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_fs_dirent)
 
 static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
 			     void __user *dirent, filldir_t filldir,
@@ -741,7 +740,7 @@ static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
 static int fat_dir_ioctl(struct inode *inode, struct file *filp,
 			 unsigned int cmd, unsigned long arg)
 {
-	struct dirent __user *d1 = (struct dirent __user *)arg;
+	struct __fat_fs_dirent __user *d1 = (struct __fat_fs_dirent __user *)arg;
 	int short_only, both;
 
 	switch (cmd) {
@@ -757,7 +756,7 @@ static int fat_dir_ioctl(struct inode *inode, struct file *filp,
 		return fat_generic_ioctl(inode, filp, cmd, arg);
 	}
 
-	if (!access_ok(VERIFY_WRITE, d1, sizeof(struct dirent[2])))
+	if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_fs_dirent[2])))
 		return -EFAULT;
 	/*
 	 * Yes, we don't need this put_user() absolutely. However old

Re: [2.6 patch] fat/dir.c: switch to struct __fat_fs_dirent

[OGAWA Hirofumi]

Adrian Bunk <bunk@kernel.org> writes:

> Can you apply the patch below after your patch?

Ok.

> Since fat was the only user of struct dirent in the kernel (there's an 
> unused JFS #define I'll also kill) we can then get rid of the 
> conflicting structs.

Sounds good. Thanks.

> <--  snip  -->
>
>
> struct __fat_fs_dirent is what was formerly the kernel struct dirent
> (that was different from the userspace struct dirent).
>
> Converting all fat users to struct __fat_fs_dirent will allow us to get 
> rid of the conflicting struct dirent definition.
>
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
>
> ---
>
>  fs/fat/dir.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> b8c595b4b225c2485d8dbc7ff4cbcdc8e6113672 diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 486725e..041a112 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -17,7 +17,6 @@
>  #include <linux/slab.h>
>  #include <linux/time.h>
>  #include <linux/msdos_fs.h>
> -#include <linux/dirent.h>
>  #include <linux/smp_lock.h>
>  #include <linux/buffer_head.h>
>  #include <linux/compat.h>
> @@ -715,7 +714,7 @@ efault:									   \
>  	return -EFAULT;							   \
>  }
>  
> -FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, dirent)
> +FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_fs_dirent)
>  
>  static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
>  			     void __user *dirent, filldir_t filldir,
> @@ -741,7 +740,7 @@ static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
>  static int fat_dir_ioctl(struct inode *inode, struct file *filp,
>  			 unsigned int cmd, unsigned long arg)
>  {
> -	struct dirent __user *d1 = (struct dirent __user *)arg;
> +	struct __fat_fs_dirent __user *d1 = (struct __fat_fs_dirent __user *)arg;
>  	int short_only, both;
>  
>  	switch (cmd) {
> @@ -757,7 +756,7 @@ static int fat_dir_ioctl(struct inode *inode, struct file *filp,
>  		return fat_generic_ioctl(inode, filp, cmd, arg);
>  	}
>  
> -	if (!access_ok(VERIFY_WRITE, d1, sizeof(struct dirent[2])))
> +	if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_fs_dirent[2])))
>  		return -EFAULT;
>  	/*
>  	 * Yes, we don't need this put_user() absolutely. However old
>

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[2.6 patch] remove the in-kernel struct dirent{,64}

[Adrian Bunk]

The kernel struct dirent{,64} were different from the ones in 
userspace.

Even worse, we exported the kernel ones to userspace.

But after the fat usages are fixed we can remove the conflicting 
kernel versions.

Reported-by: H. Peter Anvin <hpa@kernel.org>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

This patch has to be applied after the two patches that change fat to 
use __fat_fs_dirent.

 include/linux/Kbuild   |    1 -
 include/linux/dirent.h |   20 --------------------
 2 files changed, 21 deletions(-)

d447529e60e097e260ed43ca97b04dde36954c8b diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index 93b9885..4d3649a 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -186,7 +186,6 @@ unifdef-y += connector.h
 unifdef-y += cuda.h
 unifdef-y += cyclades.h
 unifdef-y += dccp.h
-unifdef-y += dirent.h
 unifdef-y += dlm.h
 unifdef-y += dlm_plock.h
 unifdef-y += edd.h
diff --git a/include/linux/dirent.h b/include/linux/dirent.h
index 5d6023b..f072fb8 100644
--- a/include/linux/dirent.h
+++ b/include/linux/dirent.h
@@ -1,23 +1,6 @@
 #ifndef _LINUX_DIRENT_H
 #define _LINUX_DIRENT_H
 
-struct dirent {
-	long		d_ino;
-	__kernel_off_t	d_off;
-	unsigned short	d_reclen;
-	char		d_name[256]; /* We must not include limits.h! */
-};
-
-struct dirent64 {
-	__u64		d_ino;
-	__s64		d_off;
-	unsigned short	d_reclen;
-	unsigned char	d_type;
-	char		d_name[256];
-};
-
-#ifdef __KERNEL__
-
 struct linux_dirent64 {
 	u64		d_ino;
 	s64		d_off;
@@ -26,7 +9,4 @@ struct linux_dirent64 {
 	char		d_name[0];
 };
 
-#endif	/* __KERNEL__ */
-
-
 #endif