Re: [PATCH] Code style fix for open_exec
Christoph Hellwig wrote: On Wed, Oct 03, 2007 at 10:40:25PM -0400, Casey Dahlin wrote: From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 From: Casey Dahlin <[EMAIL PROTECTED]> Date: Wed, 3 Oct 2007 22:01:49 -0400 Subject: [PATCH] Code style fix for open_exec Fix a horribly mangled 5 level indent and severe abuse of goto in the open_exec function. While the function is not nice your version makes it even worse. Ouch :) I don't think I did so bad. Try this one instead which has been in my pending testing tree for a while: Pretty much all of the stylistic changes were recommended to me already off list by Joe Perches. I have that in my branch now, and was waiting on any further feedback before resubmitting. There's a couple of things here though (particularly the unlikelys) that are worth having. This is the better code. Index: linux-2.6/fs/exec.c === --- linux-2.6.orig/fs/exec.c2007-09-16 20:29:01.0 +0200 +++ linux-2.6/fs/exec.c 2007-09-16 20:29:56.0 +0200 @@ -669,41 +669,55 @@ EXPORT_SYMBOL(setup_arg_pages); #endif /* CONFIG_MMU */ +/** + * open_exec - open file for execution + * @name: filename to open + * + * Open the file pointed to by @filename in the current namespace + * for execution. Error out if the file is not a regular file or + * this mount does not allow executing files on it. + */ 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, , FMODE_READ|FMODE_EXEC); - file = ERR_PTR(err); + err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, , + FMODE_READ|FMODE_EXEC); + if (unlikely(err)) + goto out; - if (!err) { - struct inode *inode = nd.dentry->d_inode; - file = ERR_PTR(-EACCES); - if (!(nd.mnt->mnt_flags & MNT_NOEXEC) && - S_ISREG(inode->i_mode)) { - int err = vfs_permission(, MAY_EXEC); - file = ERR_PTR(err); - if (!err) { - file = nameidata_to_filp(, O_RDONLY); - if (!IS_ERR(file)) { - err = deny_write_access(file); - if (err) { - fput(file); - file = ERR_PTR(err); - } - } -out: - return file; - } - } - release_open_intent(); - path_release(); + err = -EACCES; + if (unlikely(nd.mnt->mnt_flags & MNT_NOEXEC)) + goto out_path_release; + if (unlikely(!S_ISREG(nd.dentry->d_inode->i_mode))) + goto out_path_release; + + err = vfs_permission(, MAY_EXEC); + if (unlikely(err)) + goto out_path_release; + + file = nameidata_to_filp(, O_RDONLY); + if (unlikely(IS_ERR(file))) { + err = PTR_ERR(file); + goto out; + } + + err = deny_write_access(file); + if (unlikely(err)) { + fput(file); + goto out; } - goto out; -} + return file; + + out_path_release: + release_open_intent(); + path_release(); + out: + return ERR_PTR(err); +} EXPORT_SYMBOL(open_exec); int kernel_read(struct file *file, unsigned long offset, - 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/ - 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/
Re: [PATCH] Code style fix for open_exec
On Wed, Oct 03, 2007 at 10:40:25PM -0400, Casey Dahlin wrote: > From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 > From: Casey Dahlin <[EMAIL PROTECTED]> > Date: Wed, 3 Oct 2007 22:01:49 -0400 > Subject: [PATCH] Code style fix for open_exec > > Fix a horribly mangled 5 level indent and severe abuse of goto in the > open_exec > function. While the function is not nice your version makes it even worse. Try this one instead which has been in my pending testing tree for a while: Index: linux-2.6/fs/exec.c === --- linux-2.6.orig/fs/exec.c2007-09-16 20:29:01.0 +0200 +++ linux-2.6/fs/exec.c 2007-09-16 20:29:56.0 +0200 @@ -669,41 +669,55 @@ EXPORT_SYMBOL(setup_arg_pages); #endif /* CONFIG_MMU */ +/** + * open_exec - open file for execution + * @name: filename to open + * + * Open the file pointed to by @filename in the current namespace + * for execution. Error out if the file is not a regular file or + * this mount does not allow executing files on it. + */ 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, , FMODE_READ|FMODE_EXEC); - file = ERR_PTR(err); + err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, , + FMODE_READ|FMODE_EXEC); + if (unlikely(err)) + goto out; - if (!err) { - struct inode *inode = nd.dentry->d_inode; - file = ERR_PTR(-EACCES); - if (!(nd.mnt->mnt_flags & MNT_NOEXEC) && - S_ISREG(inode->i_mode)) { - int err = vfs_permission(, MAY_EXEC); - file = ERR_PTR(err); - if (!err) { - file = nameidata_to_filp(, O_RDONLY); - if (!IS_ERR(file)) { - err = deny_write_access(file); - if (err) { - fput(file); - file = ERR_PTR(err); - } - } -out: - return file; - } - } - release_open_intent(); - path_release(); + err = -EACCES; + if (unlikely(nd.mnt->mnt_flags & MNT_NOEXEC)) + goto out_path_release; + if (unlikely(!S_ISREG(nd.dentry->d_inode->i_mode))) + goto out_path_release; + + err = vfs_permission(, MAY_EXEC); + if (unlikely(err)) + goto out_path_release; + + file = nameidata_to_filp(, O_RDONLY); + if (unlikely(IS_ERR(file))) { + err = PTR_ERR(file); + goto out; + } + + err = deny_write_access(file); + if (unlikely(err)) { + fput(file); + goto out; } - goto out; -} + return file; + + out_path_release: + release_open_intent(); + path_release(); + out: + return ERR_PTR(err); +} EXPORT_SYMBOL(open_exec); int kernel_read(struct file *file, unsigned long offset, - 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/
Re: [PATCH] Code style fix for open_exec
On Wed, Oct 03, 2007 at 10:40:25PM -0400, Casey Dahlin wrote: From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 From: Casey Dahlin [EMAIL PROTECTED] Date: Wed, 3 Oct 2007 22:01:49 -0400 Subject: [PATCH] Code style fix for open_exec Fix a horribly mangled 5 level indent and severe abuse of goto in the open_exec function. While the function is not nice your version makes it even worse. Try this one instead which has been in my pending testing tree for a while: Index: linux-2.6/fs/exec.c === --- linux-2.6.orig/fs/exec.c2007-09-16 20:29:01.0 +0200 +++ linux-2.6/fs/exec.c 2007-09-16 20:29:56.0 +0200 @@ -669,41 +669,55 @@ EXPORT_SYMBOL(setup_arg_pages); #endif /* CONFIG_MMU */ +/** + * open_exec - open file for execution + * @name: filename to open + * + * Open the file pointed to by @filename in the current namespace + * for execution. Error out if the file is not a regular file or + * this mount does not allow executing files on it. + */ 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 (unlikely(err)) + goto out; - if (!err) { - struct inode *inode = nd.dentry-d_inode; - file = ERR_PTR(-EACCES); - if (!(nd.mnt-mnt_flags MNT_NOEXEC) - 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); - 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_release(nd); + err = -EACCES; + if (unlikely(nd.mnt-mnt_flags MNT_NOEXEC)) + goto out_path_release; + if (unlikely(!S_ISREG(nd.dentry-d_inode-i_mode))) + goto out_path_release; + + err = vfs_permission(nd, MAY_EXEC); + if (unlikely(err)) + goto out_path_release; + + file = nameidata_to_filp(nd, O_RDONLY); + if (unlikely(IS_ERR(file))) { + err = PTR_ERR(file); + goto out; + } + + err = deny_write_access(file); + if (unlikely(err)) { + fput(file); + goto out; } - goto out; -} + return file; + + out_path_release: + release_open_intent(nd); + path_release(nd); + out: + return ERR_PTR(err); +} EXPORT_SYMBOL(open_exec); int kernel_read(struct file *file, unsigned long offset, - 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/
Re: [PATCH] Code style fix for open_exec
Christoph Hellwig wrote: On Wed, Oct 03, 2007 at 10:40:25PM -0400, Casey Dahlin wrote: From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 From: Casey Dahlin [EMAIL PROTECTED] Date: Wed, 3 Oct 2007 22:01:49 -0400 Subject: [PATCH] Code style fix for open_exec Fix a horribly mangled 5 level indent and severe abuse of goto in the open_exec function. While the function is not nice your version makes it even worse. Ouch :) I don't think I did so bad. Try this one instead which has been in my pending testing tree for a while: Pretty much all of the stylistic changes were recommended to me already off list by Joe Perches. I have that in my branch now, and was waiting on any further feedback before resubmitting. There's a couple of things here though (particularly the unlikelys) that are worth having. This is the better code. Index: linux-2.6/fs/exec.c === --- linux-2.6.orig/fs/exec.c2007-09-16 20:29:01.0 +0200 +++ linux-2.6/fs/exec.c 2007-09-16 20:29:56.0 +0200 @@ -669,41 +669,55 @@ EXPORT_SYMBOL(setup_arg_pages); #endif /* CONFIG_MMU */ +/** + * open_exec - open file for execution + * @name: filename to open + * + * Open the file pointed to by @filename in the current namespace + * for execution. Error out if the file is not a regular file or + * this mount does not allow executing files on it. + */ 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 (unlikely(err)) + goto out; - if (!err) { - struct inode *inode = nd.dentry-d_inode; - file = ERR_PTR(-EACCES); - if (!(nd.mnt-mnt_flags MNT_NOEXEC) - 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); - 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_release(nd); + err = -EACCES; + if (unlikely(nd.mnt-mnt_flags MNT_NOEXEC)) + goto out_path_release; + if (unlikely(!S_ISREG(nd.dentry-d_inode-i_mode))) + goto out_path_release; + + err = vfs_permission(nd, MAY_EXEC); + if (unlikely(err)) + goto out_path_release; + + file = nameidata_to_filp(nd, O_RDONLY); + if (unlikely(IS_ERR(file))) { + err = PTR_ERR(file); + goto out; + } + + err = deny_write_access(file); + if (unlikely(err)) { + fput(file); + goto out; } - goto out; -} + return file; + + out_path_release: + release_open_intent(nd); + path_release(nd); + out: + return ERR_PTR(err); +} EXPORT_SYMBOL(open_exec); int kernel_read(struct file *file, unsigned long offset, - 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/ - 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/
[PATCH] Code style fix for open_exec
From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 From: Casey Dahlin <[EMAIL PROTECTED]> Date: Wed, 3 Oct 2007 22:01:49 -0400 Subject: [PATCH] Code style fix for open_exec Fix a horribly mangled 5 level indent and severe abuse of goto in the open_exec function. Signed-off-by: Casey Dahlin <[EMAIL PROTECTED]> diff --git a/fs/exec.c b/fs/exec.c index c21a8cc..d73da5a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -676,32 +676,32 @@ struct file *open_exec(const char *name) struct file *file; err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, , FMODE_READ|FMODE_EXEC); + +if (err) return ERR_PTR(err); + +file = ERR_PTR(-EACCES); +if ((nd.mnt->mnt_flags & MNT_NOEXEC) || +!S_ISREG(nd.dentry->d_inode->i_mode)) +goto fail; + +err = vfs_permission(, MAY_EXEC); file = ERR_PTR(err); +if (err) goto fail; -if (!err) { -struct inode *inode = nd.dentry->d_inode; -file = ERR_PTR(-EACCES); -if (!(nd.mnt->mnt_flags & MNT_NOEXEC) && -S_ISREG(inode->i_mode)) { -int err = vfs_permission(, MAY_EXEC); -file = ERR_PTR(err); -if (!err) { -file = nameidata_to_filp(, O_RDONLY); -if (!IS_ERR(file)) { -err = deny_write_access(file); -if (err) { -fput(file); -file = ERR_PTR(err); -} -} -out: -return file; -} -} -release_open_intent(); -path_release(); +file = nameidata_to_filp(, O_RDONLY); +if (IS_ERR(file)) return file; + +err = deny_write_access(file); +if (err) { +fput(file); +file = ERR_PTR(err); } -goto out; + +return file; +fail: +release_open_intent(); +path_release(); +return file; } EXPORT_SYMBOL(open_exec); - 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/
[PATCH] Code style fix for open_exec
From d2a6c5d29dc34cfea892124ab72b4eb55d2f8a80 Mon Sep 17 00:00:00 2001 From: Casey Dahlin [EMAIL PROTECTED] Date: Wed, 3 Oct 2007 22:01:49 -0400 Subject: [PATCH] Code style fix for open_exec Fix a horribly mangled 5 level indent and severe abuse of goto in the open_exec function. Signed-off-by: Casey Dahlin [EMAIL PROTECTED] diff --git a/fs/exec.c b/fs/exec.c index c21a8cc..d73da5a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -676,32 +676,32 @@ struct file *open_exec(const char *name) struct file *file; err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, nd, FMODE_READ|FMODE_EXEC); + +if (err) return ERR_PTR(err); + +file = ERR_PTR(-EACCES); +if ((nd.mnt-mnt_flags MNT_NOEXEC) || +!S_ISREG(nd.dentry-d_inode-i_mode)) +goto fail; + +err = vfs_permission(nd, MAY_EXEC); file = ERR_PTR(err); +if (err) goto fail; -if (!err) { -struct inode *inode = nd.dentry-d_inode; -file = ERR_PTR(-EACCES); -if (!(nd.mnt-mnt_flags MNT_NOEXEC) -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); -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_release(nd); +file = nameidata_to_filp(nd, O_RDONLY); +if (IS_ERR(file)) return file; + +err = deny_write_access(file); +if (err) { +fput(file); +file = ERR_PTR(err); } -goto out; + +return file; +fail: +release_open_intent(nd); +path_release(nd); +return file; } EXPORT_SYMBOL(open_exec); - 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/