lkml.org 
[lkml]   [2003]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: How to avoid the woord 'goto' (was Re: any chance of 2.6.0-test*?)
    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

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

    \
     
     \ /
      Last update: 2005-03-22 13:32    [W:0.028 / U:29.936 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site