Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754263AbYKSTZE (ORCPT ); Wed, 19 Nov 2008 14:25:04 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752665AbYKSTYx (ORCPT ); Wed, 19 Nov 2008 14:24:53 -0500 Received: from rv-out-0506.google.com ([209.85.198.238]:29919 "EHLO rv-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752193AbYKSTYx (ORCPT ); Wed, 19 Nov 2008 14:24:53 -0500 Message-ID: Date: Wed, 19 Nov 2008 11:24:51 -0800 From: "John McCutchan" To: mtk.manpages@gmail.com Subject: Re: [patch] Fix type errors in inotify interfaces Cc: "Andrew Morton" , lkml , "Robert Love" , "Vegard Nossum" , "Ulrich Drepper" In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <4923230D.8090301@gmail.com> <20081119000342.05b2de85.akpm@linux-foundation.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3499 Lines: 90 On Wed, Nov 19, 2008 at 11:10 AM, Michael Kerrisk 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 > wrote: >> On Tue, 18 Nov 2008 15:18:21 -0500 Michael Kerrisk 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 McCutchan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/