8 ms·
This needs some heavy checking... (line 1029)
- dennis_moore 5y agoI 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.
- jfk13 5y agoIt's a touch nicer (IMO) if you link directly to the specific lines you're referring to, like: https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3485d6d383c27ec473ed/kernel/sys.c#L1028-L1038 https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3...
- harrylepotter 5y agoHate to break it to you, but December 1991 was 30 years ago.
- the_svd_doctor 5y agoYes but git-blame shows 17 years ago.
- quickthrower2 5y agoGit was released 7 April 2005, I imagine it existed and was used before v1.0 release, so 17 years checks out. It is git's age, not the code.
- gryfft 5y agoYes, but some of the history has been omitted. [1] Line 1035 includes the signoff "-TYT, 12/1291" 1. https://github.com/torvalds/linux/commit/1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 https://github.com/torvalds/linux/commit/1da177e4c3f41524e88...
- deleted 5y ago[deleted]
- mark-r 5y agoThanks for making me feel old.
- sli 5y agoThat'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/f4bc5bbb5fef3cf421ba3485d6d383c27ec473ed/kernel/sys.c#L1028 https://github.com/torvalds/linux/blame/f4bc5bbb5fef3cf421ba...
- ajsfoux234 5y agoThat commit seems to include all the code written before 2005. The commit message notes that it doesn't bother including the full history.
- gowld 5y agoThe comment says 12/12/91
- pfarrell 5y agoSee 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/1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 https://github.com/torvalds/linux/commit/1da177e4c3f41524e88...
- onedognight 5y agoLinus was in the middle of designing git when he wrote this. I think we can cut him some slack.
- quickthrower2 5y agoHa ha. Yeah Linus worked on Linux before Git unfortunately. Or fortunately maybe (because the world needed Linux before it needed git)
- quickthrower2 5y agoIt'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!
- codingkev 5y agoYou can link directly to a line by appending #<line_number> https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3485d6d383c27ec473ed/kernel/sys.c#L1029 https://github.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3...
- pan69 5y agoOr just click on the line number to change the URL to that line.
- gabrielsroka 5y agoAnd shift-click on a second line to select a block.
- whalesalad 5y agoAnother fun github hack: if you browse the repo for a particular file, the URL will be something like github.com/user/repo/blob/master/foo/bar/baz.py But this file can change, meaning this url might not have the same content a week or month down the road. This is particularly troublesome when you have lines selected. To fix this, press `y` and your url will get expanded to include the current commit hash: github.com/user/repo/blob/e4ecae.../foo/bar/baz.py Now your URL is safe to share.
- umvi 5y agoYou can also change "github.com" to "github1s.com" or "github.dev" if you want to view it in VSCode in browser. i.e. https://github.dev/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3485d6d383c27ec473ed/kernel/sys.c#L1029 https://github.dev/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3... https://github1s.com/torvalds/linux/blob/f4bc5bbb5fef3cf421ba3485d6d383c27ec473ed/kernel/sys.c#L1029 https://github1s.com/torvalds/linux/blob/f4bc5bbb5fef3cf421b...
- thunderbong 5y agoOr just press '.' if you're logged in
- 5y ago
- flerchin 5y agoI guess it's too scary to touch, but I feel like such code could be refactored to not use goto, amongst other issues.
- Jtsummers 5y agoThe 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.
- cosmolev 5y agoIt could be refactored and it could become less efficient afterwards.
- Jtsummers 5y agoLooking 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.
- lapetitejort 5y agoI 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.
- umvi 5y agoGotos 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 5y agoI'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!
- dblohm7 5y agoOne 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.
- TheSockStealer 5y agoUsed like this, it is not much different than a try-catch block
- SXX 5y agoYeah and it's make sense to use it this way since there is no try-catch in C.
- jimmygrapes 5y ago
- boricj 5y agoAs 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 5y ago/* * Samma på svenska.. */
- m463 5y ago"SAMMA PÅ SVENSKA" (english: "same in swedish")
- flerchin 5y agoIn a modern codebase, would there be tests for code like this? Is it too late for a plucky contributor to start adding them?
- pm215 5y agoIt's not in the codebase proper, but the Linux Test Project https://github.com/linux-test-project/ltp https://github.com/linux-test-project/ltp is probably a good place to start to see what's currently being tested for a syscall and to add new tests if there's a gap. https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/setpgid https://github.com/linux-test-project/ltp/tree/master/testca... is some tests of this particular syscall.
- turdnagel 5y agoA 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 https://news.ycombinator.com/item?id=7282005
- emerged 5y agoIn 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).