[PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[zhong jiang]

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 <zhongjiang@huawei.com>
---
 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..893bde3 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(skb_copy_bits(skb, offset,
+						  skb_put(nskb, size), size));
 				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(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

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[Jiri Benc]

On Mon, 10 Sep 2018 22:38:02 +0800, zhong jiang wrote:
> +			BUG(skb_copy_bits(skb, ptr, &tmp, 1));

I doubt this builds.

 Jiri

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5317 bytes --]

Hi zhong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/zhong-jiang/net-ipv4-Use-BUG_ON-directly-instead-of-a-if-condition-followed-by-BUG/20180911-034152
config: x86_64-randconfig-x009-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net//ipv4/tcp_input.c: In function 'tcp_collapse':
>> net//ipv4/tcp_input.c:4925:35: error: macro "BUG" passed 1 arguments, but takes just 0
            skb_put(nskb, size), size));
                                      ^
>> net//ipv4/tcp_input.c:4924:5: error: 'BUG' undeclared (first use in this function)
        BUG(skb_copy_bits(skb, offset,
        ^~~
   net//ipv4/tcp_input.c:4924:5: note: each undeclared identifier is reported only once for each function it appears in
   net//ipv4/tcp_input.c: In function 'tcp_urg':
   net//ipv4/tcp_input.c:5318:40: error: macro "BUG" passed 1 arguments, but takes just 0
       BUG(skb_copy_bits(skb, ptr, &tmp, 1));
                                           ^
   net//ipv4/tcp_input.c:5318:4: error: 'BUG' undeclared (first use in this function)
       BUG(skb_copy_bits(skb, ptr, &tmp, 1));
       ^~~

vim +/BUG +4925 net//ipv4/tcp_input.c

  4838	
  4839	/* Collapse contiguous sequence of skbs head..tail with
  4840	 * sequence numbers start..end.
  4841	 *
  4842	 * If tail is NULL, this means until the end of the queue.
  4843	 *
  4844	 * Segments with FIN/SYN are not collapsed (only because this
  4845	 * simplifies code)
  4846	 */
  4847	static void
  4848	tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
  4849		     struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
  4850	{
  4851		struct sk_buff *skb = head, *n;
  4852		struct sk_buff_head tmp;
  4853		bool end_of_skbs;
  4854	
  4855		/* First, check that queue is collapsible and find
  4856		 * the point where collapsing can be useful.
  4857		 */
  4858	restart:
  4859		for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
  4860			n = tcp_skb_next(skb, list);
  4861	
  4862			/* No new bits? It is possible on ofo queue. */
  4863			if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4864				skb = tcp_collapse_one(sk, skb, list, root);
  4865				if (!skb)
  4866					break;
  4867				goto restart;
  4868			}
  4869	
  4870			/* The first skb to collapse is:
  4871			 * - not SYN/FIN and
  4872			 * - bloated or contains data before "start" or
  4873			 *   overlaps to the next one.
  4874			 */
  4875			if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
  4876			    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
  4877			     before(TCP_SKB_CB(skb)->seq, start))) {
  4878				end_of_skbs = false;
  4879				break;
  4880			}
  4881	
  4882			if (n && n != tail &&
  4883			    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
  4884				end_of_skbs = false;
  4885				break;
  4886			}
  4887	
  4888			/* Decided to skip this, advance start seq. */
  4889			start = TCP_SKB_CB(skb)->end_seq;
  4890		}
  4891		if (end_of_skbs ||
  4892		    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4893			return;
  4894	
  4895		__skb_queue_head_init(&tmp);
  4896	
  4897		while (before(start, end)) {
  4898			int copy = min_t(int, SKB_MAX_ORDER(0, 0), end - start);
  4899			struct sk_buff *nskb;
  4900	
  4901			nskb = alloc_skb(copy, GFP_ATOMIC);
  4902			if (!nskb)
  4903				break;
  4904	
  4905			memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
  4906	#ifdef CONFIG_TLS_DEVICE
  4907			nskb->decrypted = skb->decrypted;
  4908	#endif
  4909			TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
  4910			if (list)
  4911				__skb_queue_before(list, skb, nskb);
  4912			else
  4913				__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
  4914			skb_set_owner_r(nskb, sk);
  4915	
  4916			/* Copy data, releasing collapsed skbs. */
  4917			while (copy > 0) {
  4918				int offset = start - TCP_SKB_CB(skb)->seq;
  4919				int size = TCP_SKB_CB(skb)->end_seq - start;
  4920	
  4921				BUG_ON(offset < 0);
  4922				if (size > 0) {
  4923					size = min(copy, size);
> 4924					BUG(skb_copy_bits(skb, offset,
> 4925							  skb_put(nskb, size), size));
  4926					TCP_SKB_CB(nskb)->end_seq += size;
  4927					copy -= size;
  4928					start += size;
  4929				}
  4930				if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4931					skb = tcp_collapse_one(sk, skb, list, root);
  4932					if (!skb ||
  4933					    skb == tail ||
  4934					    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4935						goto end;
  4936	#ifdef CONFIG_TLS_DEVICE
  4937					if (skb->decrypted != nskb->decrypted)
  4938						goto end;
  4939	#endif
  4940				}
  4941			}
  4942		}
  4943	end:
  4944		skb_queue_walk_safe(&tmp, skb, n)
  4945			tcp_rbtree_insert(root, skb);
  4946	}
  4947	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27230 bytes --]

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5670 bytes --]

