There's opinionated, and there's toxic, and I would argue that this is far more of the latter. That said, people in powerful positions will often get away with this, without many larger negative consequences. With that much power and influence comes a lot more slack in the way one behaves.
I don't have much tolerance for Linus like behavior in places that I work day-to-day though, without a lot of protective social niceties.
Strong language and mean words aren't toxic. I'd rather know the boss is pissed off than have him smile to my face while slowly hating my guts more and more. If you don't let people blow off steam and express their honest opinions people bottle it up and you end up with a nasty passive aggressive, underhanded environment where everyone is lying and that's actually toxic.
When communication is carried out over text it can be difficult to convey seriousness, intent,tone and emotion behind overly professionally sanitized HR speak. Linus' example here is marvelous in comparison and gets his point across quite well.
If Finns are anything like us Danes, they actually like to swear a lot. DHH has some of it as well, and it seems it isn't meant as bad or as rude as native english speakers understands it.
That seems like a very sheltered perspective. It is clear that Mr. Torvald used the language he did to express and support his thesis. This was not a personal attack, a lashing out, or any other sort of fit. There was a clear message: stop writing shit code. He began by explaining his definition of 'shit code' with an example. Then, he explained why we don't want it. Finally, he presented a solution. He did all of this while still conveying his conviction. Sure, we could sit around crying about the f word, or we could accept the fact that sometimes when we do shitty things to people it pisses them off. Mr. Torvald was clearly pissed off from people doing shitty things to him. In this case, trying to taint a project he's spent the last decade working on. Should he have hidden this frustration just to appeal to your PC upbringing? Absolutely not. It was refreshing to read something sincere. Toxic? That code was toxic. That code would have had real world consequences that affect millions of people. A few harsh words mean nothing in comparison.
I personally also do not like bullying and if something like this were to come from someone_I_did_not_respect , then I too would not have much tolerance for the behaviour. On the lkml however, people respect linus, and that's the reason why he gets away with it. Not because of some 'slack in the way one behaves' as you put it. Take a look at the response:
I would imagine that if you're is being flamed by linus on lkml, it is most likely because you messed up big-time. Don't expect to be reprimanded with sugar coating. More so if you are an adult/mature/experienced (in terms of contributions) person who should've known better.
...anybody who thinks that the above is
(a) legible
(b) efficient (even with the magical compiler support)
(c) particularly safe
is just incompetent and out to lunch.
The rant is about being overly fancy with the code and making it hard to read. If one scans the line, the thing that stands out first is the function being used. Not only is it a gccism that has never been used before, but it's a gccism that is used in a poor manner. By copying the "safe" subtraction to the same variable, you've now changed the behavior of the function. Before, if the mtu was too small, it would goto the error handler and not alter the mtu. The new function, if the resulting value is between 0 and 7, will overwrite mtu, match the second conditional, and be shipped off to the error handler with a new, invalid, value.
The second part of cleverness is in fact even more damning than that. The conditional || mtu <= 7 is a siren's call for anyone looking for a cheap optimization. To someone not paying attention, this looks like it could be a nice patch -- take out an or that couldn't possibly be true under any circumstance, and save a few ops on every incoming packet. Bundle it in with other optimizations, and you've got an overflow just waiting to go off.
So no, it's not bikeshedding, it's being too clever for one's own good, and in one line adds a new function call that's just barely been proposed for a standardized version of C, and has a pair of logic errors to boot. In short, ticking off pretty much everything you don't want to do in a line of code.
if (overflow_usub(mtu, hlen + sizeof(struct frag_hdr), &mtu) || mtu <= 7)
goto fail_toobig;
Any code which relies on the short-circuiting behavior of a logical operator in such an obscure way is bad code. Linus may have gone a bit overboard in his critique, but, again, it was almost all criticism of the code, not the person. The only criticism of people is him calling people who like that code "out to lunch".
Agreed. The code is abominable. I would have developed rage looking at that checkin in my project too. Although since I'm not Linus, I might have checked my rage a bit and used words like "disappointing" and "unnecessarily complex".
> Any code which relies on the short-circuiting behavior of a logical operator is bad code.
What is obscure about it? I hate to think about how even more verbose C would be without short-circuiting. Now if there had been a side effect... well, it would still be debatable - but at least you'd have a watered down cert recommendation to point to.
Negating the above statement IMO you can reasonably interpret as "you're competent so can't honestly think this is legible, efficient, or particularly safe... (fix it)".
Yes, simplicity... what a concept. Something the software engineering discipline seems to either loathe, be afraid of, or just want to avoid at any cost.
For example, why something as simple as 'git pull' has had so much confusion and discussion. Why there are millions of views on S.O. on just the differences between pull and fetch?
Something the software engineering discipline seems to either loathe, be afraid of, or just want to avoid at any cost.
I think it has to do with the fact that a lot of developers are taught that abstraction/indirection are good things (and to a certain extent, they are), and can be used to manage complexity; but not that they're more of a last-resort option compared to actually making the design as simple as possible.
Linus' rant about "helper functions" reminds me of a time when I taught an intro programming course and helped a student who, when faced with not knowing how to solve a problem, decided to take "write a helper function" as his mantra (apparently he read that somewhere in a book) and ended up with around a dozen-level-deep nested set of functions that turned out not to do anything useful at all - each one just called another and returned its result, except the deepest one, which he came to ask me about. He thought that the solution would somehow come to him more easily the more "abstract" (in his terms) the code became, and went steadfastly down that path - until common sense took over and he came to me wondering what he did and why it didn't work.
We are still in the first century of software development, I doubt any code written today will be relevant by the time we enter the second. The computer science class of 2065 will enjoy seeing all these chaotic things we made, running in virtual machines hosted by museums that are yet to be built :-)
Too weak. Uses weasel words. ie: "Consider this replacement:" Linus' version is obviously too strong for some peoples' tastes, but the rewritten version doesn't send a clear message. A lot of things can be said about Linus' typical comments, but the messaging being ambiguous is not one of them.
At his scale, any little bit helps, and crap is guaranteed to get through regardless. Can you imagine how worse it would be if he didn't lay down the law?
I've seen this sort of language creeping into discussions over the past year or two, and it feels quite strange. Lots of phrasing statements of fact as questions, etc. I'm not sure what this accomplishes -- it certainly makes the heart of many technical discussions harder to get to.
Those aren't weasel words, that's direct feedback.
The rewritten message clearly indicates a rejection of the submitted code, so directing the submitter to consider the example replacement seems a constructive and appropriate response.
The rewritten version neglects the criticism that the submitted code is too fiddly relying on the short-circuit of the || operator, but Linus failed to mention that as well :p
I disagree. "Consider this replacement" is practically an invitation to debate; or worse, possibly misunderstood as "I agree your way works, but I like my way better, so let me be capricious about it and reject your PR because I can." Another poster re-rewrote it in a way that I feel is at least unambiguous.
There has already been some comments on that message. Unless you are saying Linus is always correct? Or should people be fearful and not want to say anything at all to him? Looks a bit that way.
Even if you don't consider them to be weasel words, it loses all emotional dynamic range that conveys the relative seriousness of the issue. When Linus is already limited in his communication with contributers by the nature of email, how does he separate minor problems from major ones? That rewritten post reads like it could be for a small stylistic mistake of relative unimportance. These "Linus rants" come up rarely enough that we're able to flag what is important to him (such as don't break user-space) from the thousands of other disagreements he has with contributers.
Took out the hedging words. It could probably still be made more firm and remain civil.
-------------
Linus’s Rant RE-Rewritten
This is the old code in net/ipv6/ip6_output.c:
mtu -= hlen + sizeof(struct frag_hdr);
and this your replacement:
if (overflow_usub(mtu, hlen + sizeof(struct frag_hdr), &mtu) || mtu <= 7)
goto fail_toobig;
The problem is that the overflow_usub() function is non-standard and requires special compiler support to generate reasonably efficient code. That function hasn't been used anywhere else in the code base before. Also, the code uses two conditionals. Here is how it should have been written:
if (mtu < hlen + sizeof(struct frag_hdr) + 8)
goto fail_toobig;
mtu -= hlen + sizeof(struct frag_hdr);
It takes the same number of lines, doesn't need a little-known helper function, is clearer in its intent and is easier to read. There could still be overflow issues if the "hlen + xyz" expression overflows, but the "overflow_usub()" code has that same problem.
Also, we are near to rc7 time, and we don't want conflicts at this point.
Thank you for submitting the code, but, for all the aforementioned reasons, I have to reject it and will do so for similar code in the future.
I feel like the rewrite, in an attempt to succinct, completely misses the point of Linus' rant. I might be reading too much into it, but its less about "using nonstandard functions" and more about "people thinking they're smarter than everyone else and submitting esoteric and/or obscure code."
The rewrite should make better effort to translate
>and a shiny new nonstandard function that wants particular compiler support to generate even half-way sane code, and even then generates worse code? A shiny function that we have never ever needed anywhere else, and that is just compiler-masturbation.
I'm not a kernel developer, but I'm sure others can sympathize with seeing code that was submitted that performs worse, but was only added because it uses some shiny new framework/tool/library/technique they read on a blog.
Here's the problem - NO-ONE WILL FUCKING READ THAT. (Sorry, just swearing to grab attention).
Linus isn't just talking to the submitter. He knows perfectly well that his rants will be read by everyone in the community. A polite "plz fix it" will go unnoticed.
I'm not sure whether there's some common understanding that his rants are just an act, to get everyone to read them. Or if Linus thinks that it's actually justified to be a dick when people fuck up. But I think it's a factor - when Linus rants, everyone pays attention, and Linus will certainly be aware of that.
Yes, it would be nice if people paid attention to him being polite. But if he wrote it like the rewritten suggestion, only a handful of people would read it, and they're probably not the the type of people who are making the mistakes to begin with.
If people ask Linus to do anything, ask him to politely reach out to the people he flames, and let them know that the reason for his grandstanding is mostly just to grab eyeballs. It raises an important issue, and while he may be annoyed at the bad submit the level of vitriol is only really there to grab attention.
Seriously? I can understand if people do it a few times and ignore him, but on the first occasion someone makes an attempt at submitting some code they honestly believed would be helpful and he craps on them from a great height? Charming.
Don't call it a meltdown for goodness' sake. Linus only gives this treatment to the prima donnas at his level to keep them in place. It may sound impolite to say so, but some of them act as if they're God's gift to Earth.
Linus is doing what he needs to do to keep everyone in their place. Some of these programmers don't like taking no for an answer. I've been that guy before. I've been a real butthead to people with my code. So I respect what Linus is doing. Because I had it coming, and so did the recipient of this rant.
I too, have been a pompous ass before (probably still am from time to time), but I'm much happier with the things I say in public after having had it pointed out to me a couple of times early on.
The title of this post is editorialized. I do think the topic is worthy of discussion though, so I upvoted it.
In spite of the tone and language, Torvalds manages to keep the issue fairly impersonal (The reference to 'braindamage' sort of crosses the line). Nowhere does he specifically call any specific individual out, even though he easily could with git blame on the specific line as well as grabbing the author, committer(s), and reviewers (individuals responsible for signing off the commit) information from git log.
Still, if my code received that sort of treatment, I would probably be in tears.
> if my code went that sort of treatment, I would probably be in tears.
I think the reason Linus feels justified in being an asshole is that he feels you can't accidentally write code this way. Rather, he thinks it's someone trying to make themselves look smart at the expense of the project, which is itself assholish behavior. I'm not saying he's right, but if you look at it from his perspective it's at least logically consistent.
Asserting dominance with aggressive behavior is fine I guess (with in boundaries) - it makes calling out someone much more powerful but at the same time makes you look like a tool if you're wrong and you lose respect faster - double edged sword.
If I maintained a large project and saw a pull request like that I'd lose my cool as well. Linus clearly needs better and more thorough subsystem maintainers so code like that gets corrected (or rejected) earlier.
Linus is really incompatible with the way Software Engineering (safe spaces, toxic, problematic) is going, it is going to be a very enjoyable show to see the two sides butt heads constantly as they disagree with how he deal with people.
What leads you to believe that it's actually going that way? A couple high profile winers with too many twitter followers and no horse in the race does not a trend make.
If anyone wrote a code review this way at a workplace, they would likely be reprimanded, do it again and likely fired. I'm not sure why we can't expect basic levels of professionalism out of project maintainers, much less decency.
From: Rasmus Villemoes
Date: Wed Oct 28 2015 - 10:30:41 EST
On Wed, Oct 28 2015, Hannes Frederic Sowa <hannes@xxxxxxxxxxxxxxxxxxx> wrote:
> Hi Linus,
>
> On Wed, Oct 28, 2015, at 10:39, Linus Torvalds wrote:
>> Get rid of it. And I don't *ever* want to see that shit again.
>
> I don't want to give up on that this easily:
>
> In future I would like to see an interface like this. It is often hard
> to do correct overflow/wrap-around tests and it would be great if there
> are helper functions which could easily and without a lot of thinking be
> used by people to remove those problems from the kernel.
I agree - proper overflow checking can be really hard. Quick, assuming a
and b have the same unsigned integer type, is 'a+b<a' sufficient to
check overflow? Of course not (hint: promotion rules). And as you say,
it gets even more complicated for signed types.
A few months ago I tried posting a complete set of fallbacks for older
compilers (https://lkml.org/lkml/2015/7/19/358), but nothing really
happened. Now I know where Linus stands, so I guess I can just delete
that branch.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
To be fair, Linus is the boss. He is the one who controls the Linux kernel development and just like at any firm, if the employees aren't doing a good job, they get shouted at. The difference here being that Linus' rants are in the public because of lkml while the scalding meted out by other bosses remain in the confines of their respective offices. Just another day at work here.
92 comments
[ 0.16 ms ] story [ 137 ms ] threadI'm all for not pulling in useless broken code, but why the public rant?
I don't have much tolerance for Linus like behavior in places that I work day-to-day though, without a lot of protective social niceties.
David wrote about it here, https://signalvnoise.com/posts/1214-profanity-works
This is the Linux kernel, it runs the world.
http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html
I would imagine that if you're is being flamed by linus on lkml, it is most likely because you messed up big-time. Don't expect to be reprimanded with sugar coating. More so if you are an adult/mature/experienced (in terms of contributions) person who should've known better.
Also it's cathartic.
I wasn't immediately sure if it's Linus doing the bike shedding- In fact it appears to me more like the PR is the culprit.
https://en.wikipedia.org/wiki/Parkinson's_law_of_triviality
Or:
http://www.urbandictionary.com/define.php?term=bikeshedding
The second part of cleverness is in fact even more damning than that. The conditional || mtu <= 7 is a siren's call for anyone looking for a cheap optimization. To someone not paying attention, this looks like it could be a nice patch -- take out an or that couldn't possibly be true under any circumstance, and save a few ops on every incoming packet. Bundle it in with other optimizations, and you've got an overflow just waiting to go off.
So no, it's not bikeshedding, it's being too clever for one's own good, and in one line adds a new function call that's just barely been proposed for a standardized version of C, and has a pair of logic errors to boot. In short, ticking off pretty much everything you don't want to do in a line of code.
In short, I'm not seeing a meltdown.
https://en.wikipedia.org/wiki/Short-circuit_evaluation#Commo...
What is obscure about it? I hate to think about how even more verbose C would be without short-circuiting. Now if there had been a side effect... well, it would still be debatable - but at least you'd have a watered down cert recommendation to point to.
https://lkml.org/lkml/2012/12/23/75
Is Mauro still contributing to the Linux kernel?
For example, why something as simple as 'git pull' has had so much confusion and discussion. Why there are millions of views on S.O. on just the differences between pull and fetch?
http://stackoverflow.com/questions/292357/what-are-the-diffe... ==> Viewed: 1080104 times
Good code doesn't just pass unit tests. It should be know-able, think-able, make some level of sense and generally be easily understandable.
I think it has to do with the fact that a lot of developers are taught that abstraction/indirection are good things (and to a certain extent, they are), and can be used to manage complexity; but not that they're more of a last-resort option compared to actually making the design as simple as possible.
Linus' rant about "helper functions" reminds me of a time when I taught an intro programming course and helped a student who, when faced with not knowing how to solve a problem, decided to take "write a helper function" as his mantra (apparently he read that somewhere in a book) and ended up with around a dozen-level-deep nested set of functions that turned out not to do anything useful at all - each one just called another and returned its result, except the deepest one, which he came to ask me about. He thought that the solution would somehow come to him more easily the more "abstract" (in his terms) the code became, and went steadfastly down that path - until common sense took over and he came to me wondering what he did and why it didn't work.
Those aren't weasel words, that's direct feedback.
The rewritten message clearly indicates a rejection of the submitted code, so directing the submitter to consider the example replacement seems a constructive and appropriate response.
The rewritten version neglects the criticism that the submitted code is too fiddly relying on the short-circuit of the || operator, but Linus failed to mention that as well :p
-------------
Linus’s Rant RE-Rewritten
This is the old code in net/ipv6/ip6_output.c:
and this your replacement: The problem is that the overflow_usub() function is non-standard and requires special compiler support to generate reasonably efficient code. That function hasn't been used anywhere else in the code base before. Also, the code uses two conditionals. Here is how it should have been written: It takes the same number of lines, doesn't need a little-known helper function, is clearer in its intent and is easier to read. There could still be overflow issues if the "hlen + xyz" expression overflows, but the "overflow_usub()" code has that same problem.Also, we are near to rc7 time, and we don't want conflicts at this point.
Thank you for submitting the code, but, for all the aforementioned reasons, I have to reject it and will do so for similar code in the future.
The rewrite should make better effort to translate
>and a shiny new nonstandard function that wants particular compiler support to generate even half-way sane code, and even then generates worse code? A shiny function that we have never ever needed anywhere else, and that is just compiler-masturbation.
I'm not a kernel developer, but I'm sure others can sympathize with seeing code that was submitted that performs worse, but was only added because it uses some shiny new framework/tool/library/technique they read on a blog.
Linus isn't just talking to the submitter. He knows perfectly well that his rants will be read by everyone in the community. A polite "plz fix it" will go unnoticed.
I'm not sure whether there's some common understanding that his rants are just an act, to get everyone to read them. Or if Linus thinks that it's actually justified to be a dick when people fuck up. But I think it's a factor - when Linus rants, everyone pays attention, and Linus will certainly be aware of that.
Yes, it would be nice if people paid attention to him being polite. But if he wrote it like the rewritten suggestion, only a handful of people would read it, and they're probably not the the type of people who are making the mistakes to begin with.
If people ask Linus to do anything, ask him to politely reach out to the people he flames, and let them know that the reason for his grandstanding is mostly just to grab eyeballs. It raises an important issue, and while he may be annoyed at the bad submit the level of vitriol is only really there to grab attention.
Imagine if all tech managers in behaves like him, I think most of us would be in other industries.
Linus is doing what he needs to do to keep everyone in their place. Some of these programmers don't like taking no for an answer. I've been that guy before. I've been a real butthead to people with my code. So I respect what Linus is doing. Because I had it coming, and so did the recipient of this rant.
>> Get rid of it. And I don't ever want to see that shit again.
> I don't want to give up on that this easily
In spite of the tone and language, Torvalds manages to keep the issue fairly impersonal (The reference to 'braindamage' sort of crosses the line). Nowhere does he specifically call any specific individual out, even though he easily could with git blame on the specific line as well as grabbing the author, committer(s), and reviewers (individuals responsible for signing off the commit) information from git log.
Still, if my code received that sort of treatment, I would probably be in tears.
I think the reason Linus feels justified in being an asshole is that he feels you can't accidentally write code this way. Rather, he thinks it's someone trying to make themselves look smart at the expense of the project, which is itself assholish behavior. I'm not saying he's right, but if you look at it from his perspective it's at least logically consistent.
Self censoring the swear words - how civil :D
Asserting dominance with aggressive behavior is fine I guess (with in boundaries) - it makes calling out someone much more powerful but at the same time makes you look like a tool if you're wrong and you lose respect faster - double edged sword.
Linus"
And this is why Linux dominates practically all computing platforms everywhere.
http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html
meh, it's just another day at lkml, not a meltdown.
Please don't ever manage people. Thanks.
I think when you hire someone the least you can expect from them is to not cuss out a co-worker for a mistake.
http://lkml.iu.edu/hypermail/linux/kernel/1510.3/03200.html
Also: Linus rarely insults people, he criticizes the code, albeit harshly. http://blog.codinghorror.com/egoless-programming-you-are-not...