[PATCH 1/9 net-next] ipv4: replace if/BUG by BUG_ON

[PATCH 1/9 net-next] ipv4: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv4/devinet.c       | 3 +--
 net/ipv4/esp4.c          | 3 +--
 net/ipv4/icmp.c          | 3 +--
 net/ipv4/ip_output.c     | 4 ++--
 net/ipv4/ping.c          | 3 +--
 net/ipv4/tcp_input.c     | 8 ++++----
 net/ipv4/tcp_minisocks.c | 4 ++--
 net/ipv4/tcp_output.c    | 3 +--
 8 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 975ee5e..e4b53f7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1715,8 +1715,7 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla)
 	if (!in_dev)
 		return -EAFNOSUPPORT;
 
-	if (nla_parse_nested(tb, IFLA_INET_MAX, nla, NULL) < 0)
-		BUG();
+	BUG_ON(nla_parse_nested(tb, IFLA_INET_MAX, nla, NULL) < 0);
 
 	if (tb[IFLA_INET_CONF]) {
 		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 60173d4..02d41d0 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -289,8 +289,7 @@ static int esp_input_done2(struct sk_buff *skb, int err)
 	if (unlikely(err))
 		goto out;
 
-	if (skb_copy_bits(skb, skb->len-alen-2, nexthdr, 2))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2));
 
 	err = -EINVAL;
 	padlen = nexthdr[0];
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 5e56401..6d59e26 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -922,8 +922,7 @@ static bool icmp_timestamp(struct sk_buff *skb)
 	icmp_param.data.times[1] = htonl((tv.tv_sec % 86400) * MSEC_PER_SEC +
 					 tv.tv_nsec / NSEC_PER_MSEC);
 	icmp_param.data.times[2] = icmp_param.data.times[1];
-	if (skb_copy_bits(skb, 0, &icmp_param.data.times[0], 4))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, 0, &icmp_param.data.times[0], 4));
 	icmp_param.data.icmph	   = *icmp_hdr(skb);
 	icmp_param.data.icmph.type = ICMP_TIMESTAMPREPLY;
 	icmp_param.data.icmph.code = 0;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 8259e77..f58f52b 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -695,8 +695,8 @@ slow_path:
 		/*
 		 *	Copy a block of the IP datagram.
 		 */
-		if (skb_copy_bits(skb, ptr, skb_transport_header(skb2), len))
-			BUG();
+		BUG_ON(skb_copy_bits(skb, ptr, skb_transport_header(skb2),
+				     len));
 		left -= len;
 
 		/*
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 344e7cd..5536c2b 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -608,8 +608,7 @@ int ping_getfrag(void *from, char *to,
 
 	if (offset == 0) {
 		fraglen -= sizeof(struct icmphdr);
-		if (fraglen < 0)
-			BUG();
+		BUG_ON(fraglen < 0);
 		if (csum_and_copy_from_iter(to + sizeof(struct icmphdr),
 			    fraglen, &pfh->wcheck,
 			    &pfh->msg->msg_iter) != fraglen)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 023196f..7ae07a4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4642,8 +4642,9 @@ restart:
 			BUG_ON(offset < 0);
 			if (size > 0) {
 				size = min(copy, size);
-				if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
-					BUG();
+				BUG_ON(skb_copy_bits(skb, offset,
+						     skb_put(nskb, size),
+						     size));
 				TCP_SKB_CB(nskb)->end_seq += size;
 				copy -= size;
 				start += size;
@@ -4966,8 +4967,7 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *t
 		/* Is the urgent pointer pointing into this packet? */
 		if (ptr < skb->len) {
 			u8 tmp;
-			if (skb_copy_bits(skb, ptr, &tmp, 1))
-				BUG();
+			BUG_ON(skb_copy_bits(skb, ptr, &tmp, 1));
 			tp->urg_data = TCP_URG_VALID | tmp;
 			if (!sock_flag(sk, SOCK_DEAD))
 				sk->sk_data_ready(sk);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 274e96f..90e5c9f 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -334,8 +334,8 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
 			key = tp->af_specific->md5_lookup(sk, sk);
 			if (key != NULL) {
 				tcptw->tw_md5_key = kmemdup(key, sizeof(*key), GFP_ATOMIC);
-				if (tcptw->tw_md5_key && !tcp_alloc_md5sig_pool())
-					BUG();
+				BUG_ON(tcptw->tw_md5_key &&
+				       !tcp_alloc_md5sig_pool());
 			}
 		} while (0);
 #endif
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 2e69b8d..aa78fc2 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2564,8 +2564,7 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
 		return -EBUSY;
 
 	if (before(TCP_SKB_CB(skb)->seq, tp->snd_una)) {
-		if (before(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
-			BUG();
+		BUG_ON(before(TCP_SKB_CB(skb)->end_seq, tp->snd_una));
 		if (tcp_trim_head(sk, skb, tp->snd_una - TCP_SKB_CB(skb)->seq))
 			return -ENOMEM;
 	}
-- 
1.9.1

[PATCH 2/9 net-next] net: core: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/core/rtnetlink.c |  3 +--
 net/core/skbuff.c    | 15 ++++++---------
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b96ac21..ed496d3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1770,8 +1770,7 @@ static int do_setlink(const struct sk_buff *skb,
 		nla_for_each_nested(af, tb[IFLA_AF_SPEC], rem) {
 			const struct rtnl_af_ops *af_ops;
 
-			if (!(af_ops = rtnl_af_lookup(nla_type(af))))
-				BUG();
+			BUG_ON(!(af_ops = rtnl_af_lookup(nla_type(af))));
 
 			err = af_ops->set_link_af(dev, af);
 			if (err < 0)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cdb939b..704ba4d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1056,8 +1056,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
 	/* Set the tail pointer and length */
 	skb_put(n, skb->len);
 
-	if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
 
 	copy_skb_header(n, skb);
 	return n;
@@ -1154,8 +1153,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 
 	BUG_ON(nhead < 0);
 
-	if (skb_shared(skb))
-		BUG();
+	BUG_ON(skb_shared(skb));
 
 	size = SKB_DATA_ALIGN(size);
 
@@ -1289,9 +1287,8 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
 		head_copy_off = newheadroom - head_copy_len;
 
 	/* Copy the linear header and data. */
-	if (skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
-			  skb->len + head_copy_len))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
+			     skb->len + head_copy_len));
 
 	copy_skb_header(n, skb);
 
@@ -1567,8 +1564,8 @@ unsigned char *__pskb_pull_tail(struct sk_buff *skb, int delta)
 			return NULL;
 	}
 
