Messages in this thread | | | Date | Wed, 19 Nov 2008 11:24:51 -0800 | From | "John McCutchan" <> | Subject | Re: [patch] Fix type errors in inotify interfaces |
| |
On Wed, Nov 19, 2008 at 11:10 AM, Michael Kerrisk <mtk.manpages@googlemail.com> wrote: > (Would be nice to see an Aacked-by from Robert or John on this patch.) > > On Wed, Nov 19, 2008 at 3:03 AM, Andrew Morton > <akpm@linux-foundation.org> wrote: >> 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); >> > > Yes -- there is no sane reason for a negative watch descriptor to > inotify_rm_watch(); this change is mainly about consistency. (The > glibc change is probably more important.) > >> 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 ;) >
Seems sane enough.
Acked-by: John McCutchan <john@johnmccutchan.com>
-- John McCutchan <john@johnmccutchan.com>
| |