Hi zhong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/zhong-jiang/net-ipv4-Use-BUG_ON-directly-instead-of-a-if-condition-followed-by-BUG/20180911-034152
config: mips-rt305x_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   net/ipv4/tcp_input.c: In function 'tcp_collapse':
>> net/ipv4/tcp_input.c:4924:5: error: too many arguments to function 'BUG'
        BUG(skb_copy_bits(skb, offset,
        ^~~
   In file included from include/linux/bug.h:5:0,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from net/ipv4/tcp_input.c:67:
   arch/mips/include/asm/bug.h:12:31: note: declared here
    static inline void __noreturn BUG(void)
                                  ^~~
   net/ipv4/tcp_input.c: In function 'tcp_urg':
   net/ipv4/tcp_input.c:5318:4: error: too many arguments to function 'BUG'
       BUG(skb_copy_bits(skb, ptr, &tmp, 1));
       ^~~
   In file included from include/linux/bug.h:5:0,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from net/ipv4/tcp_input.c:67:
   arch/mips/include/asm/bug.h:12:31: note: declared here
    static inline void __noreturn BUG(void)
                                  ^~~

vim +/BUG +4924 net/ipv4/tcp_input.c

  4838	
  4839	/* Collapse contiguous sequence of skbs head..tail with
  4840	 * sequence numbers start..end.
  4841	 *
  4842	 * If tail is NULL, this means until the end of the queue.
  4843	 *
  4844	 * Segments with FIN/SYN are not collapsed (only because this
  4845	 * simplifies code)
  4846	 */
  4847	static void
  4848	tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
  4849		     struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
  4850	{
  4851		struct sk_buff *skb = head, *n;
  4852		struct sk_buff_head tmp;
  4853		bool end_of_skbs;
  4854	
  4855		/* First, check that queue is collapsible and find
  4856		 * the point where collapsing can be useful.
  4857		 */
  4858	restart:
  4859		for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
  4860			n = tcp_skb_next(skb, list);
  4861	
  4862			/* No new bits? It is possible on ofo queue. */
  4863			if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4864				skb = tcp_collapse_one(sk, skb, list, root);
  4865				if (!skb)
  4866					break;
  4867				goto restart;
  4868			}
  4869	
  4870			/* The first skb to collapse is:
  4871			 * - not SYN/FIN and
  4872			 * - bloated or contains data before "start" or
  4873			 *   overlaps to the next one.
  4874			 */
  4875			if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
  4876			    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
  4877			     before(TCP_SKB_CB(skb)->seq, start))) {
  4878				end_of_skbs = false;
  4879				break;
  4880			}
  4881	
  4882			if (n && n != tail &&
  4883			    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
  4884				end_of_skbs = false;
  4885				break;
  4886			}
  4887	
  4888			/* Decided to skip this, advance start seq. */
  4889			start = TCP_SKB_CB(skb)->end_seq;
  4890		}
  4891		if (end_of_skbs ||
  4892		    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4893			return;
  4894	
  4895		__skb_queue_head_init(&tmp);
  4896	
  4897		while (before(start, end)) {
  4898			int copy = min_t(int, SKB_MAX_ORDER(0, 0), end - start);
  4899			struct sk_buff *nskb;
  4900	
  4901			nskb = alloc_skb(copy, GFP_ATOMIC);
  4902			if (!nskb)
  4903				break;
  4904	
  4905			memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
  4906	#ifdef CONFIG_TLS_DEVICE
  4907			nskb->decrypted = skb->decrypted;
  4908	#endif
  4909			TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
  4910			if (list)
  4911				__skb_queue_before(list, skb, nskb);
  4912			else
  4913				__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
  4914			skb_set_owner_r(nskb, sk);
  4915	
  4916			/* Copy data, releasing collapsed skbs. */
  4917			while (copy > 0) {
  4918				int offset = start - TCP_SKB_CB(skb)->seq;
  4919				int size = TCP_SKB_CB(skb)->end_seq - start;
  4920	
  4921				BUG_ON(offset < 0);
  4922				if (size > 0) {
  4923					size = min(copy, size);
> 4924					BUG(skb_copy_bits(skb, offset,
  4925							  skb_put(nskb, size), size));
  4926					TCP_SKB_CB(nskb)->end_seq += size;
  4927					copy -= size;
  4928					start += size;
  4929				}
  4930				if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4931					skb = tcp_collapse_one(sk, skb, list, root);
  4932					if (!skb ||
  4933					    skb == tail ||
  4934					    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4935						goto end;
  4936	#ifdef CONFIG_TLS_DEVICE
  4937					if (skb->decrypted != nskb->decrypted)
  4938						goto end;
  4939	#endif
  4940				}
  4941			}
  4942		}
  4943	end:
  4944		skb_queue_walk_safe(&tmp, skb, n)
  4945			tcp_rbtree_insert(root, skb);
  4946	}
  4947	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 14248 bytes --]

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[zhong jiang]