-	if (skb_copy_bits(skb, skb_headlen(skb), skb_tail_pointer(skb), delta))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, skb_headlen(skb), skb_tail_pointer(skb),
+			     delta));
 
 	/* Optimization: no fragments, no reasons to preestimate
 	 * size of pulled pages. Superb.
-- 
1.9.1

[PATCH 3/9 net-next] af_key: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/key/af_key.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9255fd9..c77dcc9 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -889,8 +889,7 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
 		pfkey_sockaddr_fill(&x->props.saddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	/* dst address */
 	addr = (struct sadb_address*) skb_put(skb,
@@ -906,8 +905,7 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x,
 		pfkey_sockaddr_fill(&x->id.daddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	if (!xfrm_addr_equal(&x->sel.saddr, &x->props.saddr,
 			     x->props.family)) {
@@ -2030,11 +2028,8 @@ static int pfkey_xfrm_policy2msg(struct sk_buff *skb, const struct xfrm_policy *
 	addr->sadb_address_proto = pfkey_proto_from_xfrm(xp->selector.proto);
 	addr->sadb_address_prefixlen = xp->selector.prefixlen_s;
 	addr->sadb_address_reserved = 0;
-	if (!pfkey_sockaddr_fill(&xp->selector.saddr,
-				 xp->selector.sport,
-				 (struct sockaddr *) (addr + 1),
-				 xp->family))
-		BUG();
+	BUG_ON(!pfkey_sockaddr_fill(&xp->selector.saddr, xp->selector.sport,
+				    (struct sockaddr *)(addr + 1), xp->family));
 
 	/* dst address */
 	addr = (struct sadb_address*) skb_put(skb,
@@ -3133,8 +3128,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 		pfkey_sockaddr_fill(&x->props.saddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	/* dst address */
 	addr = (struct sadb_address*) skb_put(skb,
@@ -3149,8 +3143,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 		pfkey_sockaddr_fill(&x->id.daddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	pol = (struct sadb_x_policy *)  skb_put(skb, sizeof(struct sadb_x_policy));
 	pol->sadb_x_policy_len = sizeof(struct sadb_x_policy)/sizeof(uint64_t);
@@ -3346,8 +3339,7 @@ static int pfkey_send_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr,
 		pfkey_sockaddr_fill(&x->props.saddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	/* NAT_T_SPORT (old port) */
 	n_port = (struct sadb_x_nat_t_port*) skb_put(skb, sizeof (*n_port));
@@ -3369,8 +3361,7 @@ static int pfkey_send_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr,
 		pfkey_sockaddr_fill(ipaddr, 0,
 				    (struct sockaddr *) (addr + 1),
 				    x->props.family);
-	if (!addr->sadb_address_prefixlen)
-		BUG();
+	BUG_ON(!addr->sadb_address_prefixlen);
 
 	/* NAT_T_DPORT (new port) */
 	n_port = (struct sadb_x_nat_t_port*) skb_put(skb, sizeof (*n_port));
-- 
1.9.1

[PATCH 4/9 net-next] ipv6: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv6/addrconf.c                     | 3 +--
 net/ipv6/esp6.c                         | 3 +--
 net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2660263..e205918 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4805,8 +4805,7 @@ static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
 	if (!idev)
 		return -EAFNOSUPPORT;
 
-	if (nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0)
-		BUG();
+	BUG_ON(nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0);
 
 	if (tb[IFLA_INET6_TOKEN]) {
 		err = inet6_set_iftoken(idev, nla_data(tb[IFLA_INET6_TOKEN]));
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index e48f2c7..9e51b69 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -280,8 +280,7 @@ static int esp_input_done2(struct sk_buff *skb, int err)
 	if (unlikely(err))
 		goto out;
 
-	if (skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2))
-		BUG();
+	BUG_ON(skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2));
 
 	err = -EINVAL;
 	padlen = nexthdr[0];
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 6f187c8..cea1a4a 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -538,8 +538,7 @@ find_prev_fhdr(struct sk_buff *skb, u8 *prevhdrp, int *prevhoff, int *fhoff)
 			pr_debug("too short\n");
 			return -1;
 		}
-		if (skb_copy_bits(skb, start, &hdr, sizeof(hdr)))
-			BUG();
+		BUG_ON(skb_copy_bits(skb, start, &hdr, sizeof(hdr)));
 		if (nexthdr == NEXTHDR_AUTH)
 			hdrlen = (hdr.hdrlen+2)<<2;
 		else
-- 
1.9.1

[PATCH 5/9 net-next] xfrm: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/xfrm/xfrm_policy.c |  5 ++---
 net/xfrm/xfrm_user.c   | 27 +++++++++------------------
 2 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 638af06..15de248 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -306,9 +306,8 @@ EXPORT_SYMBOL(xfrm_policy_alloc);
 void xfrm_policy_destroy(struct xfrm_policy *policy)
 {
 	BUG_ON(!policy->walk.dead);
-
-	if (del_timer(&policy->timer) || del_timer(&policy->polq.hold_timer))
-		BUG();
+	BUG_ON(del_timer(&policy->timer) ||
+	       del_timer(&policy->polq.hold_timer));
 
 	security_xfrm_policy_free(policy->security);
 	kfree(policy);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 7de2ed9..b2ef371 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1081,8 +1081,7 @@ static int xfrm_get_spdinfo(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (r_skb == NULL)
 		return -ENOMEM;
 
-	if (build_spdinfo(r_skb, net, sportid, seq, *flags) < 0)
-		BUG();
+	BUG_ON(build_spdinfo(r_skb, net, sportid, seq, *flags) < 0);
 
 	return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid);
 }
@@ -1139,8 +1138,7 @@ static int xfrm_get_sadinfo(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (r_skb == NULL)
 		return -ENOMEM;
 
-	if (build_sadinfo(r_skb, net, sportid, seq, *flags) < 0)
-		BUG();
+	BUG_ON(build_sadinfo(r_skb, net, sportid, seq, *flags) < 0);
 
 	return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid);
 }
@@ -1887,8 +1885,7 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	c.seq = nlh->nlmsg_seq;
 	c.portid = nlh->nlmsg_pid;
 
-	if (build_aevent(r_skb, x, &c) < 0)
-		BUG();
+	BUG_ON(build_aevent(r_skb, x, &c) < 0);
 	err = nlmsg_unicast(net->xfrm.nlsk, r_skb, NETLINK_CB(skb).portid);
 	spin_unlock_bh(&x->lock);
 	xfrm_state_put(x);
@@ -2305,8 +2302,7 @@ static int xfrm_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		return -ENOMEM;
 
 	/* build migrate */
-	if (build_migrate(skb, m, num_migrate, k, sel, dir, type) < 0)
-		BUG();
+	BUG_ON(build_migrate(skb, m, num_migrate, k, sel, dir, type) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MIGRATE);
 }
@@ -2524,8 +2520,7 @@ static int xfrm_aevent_state_notify(struct xfrm_state *x, const struct km_event
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_aevent(skb, x, c) < 0)
-		BUG();
+	BUG_ON(build_aevent(skb, x, c) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_AEVENTS);
 }
@@ -2731,8 +2726,7 @@ static int xfrm_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *xt,
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_acquire(skb, x, xt, xp) < 0)
-		BUG();
+	BUG_ON(build_acquire(skb, x, xt, xp) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_ACQUIRE);
 }
@@ -2846,8 +2840,7 @@ static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, const struct
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, c) < 0)
-		BUG();
+	BUG_ON(build_polexpire(skb, xp, dir, c) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_EXPIRE);
 }
