Settings

Theme

This needs some heavy checking... (line 1029)

github.com

30 points by dennis_moore 4 years ago · 65 comments

Reader

codingkev 4 years ago

You can link directly to a line by appending #<line_number>

https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3...

lapetitejort 4 years ago

I spied a goto. Ctrl/Command+F spies 60 gotos. CS students: point to the Linux source code if your professor ever gives you grief about gotos.

  • dblohm7 4 years ago

    One of the first codebases I ever worked on professionally allowed gotos as part of its style guide, but only under two conditions:

    1. You're using the goto for error handling and cleanup in a function; and

    2. You only jump in a forward direction.

    • thechao 4 years ago

      The C `switch` statement and `goto` were made for each other, too.

    • TheSockStealer 4 years ago

      Used like this, it is not much different than a try-catch block

      • SXX 4 years ago

        Yeah and it's make sense to use it this way since there is no try-catch in C.

      • jimmygrapes 4 years ago

        Reminds me of BASIC best practices:

            ON ERROR GOTO handler
            [try code here] 
            GOTO finally
        
            handler:
            [catch code here]
        
            finally:
            [finally code here]
  • Jtsummers 4 years ago

    Or Knuth's take on it: https://pic.plover.com/knuth-GOTO.pdf [pdf] Structured Programming with go to Statements. Which helps to clarify why go to statements are both useful and worth minimizing, from a person who makes extensive use of goto statements in a lot of their code.

  • alkonaut 4 years ago

    Not all gotos are made equal. A hundred gotos jumping forward to the same cleanup/error tail is very readable and actually reduces complexity over alternatives - which it’s obviously why it’s used. In many languages this type of construct has the form of try/catch/finally but those are glorified forward gotos.

    Seeing even one goto jumping backward would be much more surprising, and seeing what made gotos get their bad rep, overlapping goto regions, would be completely shocking in any quality code base.

  • anyfoo 4 years ago

    Kernels written in C are often full of gotos. There's nothing wrong with gotos per se, it's only harmful if that's your (or one of your) primary means of control flow, e.g. because you don't have other good options. BASIC for example had that problem back in the day.

    • ArnoVW 4 years ago

      Well, many BASICs had GOSUB, which sort of gave you function calls (albeit sans parameters)

      • anyfoo 4 years ago

        Yes, but that was pretty much it: GOTO, GOSUB, and NEXT. And GOSUB is pretty much useless for building control flow equivalent to while/try/catch/... and even just if/else, or only "if" with more than one statement. NEXT works for only special cases of "for". So, most of the time, GOTO it is...

  • umvi 4 years ago

    Gotos can be used to effectively accomplish try/catch in C. As long as you stick to a safe goto idiom like this then they can be useful. In general, an inexperienced programmer taught that goto is okay is probably not going to stick to safe idioms and will instead create spaghetti code, hence why they are steered away.

    • SAI_Peregrinus 4 years ago

      I'd be more likely to use setjmp/longjmp to accomplish try/catch in C than goto. Goto is fine for error handling, but exceptions are a bit different, since they have the try/catch/finally structure. Personally I prefer to avoid exceptions, but if I were forced to implement them I'd not use goto!

  • sshine 4 years ago

    I’ll name Linux my go-to argument in this whole debate.

  • CodeGlitch 4 years ago

    Other gotos include:

    - if-statements

    - switch-statements

    - break

    - continue

    - exception handling

    The whole "gotos are bad" really needs to die.

    • throwaddzuzxd 4 years ago

      The whole point of "gotos are bad" is that they don't carry semantics. All your list is composed of semantic gotos, and are not considered bad.

    • sgerenser 4 years ago

      gotos arent’t bad, they’re just “considered harmful”

    • klyrs 4 years ago

      Dijkstra is dead and we can move on from his peeves.

      • Jtsummers 4 years ago

        If you think it was a "peeve" or that he considered go to statements universally bad, maybe you should actually read the letter he wrote (NB: The title was not his, but Wirth's) and the discussion around it.

dennis_mooreOP 4 years ago

I find it mildly entertaining to casually browse random 17-year-old code lying at the heart of the Linux kernel and encounter gems like this.

  • harrylepotter 4 years ago

    Hate to break it to you, but December 1991 was 30 years ago.

    • sli 4 years ago

      That's the original date on the file, but the comment being discussed (and much of the code in that function, it seems) was from 17 years ago. See the git-blame:

      https://github.com/torvalds/linux/blame/f4bc5bbb5fef3cf421ba...

      • pfarrell 4 years ago

        See the text of the comment: "12/12/91". The `git blame` is likely due to not having imported the previous history as commits when the original import to the git repo happened. I worked at a company that moved from VSS to TFS for a ~10 year code base. It took months for the consultants to get the import to preserve the history. Too bad this didn't happen in this repo.

        edit: Found this [0]

          Initial git repository build. I'm not bothering with the full history, even though we have it. We can create a separate "historical" git archive of that later if we want to, and in the meantime it's about 3.2GB when imported into git - space that would just make the early git days unnecessarily complicated, when we don't have a lot of good infrastructure for it.
        
          Let it rip!
        
        0: https://github.com/torvalds/linux/commit/1da177e4c3f41524e88...
        • onedognight 4 years ago

          Linus was in the middle of designing git when he wrote this. I think we can cut him some slack.

      • ajsfoux234 4 years ago

        That commit seems to include all the code written before 2005. The commit message notes that it doesn't bother including the full history.

      • gowld 4 years ago

        The comment says 12/12/91

    • quickthrower2 4 years ago

      Ha ha. Yeah Linus worked on Linux before Git unfortunately. Or fortunately maybe (because the world needed Linux before it needed git)

    • mark-r 4 years ago

      Thanks for making me feel old.

    • the_svd_doctor 4 years ago

      Yes but git-blame shows 17 years ago.

  • quickthrower2 4 years ago

    It's a weird feeling where if you work on this code, it must be like living archaeology, digging up remains, but at the same time sculpting new remains for the next generation. The other profession that must be like this is Law. In some ways this file feels more permanent than the blockchain. Although someone might come refactor it to Rust one day! If you do keep the comments at least!

  • jfk13 4 years ago

    It's a touch nicer (IMO) if you link directly to the specific lines you're referring to, like: https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3...

boricj 4 years ago

As someone who has done some OS development and sysadmin work, I can relate to that quote. The 1st edition of Unix is over 50 years old and some of the legacy cruft that has accumulated since is so sedimented I swear it's going to turn into oil any minute now.

dgellow 4 years ago

    /*
    * Samma på svenska..
    */
turdnagel 4 years ago

A meta comment on the comments on this article: as one might expect post-"goto fail"[1] there are a lot of people saying "hey this should be refactored, no goto!" I thought that at first when looking at the code, given what I remember of the Apple SSL vulnerability, and how "goto" has a smell. But, as it turns out, there are sane reasons to use goto in systems programming, especially as a kind of cleanup / finally block. TIL.

[1] https://news.ycombinator.com/item?id=7282005

  • emerged 4 years ago

    In plain c, goto is about the only sane way to do error handling in an ergonomic and easily maintainable way. The only cost is the goto stigma (which is generally justified).

flerchin 4 years ago

In a modern codebase, would there be tests for code like this? Is it too late for a plucky contributor to start adding them?

flerchin 4 years ago

I guess it's too scary to touch, but I feel like such code could be refactored to not use goto, amongst other issues.

  • Jtsummers 4 years ago

    The use of goto out (or similar) is idiomatic in a lot of kernel code. Since C has no notion of defer (like Go) or a finally block (like languages with exceptions), you place the code you want to always execute at the end, and then goto out instead of returning earlier.

    The other option, in functions like this, is to have duplicated "deferred" execution scattered throughout the function at each return point. This is a maintenance nightmare as it can be hard to discern which code is actually part of the "deferred" block (and needs to be copied) and which isn't, at least at a glance. So differences in two return points are clues that something may be wrong, but requires further investigation to determine if they should or should not be the same. As a function grows in size, this becomes increasingly problematic.

    It's actually a good demonstration of DRY.

  • jdeaton 4 years ago

    goto usage in systems programming is actually much more prevalent and less of an anti-pattern/smell than in other contexts. This is because you often need to ensure every resource acquired during the function is cleaned up along each branch that the function could exit through. Because in systems programming you'll often do an operation, check if it succeeded and exit if not. The goto resource cleanup pattern avoids you having to duplicate the clean-up-partially-acquired-resouces at every early-return statement.

  • svdree 4 years ago

    There was some discussion on this years ago on lkml, you can read it e.g. here: https://koblents.com/Ches/Links/20-Using-Goto-in-Linux-Kerne...

  • m463 4 years ago

    I've found that in critical "must work" code with lots of edge/failure cases that goto is sometimes a pretty robust solution.

    Also, apart from straight-line functions it's hard to avoid goto at the machine code level.

  • cosmolev 4 years ago

    It could be refactored and it could become less efficient afterwards.

    • Jtsummers 4 years ago

      Looking through that file, I didn't see a single goto (but I could have missed one) that didn't go to one of: unlock_out, out, or error. I don't think there would be any diminishment of runtime efficiency if you dropped the goto statements, and it may actually improve performance to drop them since they correspond to a jump that could be removed from each of their occurrences. Though it would also increase the code size (everything in that section of code has to be replicated), so that could be an issue. Tradeoffs, would have to be measured to know for sure.

      However, they do improve the maintainability of the code. I find it interesting, looking at them and having done a quick reread of Knuth's take on goto statements (for another comment I made here) that these uses correspond to what other languages possess as syntactic elements in the structured programming vein, which is one of the things he discusses in favor of both structured programming and goto statements. In favor of structured programming, you get new syntactic elements that provide useful semantics that make the code clearer. Why are we doing this jump? Oh, it's a conditional (if/then/else) or a loop (for, while). But when you lack the syntax and semantics for that, the goto statement can fill in the gaps (when used carefully, deliberately).

      Specifically, out mostly corresponds to defer (in Go) or finally in other languages. error is like try/catch/finally in Java and others. unlock_out would be like the with statements you get in some other languages.

      EDIT: Reexamined this specific source file. Most of the out sections are 0-2 statements in addition to the return. So in the 0 case, there ought to be a performance improvement by just returning, though I imagine a compiler can optimize the goto's away. In the 1-2 statement range it'd be a tossup on whether the increase in function size would cost more (cache miss and similar) than the indirection itself costs. I only saw one (it's a large file, I didn't look too closely) that had a larger out section than 2 statements, and that one had enough that the indirection is probably worth the performance cost.

      But, in all cases (except the 0-statement case) it still is better from a maintainability perspective, whether or not it helps performance. In the 0-statement case, I wonder if they're vestigial. If there were other statements included that were dropped for some reason. But I'm not going to dig further into this because now it's time for me to study.

  • rcoveson 4 years ago

    There also isn't a comment on every line, no test coverage that I can see, many functions are longer than 20 lines, and many of these source files are way too big at thousands of lines.

    Probably easier to throw the whole thing out and start again.

    • rmbyrro 4 years ago

      At first I thought you were joking, but perhaps you were serious about it?

      I can't tell and I'm literally LOL right now because of that

      • rcoveson 4 years ago

        I'm dead serious. These are CS 101 mistakes! I hope nobody is actually using this project.

        • rmbyrro 4 years ago

          > I hope nobody is actually using this project

          Definitely deserves upvoting lol

    • rmbyrro 4 years ago

      No, it must be a joke.

      > There also isn't a comment on every line

      This must be a joke LOL

      Why are folks downvoting?

      • Jtsummers 4 years ago

        Because of Poe's Law (https://en.wikipedia.org/wiki/Poe%27s_law):

        > Without a winking smiley or other blatant display of humor, it is utterly impossible to parody a Creationist in such a way that someone won't mistake for the genuine article.

        (the original phrasing)

        But it works for other things as well. When a position is known to be, or believed to be, accepted by a significant number of people, and someone expresses that position, without knowing them it's impossible to tell if it's sarcastic or sincere. Even if it's presented in a somewhat exaggerated form, there are people would still hold some of the views the GP expresses.

        • rcoveson 4 years ago

          > there are people would still hold some of the views the GP expresses.

          "People"? Try 100% of (good) CS professors. Research clearly dictates that gotos are harmful. Lack of comments is just as bad. As for the test coverage, that's just their loss. I'm sure this "linux" project would have much higher velocity (agile term, look it up) if they used test driven development.

        • rmbyrro 4 years ago

          Very interesting, this is indeed real. I'll be a more careful joker now, thanks :D

      • nieve 4 years ago

        Possibly because they're dragging the joke out way past where it might be amusing.

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection