
Linus meltdown on a Git pull - signa11
http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02866.html
======
Maxious
Linus’s Rant Rewritten
[http://catcode.com/comments/2015/cf20151101.html](http://catcode.com/comments/2015/cf20151101.html)

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

~~~
oxryly1
> "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

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

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

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

------
Jupe
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...](http://stackoverflow.com/questions/292357/what-are-the-differences-
between-git-pull-and-git-fetch) ==> 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.

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

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

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

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

~~~
striking
Try this email
[http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html](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

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

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

------
chris_wot
The response can be found here:

[http://lkml.iu.edu/hypermail/linux/kernel/1510.3/03200.html](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/

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

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

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

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

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

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

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

[http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html](http://lkml.iu.edu/hypermail/linux/kernel/1510.3/02919.html)

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

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

------
cbd1984

        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.

~~~
tobinfricke
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...](https://en.wikipedia.org/wiki/Short-
circuit_evaluation#Common_usage)

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

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

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

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

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

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

------
DoubleMalt
On a tangent: [http://blog.dilbert.com/post/129215160011/why-trump-
insults-...](http://blog.dilbert.com/post/129215160011/why-trump-insults-
people-part-of-my-trump)

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

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

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

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

------
Jerry2
What a horribly editorialized 'clickbait' title.

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

------
kyriakos
Pleasant fellow

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

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

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

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