@@ -3006,8 +2999,7 @@ static int xfrm_send_report(struct net *net, u8 proto,
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_report(skb, proto, sel, addr) < 0)
-		BUG();
+	BUG_ON(build_report(skb, proto, sel, addr) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_REPORT);
 }
@@ -3059,8 +3051,7 @@ static int xfrm_send_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr,
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_mapping(skb, x, ipaddr, sport) < 0)
-		BUG();
+	BUG_ON(build_mapping(skb, x, ipaddr, sport) < 0);
 
 	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MAPPING);
 }
-- 
1.9.1

[PATCH 6/9 net-next] af_packet: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/packet/af_packet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 5102c3c..fb94c39 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -547,8 +547,7 @@ static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
 {
 	struct tpacket_kbdq_core *pkc;
 
-	if (tx_ring)
-		BUG();
+	BUG_ON(tx_ring);
 
 	pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring) :
 			GET_PBDQC_FROM_RB(&po->rx_ring);
-- 
1.9.1

[PATCH 7/9 net-next] sunrpc: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 9 +++------
 net/sunrpc/svc_xprt.c             | 3 +--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 1095be9..09f8a1c6 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -840,11 +840,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
 		return stat;
 	if (integ_len > buf->len)
 		return stat;
-	if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
-		BUG();
+	BUG_ON(xdr_buf_subsegment(buf, &integ_buf, 0, integ_len));
 	/* copy out mic... */
-	if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
-		BUG();
+	BUG_ON(read_u32_from_xdr_buf(buf, integ_len, &mic.len));
 	if (mic.len > RPC_MAX_AUTH_SIZE)
 		return stat;
 	mic.data = kmalloc(mic.len, GFP_KERNEL);
@@ -1595,8 +1593,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst *rqstp)
 	BUG_ON(integ_len % 4);
 	*p++ = htonl(integ_len);
 	*p++ = htonl(gc->gc_seq);
