Hacker News new | ask | show | jobs
by ta0o0o0 3882 days ago
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.