On 2018/9/10 23:39, Jiri Benc wrote:
> On Mon, 10 Sep 2018 22:38:02 +0800, zhong jiang wrote:
>> +			BUG(skb_copy_bits(skb, ptr, &tmp, 1));
> I doubt this builds.
 This is my fault. should Be BUG_ON

 Thanks,
 zhong jiang
>  Jiri
>
> .
>

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[Sergei Shtylyov]

On 9/10/2018 5:38 PM, zhong jiang wrote:

> The if condition can be removed if we use BUG_ON directly.
> The issule is detected with the help of Coccinelle.

   Issue?

> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>   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..893bde3 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(skb_copy_bits(skb, offset,

    You said BUG_ON()?

> +						  skb_put(nskb, size), size));
>   				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(skb_copy_bits(skb, ptr, &tmp, 1));

    Likewise.

[...]

MBR, Sergei

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[zhong jiang]

On 2018/9/11 16:51, Sergei Shtylyov wrote:
> On 9/10/2018 5:38 PM, zhong jiang wrote:
>
>> The if condition can be removed if we use BUG_ON directly.
>> The issule is detected with the help of Coccinelle.
>
>   Issue?
>
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>>   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..893bde3 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(skb_copy_bits(skb, offset,
>
>    You said BUG_ON()?
 Yep. Do you think that it is worthing to do

 Thanks,
zhong jiang
>
>> +                          skb_put(nskb, size), size));
>>                   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(skb_copy_bits(skb, ptr, &tmp, 1));
>
>    Likewise.
>
> [...]
>
> MBR, Sergei
>
> .
>

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[Sergei Shtylyov]

On 9/11/2018 11:59 AM, zhong jiang wrote:

>>> The if condition can be removed if we use BUG_ON directly.
>>> The issule is detected with the help of Coccinelle.
>>
>>    Issue?
>>
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>>    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..893bde3 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(skb_copy_bits(skb, offset,
>>
>>     You said BUG_ON()?
>   Yep. Do you think that it is worthing to do

    I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to 
build the kernel with your patch at all?

>   Thanks,
> zhong jiang
[...]

MBR, Sergei

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[zhong jiang]

On 2018/9/11 17:11, Sergei Shtylyov wrote:
> On 9/11/2018 11:59 AM, zhong jiang wrote:
>
>>>> The if condition can be removed if we use BUG_ON directly.
>>>> The issule is detected with the help of Coccinelle.
>>>
>>>    Issue?
>>>
>>>>
>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>> ---
>>>>    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..893bde3 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(skb_copy_bits(skb, offset,
>>>
>>>     You said BUG_ON()?
>>   Yep. Do you think that it is worthing to do
>
>    I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to build the kernel with your patch at all?
>
 I know that the patch should be BUG_ON instead of BUG. This is my mistake.  I just want to know that it is worthing to do so.
 

 Thanks,
 zhong jiang
>>   Thanks,
>> zhong jiang
> [...]
>
> MBR, Sergei
>
>
>

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[Sergei Shtylyov]

On 9/11/2018 12:19 PM, zhong jiang wrote:

>>>>> The if condition can be removed if we use BUG_ON directly.
>>>>> The issule is detected with the help of Coccinelle.
>>>>
>>>>     Issue?
>>>>
>>>>>
>>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>>> ---
>>>>>     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..893bde3 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(skb_copy_bits(skb, offset,
>>>>
>>>>      You said BUG_ON()?
>>>    Yep. Do you think that it is worthing to do
>>
>>     I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to build the kernel with your patch at all?
>>
>   I know that the patch should be BUG_ON instead of BUG. This is my mistake.  I just want to know that it is worthing to do so.

    Yes, probably.

>   Thanks,
>   zhong jiang
[...]

MBR, Sergei

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[David Miller]

From: zhong jiang <zhongjiang@huawei.com>
Date: Mon, 10 Sep 2018 22:38:02 +0800

> 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 <zhongjiang@huawei.com>

You keep asking if this is worthy to do.

I think the most worthy thing to do is actually build test your
patches.

Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[zhong jiang]

On 2018/9/12 14:54, David Miller wrote:
> From: zhong jiang <zhongjiang@huawei.com>
> Date: Mon, 10 Sep 2018 22:38:02 +0800
>
>> 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 <zhongjiang@huawei.com>
> You keep asking if this is worthy to do.
>
> I think the most worthy thing to do is actually build test your
> patches.
 I am sorry for that.  :-[

Thanks,
zhong jiang