-	if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len))
-		BUG();
+	BUG_ON(xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len));
 	if (resbuf->tail[0].iov_base == NULL) {
 		if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
 			goto out_err;
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 163ac45..2f82e8b 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -960,8 +960,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
 	struct svc_deferred_req *dr;
 
 	/* Only do this once */
-	if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
-		BUG();
+	BUG_ON(test_and_set_bit(XPT_DEAD, &xprt->xpt_flags));
 
 	dprintk("svc: svc_delete_xprt(%p)\n", xprt);
 	xprt->xpt_ops->xpo_detach(xprt);
-- 
1.9.1

[PATCH 8/9 net-next] rxrpc: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/rxrpc/ar-accept.c   |  6 ++----
 net/rxrpc/ar-ack.c      |  6 ++----
 net/rxrpc/ar-call.c     |  3 +--
 net/rxrpc/ar-input.c    |  6 ++----
 net/rxrpc/ar-internal.h |  3 +--
 net/rxrpc/ar-key.c      |  3 +--
 net/rxrpc/ar-recvmsg.c  | 16 +++++-----------
 net/rxrpc/rxkad.c       | 19 +++++++------------
 8 files changed, 21 insertions(+), 41 deletions(-)

diff --git a/net/rxrpc/ar-accept.c b/net/rxrpc/ar-accept.c
index 6d79310..8f4cb91 100644
--- a/net/rxrpc/ar-accept.c
+++ b/net/rxrpc/ar-accept.c
@@ -377,10 +377,8 @@ struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *rx,
 	call->user_call_ID = user_call_ID;
 	rb_link_node(&call->sock_node, parent, pp);
 	rb_insert_color(&call->sock_node, &rx->calls);
-	if (test_and_set_bit(RXRPC_CALL_HAS_USERID, &call->flags))
-		BUG();
-	if (test_and_set_bit(RXRPC_CALL_ACCEPTED, &call->events))
-		BUG();
+	BUG_ON(test_and_set_bit(RXRPC_CALL_HAS_USERID, &call->flags));
+	BUG_ON(test_and_set_bit(RXRPC_CALL_ACCEPTED, &call->events));
 	rxrpc_queue_call(call);
 
 	rxrpc_get_call(call);
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index e0547f5..e648ac9 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -673,8 +673,7 @@ process_further:
 			_debug("extraction failure");
 			goto protocol_error;
 		}
-		if (!skb_pull(skb, sizeof(ack)))
-			BUG();
+		BUG_ON(!skb_pull(skb, sizeof(ack)));
 
 		latest = ntohl(sp->hdr.serial);
 		hard = ntohl(ack.firstPacket);
@@ -803,8 +802,7 @@ all_acked:
 		sp->call = call;
 		rxrpc_get_call(call);
 		spin_lock_bh(&call->lock);
-		if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0)
-			BUG();
+		BUG_ON(rxrpc_queue_rcv_skb(call, skb, true, true) < 0);
 		spin_unlock_bh(&call->lock);
 		goto process_further;
 	}
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index a9e05db..2e2c37f 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -621,8 +621,7 @@ void rxrpc_release_call(struct rxrpc_call *call)
 	       call->rx_first_oos);
 
 	spin_lock_bh(&call->lock);
-	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags))
-		BUG();
+	BUG_ON(test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags));
 	spin_unlock_bh(&call->lock);
 
 	/* dissociate from the socket
diff --git a/net/rxrpc/ar-input.c b/net/rxrpc/ar-input.c
index 481f89f..185ad34 100644
--- a/net/rxrpc/ar-input.c
+++ b/net/rxrpc/ar-input.c
@@ -478,8 +478,7 @@ static void rxrpc_process_jumbo_packet(struct rxrpc_call *call,
 
 		if (skb_copy_bits(jumbo, 0, &jhdr, sizeof(jhdr)) < 0)
 			goto protocol_error;
-		if (!pskb_pull(jumbo, sizeof(jhdr)))
-			BUG();
+		BUG_ON(!pskb_pull(jumbo, sizeof(jhdr)));
 
 		sp->hdr.seq	= htonl(ntohl(sp->hdr.seq) + 1);
 		sp->hdr.serial	= htonl(ntohl(sp->hdr.serial) + 1);
@@ -685,8 +684,7 @@ void rxrpc_data_ready(struct sock *sk)
 	if (skb_copy_bits(skb, sizeof(struct udphdr), &sp->hdr,
 			  sizeof(sp->hdr)) < 0)
 		goto bad_message;
-	if (!pskb_pull(skb, sizeof(struct udphdr) + sizeof(sp->hdr)))
-		BUG();
+	BUG_ON(!pskb_pull(skb, sizeof(struct udphdr) + sizeof(sp->hdr)));
 
 	_net("Rx RxRPC %s ep=%x call=%x:%x",
 	     sp->hdr.flags & RXRPC_CLIENT_INITIATED ? "ToServer" : "ToClient",
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 2fc1e65..3c30274 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -800,8 +800,7 @@ static inline void __rxrpc_get_local(struct rxrpc_local *local, const char *f)
 #define rxrpc_get_call(CALL)				\
 do {							\
 	CHECK_SLAB_OKAY(&(CALL)->usage);		\
-	if (atomic_inc_return(&(CALL)->usage) == 1)	\
-		BUG();					\
+	BUG_ON(atomic_inc_return(&(CALL)->usage) == 1); \
 } while(0)
 
 #define rxrpc_put_call(CALL)				\
diff --git a/net/rxrpc/ar-key.c b/net/rxrpc/ar-key.c
index db0f39f..8982d76 100644
--- a/net/rxrpc/ar-key.c
+++ b/net/rxrpc/ar-key.c
@@ -839,8 +839,7 @@ static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
 		return PTR_ERR(ci);
 	}
 
-	if (crypto_blkcipher_setkey(ci, prep->data, 8) < 0)
-		BUG();
+	BUG_ON(crypto_blkcipher_setkey(ci, prep->data, 8) < 0);
 
 	prep->payload[0] = ci;
 	_leave(" = 0");
diff --git a/net/rxrpc/ar-recvmsg.c b/net/rxrpc/ar-recvmsg.c
index b92bede..341a1b8 100644
--- a/net/rxrpc/ar-recvmsg.c
+++ b/net/rxrpc/ar-recvmsg.c
@@ -121,8 +121,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 
 		if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
 			_debug("packet from released call");
-			if (skb_dequeue(&rx->sk.sk_receive_queue) != skb)
-				BUG();
+			BUG_ON(skb_dequeue(&rx->sk.sk_receive_queue) != skb);
 			rxrpc_free_skb(skb);
 			continue;
 		}
@@ -212,9 +211,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 			/* last bit of request received */
 			if (!(flags & MSG_PEEK)) {
 				_debug("eat packet");
-				if (skb_dequeue(&rx->sk.sk_receive_queue) !=
-				    skb)
-					BUG();
+				BUG_ON(skb_dequeue(&rx->sk.sk_receive_queue) != skb);
 				rxrpc_free_skb(skb);
 			}
 			msg->msg_flags &= ~MSG_MORE;
