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

    --
    Bernd Petrovitsch Email : bernd@gams.at
    g.a.m.s gmbh Fax : +43 1 205255-900
    Prinz-Eugen-Straße 8 A-1040 Vienna/Austria/Europe
    LUGA : http://www.luga.at


    -
    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.023 / U:4.136 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site