2006-10-13 02:42:23

by Casey Dahlin

Subject: [PATCH] Readability improvement of open_exec()

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 <[email protected]>

---

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);

2006-10-13 07:30:18

by Christoph Hellwig

Subject: Re: [PATCH] Readability improvement of open_exec()

> + 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.

2006-10-13 19:41:43

by Casey Dahlin

Subject: Re: [PATCH] Readability improvement of open_exec()

> 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.

2006-10-15 18:09:24

by Randy Dunlap

Subject: Re: [PATCH] Readability improvement of open_exec()

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 <[email protected]>
>
> ---
>
> 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