@@ -238,8 +235,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		}
 
 		_debug("eat packet");
-		if (skb_dequeue(&rx->sk.sk_receive_queue) != skb)
-			BUG();
+		BUG_ON(skb_dequeue(&rx->sk.sk_receive_queue) != skb);
 		rxrpc_free_skb(skb);
 	}
 
@@ -265,8 +261,7 @@ receive_non_data_message:
 		if (ret < 0)
 			goto copy_error;
 		if (!(flags & MSG_PEEK)) {
-			if (skb_dequeue(&rx->sk.sk_receive_queue) != skb)
-				BUG();
+			BUG_ON(skb_dequeue(&rx->sk.sk_receive_queue) != skb);
 			rxrpc_free_skb(skb);
 		}
 		goto out;
@@ -317,8 +312,7 @@ terminal_message:
 
 	if (!(flags & MSG_PEEK)) {
 		_net("free terminal skb %p", skb);
-		if (skb_dequeue(&rx->sk.sk_receive_queue) != skb)
-			BUG();
+		BUG_ON(skb_dequeue(&rx->sk.sk_receive_queue) != skb);
 		rxrpc_free_skb(skb);
 		rxrpc_remove_user_ID(rx, call);
 	}
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index f226709..6793d57 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -77,9 +77,8 @@ static int rxkad_init_connection_security(struct rxrpc_connection *conn)
 		goto error;
 	}
 
