Linus 又发怒了，针对丑陋的代码

最新推荐文章于 2022-04-27 10:47:27 发布

chiyikou1762

最新推荐文章于 2022-04-27 10:47:27 发布

阅读量151

点赞数

文章标签：网络

原文链接：https://my.oschina.net/jingxia/blog/697628

版权

Linus 又发怒了，针对丑陋的代码

Christ people. This is just sh*t.

看来大神这次怒的不轻。我们来看看令大神愤怒的代码是什么样的，事情的起因是一位 net 开发者，提交一系列关于内核 Networking 的补丁：

This may look a bit scary this late in the release cycle, but as is typically
the case it’s predominantly small driver fixes all over the place.
1) Fix two regressions in ipv6 route lookups, particularly wrt. output
interface specifications in the lookup key. From David Ahern.
2) Fix checks in ipv6 IPSEC tunnel pre-encap fragmentation, from
Herbert Xu.
3) Fix mis-advertisement of 1000BASE-T on bcm63xx_enet, from Simon
Arlott.
4) Some smsc phys misbehave with energy detect mode enabled, so add a
DT property and disable it on such switches. From Heiko Schocher.
5) Fix TSO corruption on TX in mv643xx_eth, from Philipp Kirchhofer.
6) Fix regression added by removal of openvswitch vport stats, from
James Morse.
7) Vendor Kconfig options should be bool, not tristate, from Andreas
Schwab.
8) Use non-_BH() net stats bump in tcp_xmit_probe_skb(), otherwise
we barf during TCP REPAIR operations.
9) Fix various bugs in openvswitch conntrack support, from Joe
Stringer.
10) Fix NETLINK_LIST_MEMBERSHIPS locking, from David Herrmann.
11) Don’t have VSOCK do sock_put() in interrupt context, from Jorgen
Hansen.
12) Fix skb_realloc_headroom() failures properly in ISDN, from Karsten
Keil.
13) Add some device IDs to qmi_wwan, from Bjorn Mork.
14) Fix ovs egress tunnel information when using lwtunnel devices,
from Pravin B Shelar.
15) Add missing NETIF_F_FRAGLIST to macvtab feature list, from Jason
Wang.
16) Fix incorrect handling of throw routes when the result of the
throw cannot find a match, from Xin Long.
17) Protect ipv6 MTU calculations from wrap-around, from Hannes
Frederic Sowa.
18) Fix failed autonegotiation on KSZ9031 micrel PHYs, from Nathan
Sullivan.
19) Add missing memory barries in descriptor accesses or xgbe driver,
from Thomas Lendacky.
20) Fix release conditon test in pppoe_release(), from Guillaume Nault.
21) Fix gianfar bugs wrt. filter configuration, from Claudiu Manoil.
22) Fix violations of RX buffer alignment in sh_eth driver, from Sergei
Shtylyov.
23) Fixing missing of_node_put() calls in various places around the
networking, from Julia Lawall.
24) Fix incorrect leaf now walking in ipv4 routing tree, from Alexander
Duyck.
25) RDS doesn’t check pskb_pull()/pskb_trim() return values, from
Sowmini Varadhan.
26) Fix VLAN configuration in mlx4 driver, from Jack Morgenstein.
Please pull, thanks a lot.
The following changes since commit 1099f86044111e9a7807f09523e42d4c9d0fb781:

非常大的修改，可以说作者还是很用心，这里不得不为作者感到一丝委屈。这其中令大神愤怒的部分来自：

overflow-arith: begin to add support for overflow builtins functions
I add a new header, linux/overflow-arith.h, as the central place to add
overflow and wrap-around checking functions. The reason I am doing so
is that it can make use of compiler supported builtin functions which
can leverage hardware.
As I need this for a fix in the ipv6 stack, which is also included in
this series, I propose to add it sooner than later over Davem’s net
tree. This is also the reason why I start slowly with only the one
function I need at this time.

 #include <linux/errno.h>
 #include <linux/kernel.h>
+#include <linux/overflow-arith.h>
 #include <linux/string.h>
 #include <linux/socket.h>
 #include <linux/net.h>
 @@ -584,7 +585,10 @@ int ip6_fragment(struct sock *sk, struct sk_buff *skb,
        if (np->frag_size)
            mtu = np->frag_size;
    }
-   mtu -= hlen + sizeof(struct frag_hdr);
+
+   if (overflow_usub(mtu, hlen + sizeof(struct frag_hdr), &mtu) ||
+       mtu <= 7)
+       goto fail_toobig;

    frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
                    &ipv6_hdr(skb)->saddr);enter code here

定义了一个他认为可以让编译器进行优化的 inline 函数：

+#pragma once
+
+#include <linux/kernel.h>
+
+#ifdef CC_HAVE_BUILTIN_OVERFLOW
+
+#define overflow_usub __builtin_usub_overflow
+
+#else
+
+static inline bool overflow_usub(unsigned int a, unsigned int b,
+                unsigned int *res)
+{
+   *res = a - b;
+   return *res > a ? true : false;
+}
+
+#endif

然后 Linus 大神不同意了：

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

The above code is sh*t, and it generates shit code. It looks >bad, and
there’s no reason for it.

大神认为应该这样写，更清晰更易懂：

Why is this not

if (mtu < hlen + sizeof(struct frag_hdr) + 8)
goto fail_toobig;
mtu -= hlen + sizeof(struct frag_hdr);

which is the same number of lines, doesn’t use crazy helper functions
that nobody knows what they do, and is much more obvious >what it actually does.

大神真的很愤怒最后丢下一句话，走了：

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

然后我们就在内核代码中看到了这个提交：

commit 1e0d69a9cc9172d7896c2113f983a74f6e8ff303
Author: Hannes Frederic Sowa hannes@stressinduktion.org
Date: Wed Oct 28 13:21:03 2015 +0100

Revert "Merge branch 'ipv6-overflow-arith'"

Linus dislikes these changes. To not hold up the net-merge let's revert
it for now and fix the bug like Linus suggested.

This reverts commit ec3661b42257d9a06cf0d318175623ac7a660113, reversing
changes made to c80dbe04612986fd6104b4a1be21681b113b5ac9.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

看到这里我们为这些认真的人点赞。最后来个彩蛋，如何机制的向内核提交补丁，还是跟我们中国的程序员学习吧：

commit f6b8dec99865ea906150e963eacbfd037b579ee9
Author: Li RongQing <roy.qing.li@gmail.com>
Date:   Thu Oct 22 11:35:05 2015 +0800

    af_key: fix two typos

    Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 83a7068..f9c9ecb 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -261,7 +261,7 @@ static int pfkey_broadcast(struct sk_buff *skb,

                err2 = pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk);

-               /* Error is cleare after succecful sending to at least one
+               /* Error is cleared after successful sending to at least one
                 * registered KM */
                if ((broadcast_flags & BROADCAST_REGISTERED) && err)
                        err = err2;

转载于:https://my.oschina.net/jingxia/blog/697628