Messages in this thread | | | Date | Wed, 19 Nov 2008 00:03:42 -0800 | From | Andrew Morton <> | Subject | Re: [patch] Fix type errors in inotify interfaces |
| |
On Tue, 18 Nov 2008 15:18:21 -0500 Michael Kerrisk <mtk.manpages@googlemail.com> wrote:
> Andrew, > > Vegard reminded me of an issue with the inotify interface > that I raised quite a while ago, offlist, with Robert; Robert > acknowledged that things should be fixed, but then neither of > us actually did anything :-{. > > The problems lie in the types used for some inotify interfaces, both > at the kernel level and at the glibc level. This mail addresses the > kernel problem. I will follow up with some suggestions for glibc > changes. > > For the sys_inotify_rm_watch() interface, the type of the 'wd' argument > is currently 'u32', it should be '__s32' . That is Robert's suggestion, > and is consistent with the other declarations of watch descriptors in > the kernel source, in particular, the inotify_event structure in > include/linux/inotify.h: > > struct inotify_event { > __s32 wd; /* watch descriptor */ > __u32 mask; /* watch mask */ > __u32 cookie; /* cookie to synchronize two events */ > __u32 len; /* length (including nulls) of name */ > char name[0]; /* stub for possible name */ > }; > > The patch below makes the changes needed for inotify_rm_watch(). > > Thanks > > Michael > > > diff --git a/fs/inotify_user.c b/fs/inotify_user.c > index d367e9b..a71f764 100644 > --- a/fs/inotify_user.c > +++ b/fs/inotify_user.c > @@ -704,7 +704,7 @@ fput_and_out: > return ret; > } > > -asmlinkage long sys_inotify_rm_watch(int fd, u32 wd) > +asmlinkage long sys_inotify_rm_watch(int fd, __s32 wd) > { > struct file *filp; > struct inotify_device *dev; > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index d6ff145..36983a5 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -550,7 +550,7 @@ asmlinkage long sys_inotify_init(void); > asmlinkage long sys_inotify_init1(int flags); > asmlinkage long sys_inotify_add_watch(int fd, const char __user *path, > u32 mask); > -asmlinkage long sys_inotify_rm_watch(int fd, u32 wd); > +asmlinkage long sys_inotify_rm_watch(int fd, __s32 wd); > > asmlinkage long sys_spu_run(int fd, __u32 __user *unpc, > __u32 __user *ustatus);
hrm. I see no sane reason why a watch descriptor should take on negative values, so in some ways the u32 was logical. (Ditto file descriptors, but I don't think I want to change that ;))
otoh, the system call via which one _obtains_ watch descriptors most certainly wants to return -ve nunmbers, to signify errors.
All too hard. I think I'll stop thinking about it and merge the patch ;)
| |