-	if (crypto_blkcipher_setkey(ci, token->kad->session_key,
-				    sizeof(token->kad->session_key)) < 0)
-		BUG();
+	BUG_ON(crypto_blkcipher_setkey(ci, token->kad->session_key,
+				       sizeof(token->kad->session_key)) < 0);
 
 	switch (conn->security_level) {
 	case RXRPC_SECURITY_PLAIN:
@@ -361,8 +360,7 @@ static int rxkad_verify_packet_auth(const struct rxrpc_call *call,
 	/* remove the decrypted packet length */
 	if (skb_copy_bits(skb, 0, &sechdr, sizeof(sechdr)) < 0)
 		goto datalen_error;
-	if (!skb_pull(skb, sizeof(sechdr)))
-		BUG();
+	BUG_ON(!skb_pull(skb, sizeof(sechdr)));
 
 	buf = ntohl(sechdr.data_size);
 	data_size = buf & 0xffff;
@@ -446,8 +444,7 @@ static int rxkad_verify_packet_encrypt(const struct rxrpc_call *call,
 	/* remove the decrypted packet length */
 	if (skb_copy_bits(skb, 0, &sechdr, sizeof(sechdr)) < 0)
 		goto datalen_error;
-	if (!skb_pull(skb, sizeof(sechdr)))
-		BUG();
+	BUG_ON(!skb_pull(skb, sizeof(sechdr)));
 
 	buf = ntohl(sechdr.data_size);
 	data_size = buf & 0xffff;
@@ -976,9 +973,8 @@ static void rxkad_decrypt_response(struct rxrpc_connection *conn,
 	ASSERT(rxkad_ci != NULL);
 
 	mutex_lock(&rxkad_ci_mutex);
-	if (crypto_blkcipher_setkey(rxkad_ci, session_key->x,
-				    sizeof(*session_key)) < 0)
-		BUG();
+	BUG_ON(crypto_blkcipher_setkey(rxkad_ci, session_key->x,
+				       sizeof(*session_key)) < 0);
 
 	memcpy(&iv, session_key, sizeof(iv));
 	desc.tfm = rxkad_ci;
@@ -1014,8 +1010,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 	abort_code = RXKADPACKETSHORT;
 	if (skb_copy_bits(skb, 0, &response, sizeof(response)) < 0)
 		goto protocol_error;
-	if (!pskb_pull(skb, sizeof(response)))
-		BUG();
+	BUG_ON(!pskb_pull(skb, sizeof(response)));
 
 	version = ntohl(response.version);
 	ticket_len = ntohl(response.ticket_len);
-- 
1.9.1

[PATCH 9/9 net-next] netfilter: replace if/BUG by BUG_ON

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/netfilter/nfnetlink_log.c | 3 +--
 net/netfilter/xt_u32.c        | 8 +++-----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 957b83a..409f5fb 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -577,8 +577,7 @@ __build_packet_message(struct nfnl_log_net *log,
 		nla->nla_type = NFULA_PAYLOAD;
 		nla->nla_len = size;
 
-		if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
-			BUG();
+		BUG_ON(skb_copy_bits(skb, 0, nla_data(nla), data_len));
 	}
 
 	nlh->nlmsg_len = inst->skb->tail - old_tail;
diff --git a/net/netfilter/xt_u32.c b/net/netfilter/xt_u32.c
index a95b5034..5ff937d 100644
--- a/net/netfilter/xt_u32.c
+++ b/net/netfilter/xt_u32.c
@@ -38,8 +38,7 @@ static bool u32_match_it(const struct xt_u32 *data,
 		if (skb->len < 4 || pos > skb->len - 4)
 			return false;
 
-		if (skb_copy_bits(skb, pos, &n, sizeof(n)) < 0)
-			BUG();
+		BUG_ON(skb_copy_bits(skb, pos, &n, sizeof(n)) < 0);
 		val   = ntohl(n);
 		nnums = ct->nnums;
 
@@ -65,9 +64,8 @@ static bool u32_match_it(const struct xt_u32 *data,
 				    pos > skb->len - at - 4)
 					return false;
 
-				if (skb_copy_bits(skb, at + pos, &n,
-						    sizeof(n)) < 0)
-					BUG();
+				BUG_ON(skb_copy_bits(skb, at + pos, &n,
+						     sizeof(n)) < 0);
 				val = ntohl(n);
 				break;
 			}
-- 
1.9.1

Re: [PATCH 7/9 net-next] sunrpc: replace if/BUG by BUG_ON

[J. Bruce Fields]

Huh, I thought this wasn't recommended:

	http://lkml.kernel.org/r/20040828125816.206ef7fa.akpm@osdl.org

	"I'd prefer that we not move code which has side-effects into
	BUG_ONs"

--b.

On Mon, Mar 30, 2015 at 11:13:15PM +0200, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  net/sunrpc/auth_gss/svcauth_gss.c | 9 +++------
>  net/sunrpc/svc_xprt.c             | 3 +--
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 1095be9..09f8a1c6 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -840,11 +840,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
>  		return stat;
>  	if (integ_len > buf->len)
>  		return stat;
> -	if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
> -		BUG();
> +	BUG_ON(xdr_buf_subsegment(buf, &integ_buf, 0, integ_len));
>  	/* copy out mic... */
> -	if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> -		BUG();
> +	BUG_ON(read_u32_from_xdr_buf(buf, integ_len, &mic.len));
>  	if (mic.len > RPC_MAX_AUTH_SIZE)
>  		return stat;
>  	mic.data = kmalloc(mic.len, GFP_KERNEL);
> @@ -1595,8 +1593,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst *rqstp)
>  	BUG_ON(integ_len % 4);
>  	*p++ = htonl(integ_len);
>  	*p++ = htonl(gc->gc_seq);
> -	if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len))
> -		BUG();
> +	BUG_ON(xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len));
>  	if (resbuf->tail[0].iov_base == NULL) {
>  		if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
>  			goto out_err;
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 163ac45..2f82e8b 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -960,8 +960,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
>  	struct svc_deferred_req *dr;
>  
>  	/* Only do this once */
> -	if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
> -		BUG();
> +	BUG_ON(test_and_set_bit(XPT_DEAD, &xprt->xpt_flags));
>  
>  	dprintk("svc: svc_delete_xprt(%p)\n", xprt);
>  	xprt->xpt_ops->xpo_detach(xprt);
> -- 
> 1.9.1

Re: [PATCH 4/9 net-next] ipv6: replace if/BUG by BUG_ON

[YOSHIFUJI Hideaki]

Hi,

Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  net/ipv6/addrconf.c                     | 3 +--
>  net/ipv6/esp6.c                         | 3 +--
>  net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +--
>  3 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 2660263..e205918 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -4805,8 +4805,7 @@ static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
>  	if (!idev)
>  		return -EAFNOSUPPORT;
>  
> -	if (nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0)
> -		BUG();
> +	BUG_ON(nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0);
>  

NACK, I do not prefer using BUG_ON() with side effects.

--yoshfuji

>  	if (tb[IFLA_INET6_TOKEN]) {
>  		err = inet6_set_iftoken(idev, nla_data(tb[IFLA_INET6_TOKEN]));
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index e48f2c7..9e51b69 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -280,8 +280,7 @@ static int esp_input_done2(struct sk_buff *skb, int err)
>  	if (unlikely(err))
>  		goto out;
>  
> -	if (skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2))
> -		BUG();
> +	BUG_ON(skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2));
>  
>  	err = -EINVAL;
>  	padlen = nexthdr[0];
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index 6f187c8..cea1a4a 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -538,8 +538,7 @@ find_prev_fhdr(struct sk_buff *skb, u8 *prevhdrp, int *prevhoff, int *fhoff)
>  			pr_debug("too short\n");
>  			return -1;
>  		}
> -		if (skb_copy_bits(skb, start, &hdr, sizeof(hdr)))
> -			BUG();
> +		BUG_ON(skb_copy_bits(skb, start, &hdr, sizeof(hdr)));
>  		if (nexthdr == NEXTHDR_AUTH)
>  			hdrlen = (hdr.hdrlen+2)<<2;
>  		else
> 

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

Re: [PATCH 4/9 net-next] ipv6: replace if/BUG by BUG_ON

[David Miller]

From: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
Date: Tue, 31 Mar 2015 12:50:27 +0900

> Hi,
> 
> Fabian Frederick wrote:
>> Signed-off-by: Fabian Frederick <fabf@skynet.be>
>> ---
>>  net/ipv6/addrconf.c                     | 3 +--
>>  net/ipv6/esp6.c                         | 3 +--
>>  net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +--
>>  3 files changed, 3 insertions(+), 6 deletions(-)
>> 
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 2660263..e205918 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -4805,8 +4805,7 @@ static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
>>  	if (!idev)
>>  		return -EAFNOSUPPORT;
>>  
>> -	if (nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0)
>> -		BUG();
>> +	BUG_ON(nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0);
>>  
> 
> NACK, I do not prefer using BUG_ON() with side effects.

Agreed, I do not like these changes at all and will not be
applying them.

Re: [PATCH 7/9 net-next] sunrpc: replace if/BUG by BUG_ON

[Fabian Frederick]

