On Sun, 2003-01-12 at 16:15, Matti Aarnio wrote:
> On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
> > Linus,
> >
> > I'm REALLY opposed to the use of the word "goto" in any code where it's
> > not needed. OF course, I'm a linux kernel newbie, so I'm in no position
> > to comment
>
> Bob,
>
> At first, read Documentation/CodingStyle of the kernel.
> Then have a look into:
>
> fs/open.c file do_sys_truncate() function.
>
> Explain how you can do that cleanly, understandably, and without
> code duplication, or ugly kludges without using those goto ?
> (And sticking to coding-style.)
I've only compiled (and haven't tested this code), but it should be much
faster than the original code. Why? Because we're eliminating an extra
"jump" in several places in the code every time open would be called.
Yes, it's more code, so the kernel is a little bigger, but it should be
faster at the same time, and memory should be less of an issue nowadays.
Here's the patch if you want to apply it (i have only compile tested it,
I haven't booted with it).. This patch applied to the 2.5.56 kernel.
--- open.c.orig 2003-01-12 16:17:01.000000000 -0500
+++ open.c 2003-01-12 16:22:32.000000000 -0500
@@ -100,44 +100,58 @@
error = -EINVAL;
if (length < 0) /* sorry, but loff_t says... */
- goto out;
+ return error;
error = user_path_walk(path, &nd);
if (error)
- goto out;
+ return error;
inode = nd.dentry->d_inode;
/* For directories it's -EISDIR, for other non-regulars - -EINVAL */
error = -EISDIR;
- if (S_ISDIR(inode->i_mode))
- goto dput_and_out;
+ if (S_ISDIR(inode->i_mode)){
+ path_release(&nd);
+ return error;
+ }
error = -EINVAL;
- if (!S_ISREG(inode->i_mode))
- goto dput_and_out;
+ if (!S_ISREG(inode->i_mode)){
+ path_release(&nd);
+ return error;
+ }
error = permission(inode,MAY_WRITE);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }
error = -EROFS;
- if (IS_RDONLY(inode))
- goto dput_and_out;
+ if (IS_RDONLY(inode)){
+ path_release(&nd);
+ return error;
+ }
error = -EPERM;
- if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
- goto dput_and_out;
+ if (IS_IMMUTABLE(inode) || IS_APPEND(inode)){
+ path_release(&nd);
+ return error;
+ }
/*
* Make sure that there are no leases.
*/
error = break_lease(inode, FMODE_WRITE);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }
error = get_write_access(inode);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }
error = locks_verify_truncate(inode, NULL, length);
if (!error) {
@@ -146,9 +160,7 @@
}
put_write_access(inode);
-dput_and_out:
path_release(&nd);
-out:
return error;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Jan 15 2003 - 22:00:42 EST