6 ms·

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

by cbd1984 11y ago

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.

chetanahuja 11y ago
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".
tobinfricke 11y ago
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#Common_usage https://en.wikipedia.org/wiki/Short-circuit_evaluation#Commo...
- damienkatz 11y ago
  The obscure part is relying on a side effect of the first clause in the second clause.
  - haberman 11y ago
    That is pretty common and readable for '&&', but I agree it is too subtle and obscure for '||'.
woodman 11y ago
> 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.
deleted 11y ago
[deleted]
tathastu 11y ago
...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.
- hayd 11y ago
  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)".

by cbd1984 11y ago

chetanahuja 11y ago

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

tobinfricke 11y ago

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#Common_usage https://en.wikipedia.org/wiki/Short-circuit_evaluation#Commo...

damienkatz 11y ago
The obscure part is relying on a side effect of the first clause in the second clause.
- haberman 11y ago
  That is pretty common and readable for '&&', but I agree it is too subtle and obscure for '||'.

woodman 11y ago

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

deleted 11y ago

[deleted]

tathastu 11y ago

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

hayd 11y ago
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)".