> On 30 March 2015 at 23:25 "J. Bruce Fields" <bfields@fieldses.org> wrote:
>
>
> Huh, I thought this wasn't recommended:
>
>       http://lkml.kernel.org/r/20040828125816.206ef7fa.akpm@osdl.org
>
>       "I'd prefer that we not move code which has side-effects into
>       BUG_ONs"

Thanks for the link, I wasn't aware of that problem. Maybe we should add some
documentation and fix coccinelle detection then ?

Regards,
Fabian

>
> --b.
>
> On Mon, Mar 30, 2015 at 11:13:15PM +0200, Fabian Frederick wrote:
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> >  net/sunrpc/auth_gss/svcauth_gss.c | 9 +++------
> >  net/sunrpc/svc_xprt.c             | 3 +--
> >  2 files changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c
> > b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 1095be9..09f8a1c6 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -840,11 +840,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct
> > xdr_buf *buf, u32 seq, struct g
> >             return stat;
> >     if (integ_len > buf->len)
> >             return stat;
> > -   if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
> > -           BUG();
> > +   BUG_ON(xdr_buf_subsegment(buf, &integ_buf, 0, integ_len));
> >     /* copy out mic... */
> > -   if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> > -           BUG();
> > +   BUG_ON(read_u32_from_xdr_buf(buf, integ_len, &mic.len));
> >     if (mic.len > RPC_MAX_AUTH_SIZE)
> >             return stat;
> >     mic.data = kmalloc(mic.len, GFP_KERNEL);
> > @@ -1595,8 +1593,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst *rqstp)
> >     BUG_ON(integ_len % 4);
> >     *p++ = htonl(integ_len);
> >     *p++ = htonl(gc->gc_seq);
> > -   if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len))
> > -           BUG();
> > +   BUG_ON(xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len));
> >     if (resbuf->tail[0].iov_base == NULL) {
> >             if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
> >                     goto out_err;
> > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > index 163ac45..2f82e8b 100644
> > --- a/net/sunrpc/svc_xprt.c
> > +++ b/net/sunrpc/svc_xprt.c
> > @@ -960,8 +960,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> >     struct svc_deferred_req *dr;
> > 
> >     /* Only do this once */
> > -   if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
> > -           BUG();
> > +   BUG_ON(test_and_set_bit(XPT_DEAD, &xprt->xpt_flags));
> > 
> >     dprintk("svc: svc_delete_xprt(%p)\n", xprt);
> >     xprt->xpt_ops->xpo_detach(xprt);
> > --
> > 1.9.1

Re: [PATCH 7/9 net-next] sunrpc: replace if/BUG by BUG_ON

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3258 bytes --]



On Tue, 31 Mar 2015, Fabian Frederick wrote:

> 
> 
> > On 30 March 2015 at 23:25 "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >
> >
> > Huh, I thought this wasn't recommended:
> >
> >       http://lkml.kernel.org/r/20040828125816.206ef7fa.akpm@osdl.org
> >
> >       "I'd prefer that we not move code which has side-effects into
> >       BUG_ONs"
> 
> Thanks for the link, I wasn't aware of that problem. Maybe we should add some
> documentation and fix coccinelle detection then ?

Maybe the comment in the Coccinelle rule could just be made more clear?  
The only practical choices are to ignore all function calls and to allow 
all fuction calls, since Coccinelle doesn't know which ones have side 
effects.

julia

