[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cfd18e0f0811191110t792cc9a4i5869bee0fb3d759c@mail.gmail.com>
Date: Wed, 19 Nov 2008 14:10:26 -0500
From: "Michael Kerrisk" <mtk.manpages@...glemail.com>
To: "Andrew Morton" <akpm@...ux-foundation.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
"Robert Love" <rlove@...gle.com>,
"Vegard Nossum" <vegard.nossum@...il.com>,
"Ulrich Drepper" <drepper@...hat.com>, john@...nmccutchan.com
Subject: Re: [patch] Fix type errors in inotify interfaces
(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@...ux-foundation.org> wrote:
> On Tue, 18 Nov 2008 15:18:21 -0500 Michael Kerrisk <mtk.manpages@...glemail.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 ;)
Thanks.
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists