On Wednesday 08 August 2007 19:58, Josef Sipek wrote: > On Wed, Aug 08, 2007 at 07:16:26PM +0200, Andreas Gruenbacher wrote: > > Create a temporary struct vfs_lookup in file_permission() instead of > > passing a NULL value. > > > > Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]> > > > > --- > > fs/namei.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > --- a/fs/namei.c > > +++ b/fs/namei.c > > @@ -292,14 +292,15 @@ int vfs_permission(struct vfs_lookup *lo > > * > > * Used to check for read/write/execute permissions on an already opened > > * file. > > - * > > - * Note: > > - * Do not use this function in new code. All access checks should > > - * be done using vfs_permission(). > > Should this comment be removed?
IMO yes. If vfs_permission() works for a piece of code it's the obvious preference. If on the other hand you really need to check permissions on a file, then why not use this function? > > */ > > int file_permission(struct file *file, int mask) > > { > > - return permission(file->f_path.dentry->d_inode, mask, NULL); > > + struct vfs_lookup lookup; > > + > > + lookup.path = file->f_path; > > + lookup.flags = 0; > > I tend to find this little bit cleaner: > > struct vfs_lookup lookup = { > .path = file->f_path, > .flags = 0, > }; I didn't use initializers because they initialize the entire data structure. In case of struct vfs_lookup, unless the LOOKUP_OPEN flag is set, then the open_intent doesn't need initialization. We could use a DEFINE_... macro; not sure this would improve anything though. > > + > > + return permission(file->f_path.dentry->d_inode, mask, &lookup); > > } > > > > /* Thanks, Andreas - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/