> Regards,
> Fabian
> 
> >
> > --b.
> >
> > On Mon, Mar 30, 2015 at 11:13:15PM +0200, Fabian Frederick wrote:
> > > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > > ---
> > >  net/sunrpc/auth_gss/svcauth_gss.c | 9 +++------
> > >  net/sunrpc/svc_xprt.c             | 3 +--
> > >  2 files changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c
> > > b/net/sunrpc/auth_gss/svcauth_gss.c
> > > index 1095be9..09f8a1c6 100644
> > > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > > @@ -840,11 +840,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct
> > > xdr_buf *buf, u32 seq, struct g
> > >             return stat;
> > >     if (integ_len > buf->len)
> > >             return stat;
> > > -   if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
> > > -           BUG();
> > > +   BUG_ON(xdr_buf_subsegment(buf, &integ_buf, 0, integ_len));
> > >     /* copy out mic... */
> > > -   if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> > > -           BUG();
> > > +   BUG_ON(read_u32_from_xdr_buf(buf, integ_len, &mic.len));
> > >     if (mic.len > RPC_MAX_AUTH_SIZE)
> > >             return stat;
> > >     mic.data = kmalloc(mic.len, GFP_KERNEL);
> > > @@ -1595,8 +1593,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst *rqstp)
> > >     BUG_ON(integ_len % 4);
> > >     *p++ = htonl(integ_len);
> > >     *p++ = htonl(gc->gc_seq);
> > > -   if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len))
> > > -           BUG();
> > > +   BUG_ON(xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len));
> > >     if (resbuf->tail[0].iov_base == NULL) {
> > >             if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
> > >                     goto out_err;
> > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > > index 163ac45..2f82e8b 100644
> > > --- a/net/sunrpc/svc_xprt.c
> > > +++ b/net/sunrpc/svc_xprt.c
> > > @@ -960,8 +960,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> > >     struct svc_deferred_req *dr;
> > > 
> > >     /* Only do this once */
> > > -   if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
> > > -           BUG();
> > > +   BUG_ON(test_and_set_bit(XPT_DEAD, &xprt->xpt_flags));
> > > 
> > >     dprintk("svc: svc_delete_xprt(%p)\n", xprt);
> > >     xprt->xpt_ops->xpo_detach(xprt);
> > > --
> > > 1.9.1
> 

Re: [PATCH 7/9 net-next] sunrpc: replace if/BUG by BUG_ON

[Fabian Frederick]

> On 31 March 2015 at 21:11 Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>
>
> On Tue, 31 Mar 2015, Fabian Frederick wrote:
>
> >
> >
> > > On 30 March 2015 at 23:25 "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > >
> > >
> > > Huh, I thought this wasn't recommended:
> > >
> > >       http://lkml.kernel.org/r/20040828125816.206ef7fa.akpm@osdl.org
> > >
> > >       "I'd prefer that we not move code which has side-effects into
> > >       BUG_ONs"
> >
> > Thanks for the link, I wasn't aware of that problem. Maybe we should add
> > some
> > documentation and fix coccinelle detection then ?
>
> Maybe the comment in the Coccinelle rule could just be made more clear? 

Fair enough. Current "WARNING: Use BUG_ON" looks misleading.
Maybe some BUG_ON(function()) detection script would be interesting as well ?
There are nearly 5000 BUG_ON() in drivers branch only ... 

Regards,
Fabian
> The only practical choices are to ignore all function calls and to allow
> all fuction calls, since Coccinelle doesn't know which ones have side
> effects.
>
> julia
>
> > Regards,
> > Fabian
> >
> > >
> > > --b.
> > >
> > > On Mon, Mar 30, 2015 at 11:13:15PM +0200, Fabian Frederick wrote:
> > > > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > > > ---
> > > >  net/sunrpc/auth_gss/svcauth_gss.c | 9 +++------
> > > >  net/sunrpc/svc_xprt.c             | 3 +--
> > > >  2 files changed, 4 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c
> > > > b/net/sunrpc/auth_gss/svcauth_gss.c
> > > > index 1095be9..09f8a1c6 100644
> > > > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > > > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > > > @@ -840,11 +840,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct
> > > > xdr_buf *buf, u32 seq, struct g
> > > >             return stat;
> > > >     if (integ_len > buf->len)
> > > >             return stat;
> > > > -   if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
> > > > -           BUG();
> > > > +   BUG_ON(xdr_buf_subsegment(buf, &integ_buf, 0, integ_len));
> > > >     /* copy out mic... */
> > > > -   if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> > > > -           BUG();
> > > > +   BUG_ON(read_u32_from_xdr_buf(buf, integ_len, &mic.len));
> > > >     if (mic.len > RPC_MAX_AUTH_SIZE)
> > > >             return stat;
> > > >     mic.data = kmalloc(mic.len, GFP_KERNEL);
> > > > @@ -1595,8 +1593,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst
> > > > *rqstp)
> > > >     BUG_ON(integ_len % 4);
> > > >     *p++ = htonl(integ_len);
> > > >     *p++ = htonl(gc->gc_seq);
> > > > -   if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len))
> > > > -           BUG();
> > > > +   BUG_ON(xdr_buf_subsegment(resbuf, &integ_buf, integ_offset,
> > > > integ_len));
> > > >     if (resbuf->tail[0].iov_base == NULL) {
> > > >             if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
> > > >                     goto out_err;
> > > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > > > index 163ac45..2f82e8b 100644
> > > > --- a/net/sunrpc/svc_xprt.c
> > > > +++ b/net/sunrpc/svc_xprt.c
> > > > @@ -960,8 +960,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> > > >     struct svc_deferred_req *dr;
> > > > 
> > > >     /* Only do this once */
> > > > -   if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
> > > > -           BUG();
> > > > +   BUG_ON(test_and_set_bit(XPT_DEAD, &xprt->xpt_flags));
> > > > 
> > > >     dprintk("svc: svc_delete_xprt(%p)\n", xprt);
> > > >     xprt->xpt_ops->xpo_detach(xprt);
> > > > --
> > > > 1.9.1
> >

Re: [PATCH 4/9 net-next] ipv6: replace if/BUG by BUG_ON

[Fabian Frederick]

> On 31 March 2015 at 17:17 David Miller <davem@davemloft.net> wrote:
>
>
> From: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
> Date: Tue, 31 Mar 2015 12:50:27 +0900
>
> > Hi,
> >
> > Fabian Frederick wrote:
> >> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> >> ---
> >>  net/ipv6/addrconf.c                     | 3 +--
> >>  net/ipv6/esp6.c                         | 3 +--
> >>  net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +--
> >>  3 files changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >> index 2660263..e205918 100644
> >> --- a/net/ipv6/addrconf.c
> >> +++ b/net/ipv6/addrconf.c
> >> @@ -4805,8 +4805,7 @@ static int inet6_set_link_af(struct net_device *dev,
> >> const struct nlattr *nla)
> >>    if (!idev)
> >>            return -EAFNOSUPPORT;
> >> 
> >> -  if (nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0)
> >> -          BUG();
> >> +  BUG_ON(nla_parse_nested(tb, IFLA_INET6_MAX, nla, NULL) < 0);
> >> 
> >
> > NACK, I do not prefer using BUG_ON() with side effects.
>
> Agreed, I do not like these changes at all and will not be
> applying them.

I guess patches like these should be reverted then ?

e3f0b86b996d
"ipv6: Use BUG_ON"

8242fc33925c
"net: ipv6: Use BUG_ON"

Some existing BUG_ON() as well ? ; eg second one in net/decnet/dn_rules.c:
void __init dn_fib_rules_init(void)
{
        dn_fib_rules_ops =
                fib_rules_register(&dn_fib_rules_ops_template, &init_net);
        BUG_ON(IS_ERR(dn_fib_rules_ops));
        BUG_ON(fib_default_rule_add(dn_fib_rules_ops, 0x7fff,
                                    RT_TABLE_MAIN, 0));
}

Regards,
Fabian