Linus meltdown on a Git pull | Hacker News Reader

Linus meltdown on a Git pull

lkml.iu.edu

86 pointssigna1183 comments

Linus’s Rant Rewritten http://catcode.com/comments/2015/cf20151101.html

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.

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.

> "Consider this replacement:"

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.

Maybe. But imagine if Captain Jean-Luc Picard said it to you as a direct order. You'd probably get busy considering.

I'm a nerd, so sue me. I'd rather have the order from Captain Janeway.

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.

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.

Deleted comment

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?

That's like saying charities don't do anything because there are still homeless people. You can improve a problem without eliminating it.

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.

It appears that his rant/abuse is his MO when he has seen something he disagrees. Being polite is not in his DNA.

Imagine if all tech managers in behaves like him, I think most of us would be in other industries.

That's shit.

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?

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.

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 :-)

I sure hope no one will have to write a computer code in 2065.

There were good reasons that the code was written the way it was. Perhaps you should follow the thread further.

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.

"No problem, I'll revert it all." - direct quote from the so-called "prima donna".

Try this email http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html

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

"Prima donna"??? Did you even read the guys responses? They are as calm and collected as anything.

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.

If you think that's a meltdown, this must be the first Linus thread you've read.

Really the title should be edited to "Linus is uncharacteristically nice to a contributor."

This one is my favorite:

https://lkml.org/lkml/2012/12/23/75

That's more like it. What a prick.

Yup, I agree.

Is Mauro still contributing to the Linux kernel?

There just pulling it from this article http://thenextweb.com/dd/2015/11/02/linux-creator-linus-torv...

The response can be found here:

http://lkml.iu.edu/hypermail/linux/kernel/1510.3/03200.html

  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/

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.

That's not the point of code reviews. If you fired someone for this, then you are a remarkably bad manager.

Please don't ever manage people. Thanks.

No point in being less aggressive (compared to Linus) when doing code reviews?

I think when you hire someone the least you can expect from them is to not cuss out a co-worker for a mistake.

Ah, drat. I misread the comment. Forget what I wrote!

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.

FWIW, here's the author's response to linus's criticism:

http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html

meh, it's just another day at lkml, not a meltdown.

" Get rid of it. And I don't ever want to see that shit again.

Linus"

And this is why Linux dominates practically all computing platforms everywhere.

    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".

In short, I'm not seeing a meltdown.

Relying on the short-circuiting behavior of || and && in C is a long-accepted idiom. It's practically the purpose of those operators.

https://en.wikipedia.org/wiki/Short-circuit_evaluation#Commo...

The obscure part is relying on a side effect of the first clause in the second clause.

That is pretty common and readable for '&&', but I agree it is too subtle and obscure for '||'.

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

  ...anybody who thinks that the above is
  
  (a) legible
  (b) efficient (even with the magical compiler support)
  (c) particularly safe

  is just incompetent

If that isn't a criticism of the person, I don't know what is.

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)".

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".

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.

How much if any of this is just bike shedding?

I'm all for not pulling in useless broken code, but why the public rant?

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.

Thank you: the most comprehensive (and damn near only) comment on the page that talks about the technical decision being represented here.

Because if we're here talking about it, there's a chance people will remember to not do it next time.

Also it's cathartic.

It's not bike shredding. Development project almost always die by a thousand cuts that needs to be avoided one LOC at a time.

This is just Linus being Linus - he's like this quite a bit. He's... very opinionated.

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.

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.

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.

Are the only possible options swearing at employees or tricking them into thinking you're happy while you secretly are upset with their code quality?

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.

David wrote about it here, https://signalvnoise.com/posts/1214-profanity-works

Toxic?! He's just harsh and with a good reason. I find sugar coating way more toxic than what Linus did.

The places where you work day-to-day are of very little importance.

This is the Linux kernel, it runs the world.

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:

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.

Yeah, nothing at all personal about:

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

"Out to lunch", "bike shedding". Ironic how colloquialisms are used so freely in an industry that despises counter-productive complications.

To be clear:

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.

wtf does "bike shedding" even mean?!

http://catb.org/~esr/jargon/html/B/bikeshedding.html

https://en.wikipedia.org/wiki/Parkinson's_law_of_triviality

Or:

http://www.urbandictionary.com/define.php?term=bikeshedding

Yeah, I can google too, just kernel developers can find what the function overflow_usub does. That's not the point.

On a tangent: http://blog.dilbert.com/post/129215160011/why-trump-insults-...

Also: Linus rarely insults people, he criticizes the code, albeit harshly. http://blog.codinghorror.com/egoless-programming-you-are-not...

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.

Am I the only one who finds this fine and completely appropriate?

> sh.t ... f.cking

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.

What a horribly editorialized 'clickbait' title.

I would guess this discourages some good programmers who would have strong positive impact from trying their hand at helping out with the kernel.

Pleasant fellow

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.

it was indeed a piece of shitty code. Linus has right to protect his baby.

Time for the quarterly "Linus said mean things to someone" post already?

Reply on news.ycombinator.com