2008-06-08 18:17:21

by S.Çağlar Onur

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

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 <[email protected]>

Cc: Frank Seidel <[email protected]>
CC: OGAWA Hirofumi <[email protected]>
CC: Onur Küçük <[email protected]>

Signed-off-by: S.Çağlar Onur <[email protected]>

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 <[email protected]>
http://cekirdek.pardus.org.tr/~caglar/

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


2008-06-08 18:36:49

by OGAWA Hirofumi

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

"S.$(D*.(Ba$(D+;(Blar Onur" <[email protected]> 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 <[email protected]>
>
> Cc: Frank Seidel <[email protected]>
> CC: OGAWA Hirofumi <[email protected]>
> CC: Onur K$(D+d+.+d(Bk <[email protected]>
>
> Signed-off-by: S.$(D*.(Ba$(D+;(Blar Onur <[email protected]>

Looks good to me. Thanks.

Acked-by: OGAWA Hirofumi <[email protected]>

> 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 <[email protected]>

2008-06-08 18:44:15

by Adrian Bunk

[permalink] [raw]
Subject: [2.6.26 patch] fat_valid_media() isn't for userspace

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 <[email protected]>
>
> Cc: Frank Seidel <[email protected]>
> CC: OGAWA Hirofumi <[email protected]>
> CC: Onur Küçük <[email protected]>
>
> Signed-off-by: S.Çağlar Onur <[email protected]>
>
> 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 <[email protected]>
Reported-by: S.Çağlar Onur <[email protected]>
Signed-off-by: Adrian Bunk <[email protected]>

---

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,

2008-06-08 18:55:44

by OGAWA Hirofumi

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

Adrian Bunk <[email protected]> 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 <[email protected]>

> 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 <[email protected]>
> Reported-by: S.$(D*.(Ba$(D+;(Blar Onur <[email protected]>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> 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 <[email protected]>

2008-06-09 01:30:57

by H. Peter Anvin

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

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.

2008-06-09 04:12:54

by OGAWA Hirofumi

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

"H. Peter Anvin" <[email protected]> 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 <[email protected]>



Signed-off-by: OGAWA Hirofumi <[email protected]>
---

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;
_

2008-06-09 04:59:26

by H. Peter Anvin

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

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

2008-06-09 08:06:16

by Adrian Bunk

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

On Mon, Jun 09, 2008 at 01:12:37PM +0900, OGAWA Hirofumi wrote:
> "H. Peter Anvin" <[email protected]> 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 <[email protected]>
>...

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

2008-06-09 22:30:42

by Adrian Bunk

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

On Mon, Jun 09, 2008 at 01:12:37PM +0900, OGAWA Hirofumi wrote:
> "H. Peter Anvin" <[email protected]> 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 <[email protected]>

---

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

2008-06-09 22:30:59

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] remove the in-kernel struct dirent{,64}

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 <[email protected]>
Signed-off-by: Adrian Bunk <[email protected]>

---

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

2008-06-09 23:12:20

by OGAWA Hirofumi

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

Adrian Bunk <[email protected]> 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 <[email protected]>
>
> ---
>
> 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 <[email protected]>