From: Rob Wilkens <robw@optonline.net>
To: Bernd Petrovitsch <bernd@gams.at>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: How to avoid the woord 'goto' (was Re: any chance of 2.6.0-test*?)
Date: Mon, 13 Jan 2003 09:56:11 -0500 [thread overview]
Message-ID: <1042469771.846.5.camel@RobsPC.RobertWilkens.com> (raw)
In-Reply-To: <2825.1042456135@frodo.gams.co.at>
Yeah, but..
My changes won't actually be incorporated (i hope) into the kernel.
I've since learned that a goto will optimize to be just as quick as what
I wrote (by studying the optimized assembler output at other
suggestion). Therefore, it was silly and pointless for me to spend the
10-12 minutes I did changing the code and testing it and e-mailing back
the changes.
So, there's nothing to worry about. Goto's are here forever.
-Rob
On Mon, 2003-01-13 at 06:08, Bernd Petrovitsch wrote:
> Rob Wilkens <robw@optonline.net> wrote:
> [...]
> >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;
> [ snipped the rest ]
>
> You just copied the logic to "cleanup and leave" the function several
> times. The (current, next and subsequent) maintainers at the next
> change in that function simply _have_ to check all cases as if they
> are different.
> Yes, _now_ you (and all others) know that they are identical. But in 6
> month after tons of patches?
>
> Perhaps you want to avoid goto's with:
> ---- snip (yes, it is purposely not a `diff -urN`) ----
> switch(0==0) {
> default:
> error = -EINVAL;
> if (length < 0) /* sorry, but loff_t says... */
> break;
> error = user_path_walk(path, &nd);
> if (error)
> break;
> inode = nd.dentry->d_inode;
>
> switch(0==0) {
> default:
> /* For directories it's -EISDIR, for other non-regulars - -EINVAL */
> error = -EISDIR;
> if (S_ISDIR(inode->i_mode))
> break;
> error = -EINVAL;
> if (!S_ISREG(inode->i_mode))
> break;
>
> error = permission(inode,MAY_WRITE);
> if (error)
> break;
>
> error = -EROFS;
> if(IS_RDONLY(inode))
> break;
>
> /* the rest omitted - the pattern should be clear */
>
> put_write_access(inode);
> break;
> }
> path_release(&nd);
> break;
> }
> return error;
> ---- snip ----
>
> FYI, backward goto's can be rewritten with:
> ---- snip ----
>
> for(;;) {
> <do something>
> if(i_want_to_go_back)
> continue;
> <do something_else>
> break;
> }
> ---- snip ----
>
> Are these two more understandable because the avoid the 'goto'?
> And try to see it not from the viewpoint of a first time reader, but
> from the viewpoint of a/the maintainer/developer (who reads this code
> probably quite often)?
>
> Bernd
next prev parent reply other threads:[~2003-01-13 14:49 UTC|newest]
Thread overview: 173+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-01-10 16:10 any chance of 2.6.0-test*? William Lee Irwin III
2003-01-10 16:28 ` Dave Jones
2003-01-10 16:41 ` Zwane Mwaikambo
2003-01-10 17:08 ` Dave Jones
2003-01-10 17:19 ` Alan Cox
2003-01-10 16:40 ` Jeff Garzik
2003-01-10 17:06 ` Dave Jones
2003-01-10 17:25 ` Jeff Garzik
2003-01-10 17:29 ` Linus Torvalds
2003-01-10 18:47 ` J.A. Magallon
2003-01-10 19:37 ` Matthew D. Pitts
2003-01-10 19:51 ` Jeff Garzik
2003-01-14 1:39 ` James H. Cloos Jr.
2003-01-15 21:46 ` [PATCH][RESEND w/ reasonable Subject] alsa before oss in Kconfig James H. Cloos Jr.
2003-01-10 18:16 ` any chance of 2.6.0-test*? Alan Cox
2003-01-10 17:37 ` Jochen Friedrich
2003-01-10 17:38 ` Linus Torvalds
2003-01-12 9:27 ` Greg KH
2003-01-12 16:55 ` Alan Cox
2003-01-12 17:05 ` Linus Torvalds
2003-01-12 17:17 ` Christoph Hellwig
2003-01-12 19:15 ` Linus Torvalds
2003-01-12 19:34 ` Rob Wilkens
2003-01-12 19:37 ` Rob Wilkens
2003-01-12 19:53 ` Tomas Szepe
2003-01-12 20:03 ` Rob Wilkens
2003-01-13 15:43 ` Terje Eggestad
2003-01-13 15:49 ` Jens Axboe
2003-01-13 16:25 ` Terje Eggestad
2003-01-13 16:26 ` Jens Axboe
2003-01-13 16:41 ` Terje Eggestad
2003-01-13 16:43 ` Jens Axboe
2003-01-13 17:00 ` Zwane Mwaikambo
2003-01-13 18:48 ` Jens Axboe
2003-01-13 22:14 ` Keith Owens
2003-01-13 22:42 ` John Levon
2003-01-13 22:49 ` Zwane Mwaikambo
2003-01-13 23:32 ` Valdis.Kletnieks
2003-01-13 23:43 ` Zwane Mwaikambo
2003-01-12 20:44 ` Alan Cox
2003-01-12 20:45 ` William Lee Irwin III
2003-01-13 13:14 ` Bernd Schmidt
2003-01-12 21:22 ` David Woodhouse
2003-01-18 13:04 ` Folkert van Heusden
2003-01-12 19:38 ` Linus Torvalds
2003-01-12 19:59 ` Rob Wilkens
2003-01-12 20:18 ` Valdis.Kletnieks
2003-01-12 20:23 ` yodaiken
2003-01-12 20:26 ` Tomas Szepe
2003-01-12 20:32 ` Sean Neakums
2003-01-12 20:51 ` Valdis.Kletnieks
2003-01-13 0:54 ` Randy Dunlap
2003-01-12 20:46 ` Kevin Puetz
2003-01-13 0:48 ` Randy Dunlap
2003-01-18 13:07 ` Folkert van Heusden
2003-01-12 20:20 ` David Ford
2003-01-12 20:22 ` Linus Torvalds
2003-01-12 20:33 ` Robert Love
2003-01-12 20:33 ` Linus Torvalds
2003-01-13 10:54 ` Horst von Brand
2003-01-13 11:09 ` Eric W. Biederman
2003-01-13 14:34 ` Terje Eggestad
2003-01-13 23:23 ` Bob Taylor
2003-01-12 20:48 ` Rob Wilkens
2003-01-12 20:59 ` Dimitrie O. Paun
2003-01-12 22:48 ` any chance of 2.6.0-test*? -> goto example Willy Tarreau
2003-01-13 0:53 ` Rob Wilkens
2003-01-13 1:31 ` Willy Tarreau
2003-01-13 16:10 ` Alexander Kellett
2003-01-14 10:36 ` Helge Hafting
2003-01-12 23:11 ` Coding style - (Was Re: any chance of 2.6.0-test*?) John Bradford
2003-01-13 11:07 ` any chance of 2.6.0-test*? Helge Hafting
2003-01-13 3:30 ` yodaiken
2003-01-12 21:29 ` Rik van Riel
2003-01-13 0:03 ` Scott Robert Ladd
2003-01-13 0:38 ` Rob Wilkens
2003-01-13 3:06 ` [OT] " J Sloan
2003-01-13 16:51 ` Emiliano Gabrielli
2003-01-13 0:38 ` Randy Dunlap
2003-01-12 19:41 ` Christoph Hellwig
2003-01-12 19:41 ` Rob Wilkens
2003-01-12 19:58 ` David Ford
2003-01-12 20:07 ` Rob Wilkens
2003-01-12 20:31 ` Oliver Neukum
2003-01-12 20:34 ` David Ford
2003-01-12 20:31 ` Robert Love
2003-01-12 21:02 ` Rob Wilkens
2003-01-12 21:15 ` Matti Aarnio
2003-01-12 21:27 ` Rob Wilkens
2003-01-12 21:40 ` Rik van Riel
2003-01-12 21:44 ` Rob Wilkens
2003-01-12 21:49 ` Aaron Lehmann
2003-01-12 22:07 ` Rob Wilkens
2003-01-12 22:18 ` Aaron Lehmann
2003-01-12 22:34 ` Rob Wilkens
2003-01-12 22:52 ` Aaron Lehmann
2003-01-12 23:11 ` Rob Wilkens
2003-01-12 23:31 ` Oliver Neukum
2003-01-12 23:39 ` Emiliano Gabrielli
2003-01-12 23:46 ` Rob Wilkens
2003-01-12 23:57 ` Emiliano Gabrielli
2003-01-13 0:08 ` Rob Wilkens
2003-01-13 16:02 ` Terje Eggestad
2003-01-12 22:53 ` Sean Neakums
2003-01-12 23:06 ` Oliver Neukum
2003-01-12 23:48 ` Rik van Riel
2003-01-12 21:58 ` Emiliano Gabrielli
2003-01-12 22:12 ` Rob Wilkens
2003-01-12 22:29 ` Olivier Galibert
2003-01-12 23:21 ` Alan Cox
2003-01-12 22:12 ` Oliver Neukum
2003-01-12 22:18 ` Aaron Lehmann
2003-01-12 22:35 ` Rob Wilkens
2003-01-13 8:15 ` Jens Axboe
2003-01-12 21:49 ` David Ford
2003-01-13 19:22 ` [OffTopic] [apology] " Rob Wilkens
2003-01-12 21:59 ` Adam Kropelin
2003-01-12 22:21 ` Rob Wilkens
2003-01-12 22:33 ` Tomas Szepe
2003-01-12 22:36 ` Emiliano Gabrielli
2003-01-12 22:38 ` Oliver Neukum
2003-01-12 22:42 ` romieu
2003-01-13 1:06 ` Randy Dunlap
2003-01-13 1:21 ` Rob Wilkens
2003-01-13 1:51 ` Nuno Monteiro
2003-01-13 2:19 ` Rob Wilkens
2003-01-13 2:51 ` Marcelo Pacheco
2003-01-13 3:37 ` Rob Wilkens
2003-01-13 4:53 ` Billy Rose
2003-01-13 3:06 ` Jochen Striepe
2003-01-13 3:40 ` Rob Wilkens
2003-01-13 9:45 ` Matti Aarnio
2003-01-13 3:48 ` Ryan Anderson
2003-01-13 4:37 ` Billy Rose
2003-01-12 22:06 ` Oliver Neukum
2003-01-12 22:22 ` Rob Wilkens
2003-01-12 22:43 ` Oliver Neukum
2003-01-12 22:51 ` Rob Wilkens
2003-01-12 23:00 ` Robert Love
2003-01-12 23:16 ` Rob Wilkens
2003-01-12 23:16 ` Nicolas Pitre
2003-01-12 23:05 ` Emiliano Gabrielli
2003-01-12 23:23 ` Oliver Neukum
2003-01-12 23:45 ` Emiliano Gabrielli
2003-01-12 23:45 ` Emiliano Gabrielli
2003-01-13 20:01 ` Horst von Brand
2003-01-12 23:16 ` Oliver Neukum
2003-01-13 2:00 ` Ryan Anderson
2003-01-13 3:09 ` Rob Wilkens
2003-01-14 2:13 ` Rik van Riel
2003-01-12 22:58 ` Robert Love
2003-01-12 22:28 ` Matti Aarnio
2003-01-13 1:26 ` David Lang
2003-01-13 2:00 ` William Lee Irwin III
2003-01-13 11:08 ` How to avoid the woord 'goto' (was Re: any chance of 2.6.0-test*?) Bernd Petrovitsch
2003-01-13 14:56 ` Rob Wilkens [this message]
2003-01-13 22:02 ` any chance of 2.6.0-test*? Val Henson
2003-01-13 10:32 ` gotos in kernel [Was: Re: any chance of 2.6.0-test*?] Horst von Brand
2003-01-13 14:53 ` Rob Wilkens
2003-01-13 13:08 ` any chance of 2.6.0-test*? Dave Jones
2003-01-13 13:40 ` Andrew Walrond
2003-01-13 13:49 ` Oliver Neukum
2003-01-14 6:52 ` Bruce Harada
2003-01-14 7:12 ` Brian Tinsley
2003-01-13 15:23 ` Richard B. Johnson
2003-01-13 6:23 ` Greg KH
2003-01-13 16:35 ` Linus Torvalds
2003-01-13 18:33 ` Alan Cox
2003-01-13 18:01 ` Linus Torvalds
2003-01-13 19:17 ` Andrew McGregor
2003-01-12 17:37 ` Russell King
2003-01-13 1:08 ` Michael Kingsbury
2003-01-13 1:52 ` William Lee Irwin III
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1042469771.846.5.camel@RobsPC.RobertWilkens.com \
--to=robw@optonline.net \
--cc=bernd@gams.at \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).