Hacker News Reader

Comment by cbd1984 | Hacker News Reader

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.

View on HN

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

Reply on news.ycombinator.com