Messages in this thread | | | Date | Thu, 22 May 2008 17:57:05 +0200 | From | Marcin Slusarz <> | Subject | Re: [PATCH 3/6] vfs: open_exec cleanup |
| |
On Mon, May 19, 2008 at 07:53:34AM +0200, Christoph Hellwig wrote: > On Mon, May 19, 2008 at 12:01:49AM +0200, Marcin Slusarz wrote: > > open_exec is needlessly indented, calls ERR_PTR with 0 argument > > (which is not valid errno) and jumps into middle of function > > just to return value. > > So clean it up a bit. > > Still looks rather messy. See below for a better version.
looks much better, thanks Reviewed-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: linux-2.6/fs/exec.c > =================================================================== > --- linux-2.6.orig/fs/exec.c 2008-05-19 07:45:18.000000000 +0200 > +++ linux-2.6/fs/exec.c 2008-05-19 07:53:17.000000000 +0200 > @@ -654,38 +654,40 @@ EXPORT_SYMBOL(setup_arg_pages); > struct file *open_exec(const char *name) > { > struct nameidata nd; > - int err; > struct file *file; > + int err; > > - err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, &nd, FMODE_READ|FMODE_EXEC); > - file = ERR_PTR(err); > + err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, &nd, > + FMODE_READ|FMODE_EXEC); > + if (err) > + goto out; > > - if (!err) { > - struct inode *inode = nd.path.dentry->d_inode; > - file = ERR_PTR(-EACCES); > - if (S_ISREG(inode->i_mode)) { > - int err = vfs_permission(&nd, MAY_EXEC); > - file = ERR_PTR(err); > - if (!err) { > - file = nameidata_to_filp(&nd, > - O_RDONLY|O_LARGEFILE); > - if (!IS_ERR(file)) { > - err = deny_write_access(file); > - if (err) { > - fput(file); > - file = ERR_PTR(err); > - } > - } > -out: > - return file; > - } > - } > - release_open_intent(&nd); > - path_put(&nd.path); > + err = -EACCES; > + if (!S_ISREG(nd.path.dentry->d_inode->i_mode)) > + goto out_path_put; > + > + err = vfs_permission(&nd, MAY_EXEC); > + if (err) > + goto out_path_put; > + > + file = nameidata_to_filp(&nd, O_RDONLY|O_LARGEFILE); > + if (IS_ERR(file)) > + return file; > + > + err = deny_write_access(file); > + if (err) { > + fput(file); > + goto out; > } > - goto out; > -} > > + return file; > + > + out_path_put: > + release_open_intent(&nd); > + path_put(&nd.path); > + out: > + return ERR_PTR(err); > +} > EXPORT_SYMBOL(open_exec); > > int kernel_read(struct file *file, unsigned long offset, >
| |