From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Horman Subject: Re: [PATCHv2] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG Date: Thu, 13 Sep 2018 12:28:59 +0200 Message-ID: <20180913102859.kpeelj3mycjnyvhd@netronome.com> References: <1536671593-14746-1-git-send-email-zhongjiang@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, edumazet@google.com, kuznet@ms2.inr.ac.ru, netdev@vger.kernel.org, linux-kernel@vger.kernel.org To: zhong jiang Return-path: Content-Disposition: inline In-Reply-To: <1536671593-14746-1-git-send-email-zhongjiang@huawei.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Tue, Sep 11, 2018 at 09:13:13PM +0800, zhong jiang wrote: > The if condition can be removed if we use BUG_ON directly. > The issule is detected with the help of Coccinelle. > > Signed-off-by: zhong jiang > --- > net/ipv4/tcp_input.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 62508a2..526c05e 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb) > 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)); Perhaps things have changed but it doesn't seem desirable to me to use BUG_ON with statements that have side-effects in case at some point BUG_ON gets compiled out. > TCP_SKB_CB(nskb)->end_seq += size; > copy -= size; > start += size; > @@ -5327,8 +5327,8 @@ 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); > -- > 1.7.12.4 >