[PATCH] Readability improvement of open_exec()

[PATCH] Readability improvement of open_exec()

[Casey Dahlin]

A fairly trivial patch that simply improves the readability of the
open_exec() function. It no longer executes primarily inside nested ifs
or has 5 levels of indentation :) Logically it should be no different
from the original. Patch applies to stock 2.6.18 kernel.


Signed-off-by: Casey Dahlin <cjdahlin@ncsu.edu>

---

diff -up exec.c.bak exec.c
--- exec.c.bak	2006-09-19 23:42:06.000000000 -0400
+++ exec.c	2006-10-12 21:42:01.000000000 -0400
@@ -474,36 +474,43 @@ static inline void free_arg_pages(struct
 struct file *open_exec(const char *name)
 {
 	struct nameidata nd;
-	int err;
+	struct inode *inode;
+	int err, perm_err;
 	struct file *file;
 
 	err = path_lookup_open(AT_FDCWD, name, LOOKUP_FOLLOW, &nd, FMODE_READ|
FMODE_EXEC);
 	file = ERR_PTR(err);
 
-	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);
-	}
+	if (err)
+		goto out;
+
+	inode = nd.dentry->d_inode;
+	file = ERR_PTR(-EACCES);
+	if ((nd.mnt->mnt_flags & MNT_NOEXEC) ||
+	    !S_ISREG(inode->i_mode))
+		goto cleanup;
+
+	perm_err = vfs_permission(&nd, MAY_EXEC);
+	file = ERR_PTR(perm_err);
+	if (perm_err)
+		goto cleanup;
+
+	file = nameidata_to_filp(&nd, O_RDONLY);
+	if (IS_ERR(file))
+		goto out;
+
+	perm_err = deny_write_access(file);
+	if (perm_err) {
+		fput(file);
+		file = ERR_PTR(err);
+	}	
 	goto out;
+
+cleanup:
+	release_open_intent(&nd);
+	path_release(&nd);
+out:
+	return file;
 }
 
 EXPORT_SYMBOL(open_exec);

Re: [PATCH] Readability improvement of open_exec()

[Christoph Hellwig]

> +	if (err)
> +		goto out;
> +
> +	inode = nd.dentry->d_inode;
> +	file = ERR_PTR(-EACCES);
> +	if ((nd.mnt->mnt_flags & MNT_NOEXEC) ||
> +	    !S_ISREG(inode->i_mode))

While you're cleaning up things you can put the whole if statement on
one line, it's less than 80 characters long.

Re: [PATCH] Readability improvement of open_exec()

[Casey Dahlin]

> While you're cleaning up things you can put the whole if statement on
> one line, it's less than 80 characters long.
> 

Ah, true. It was on one two lines back when it was tabbed all the way
over. It also seems that this mail tool turned the tabs to spaces X( so
I'll probably have to submit again anyway.

Re: [PATCH] Readability improvement of open_exec()

[Randy Dunlap]

On Thu, 12 Oct 2006 22:42:13 -0400 Casey Dahlin wrote:

> A fairly trivial patch that simply improves the readability of the
> open_exec() function. It no longer executes primarily inside nested ifs
> or has 5 levels of indentation :) Logically it should be no different
> from the original. Patch applies to stock 2.6.18 kernel.
> 
> 
> Signed-off-by: Casey Dahlin <cjdahlin@ncsu.edu>
> 
> ---
> 
> diff -up exec.c.bak exec.c
> --- exec.c.bak	2006-09-19 23:42:06.000000000 -0400
> +++ exec.c	2006-10-12 21:42:01.000000000 -0400

Please use diff and diffstat as suggested in
Documentation/SubmittingPatches, and do so from the top-level
directory of the kernel source tree, so that the full
path/file names are used in the diff lines.

---
~Randy